From owner-freebsd-current@FreeBSD.ORG Thu Nov 28 08:20:06 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7C41C486; Thu, 28 Nov 2013 08:20:06 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 1C3E71CCF; Thu, 28 Nov 2013 08:20:05 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.7/8.14.7) with ESMTP id rAS8K0Lw025018; Thu, 28 Nov 2013 10:20:00 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua rAS8K0Lw025018 Received: (from kostik@localhost) by tom.home (8.14.7/8.14.7/Submit) id rAS8K0kJ025017; Thu, 28 Nov 2013 10:20:00 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 28 Nov 2013 10:20:00 +0200 From: Konstantin Belousov To: Adrian Chadd Subject: Re: [review] sendfile / sendfile_sync refactoring Message-ID: <20131128082000.GK59496@kib.kiev.ua> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="H55dy74Id38Dzf32" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.22 (2013-10-16) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: freebsd-current , Gleb Smirnoff , "freebsd-arch@freebsd.org" X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Nov 2013 08:20:06 -0000 --H55dy74Id38Dzf32 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 27, 2013 at 11:36:44AM -0800, Adrian Chadd wrote: > Hi, >=20 > Here's part #2 in my sendfile refactoring. This is a little more > intrusive than the first patch. >=20 > http://people.freebsd.org/~adrian/netflix/20131126-sendfile-sync-refactor= -2.diff >=20 sf_buf.h is for sf buffers, and not for sendfile stuff. Please do not pollute this header. The changes you have to make to if_ti.c illustrate my point. The sys/event.h change of adding new kevent type is useless now and=20 has no relation to the rest of the patch. Patch has too many XXX notes for its triviality, some of which are baseless, e.g. the comment for struct sendfile_sync forward declaration in sys/file.h. You extracted all sfs accesses from the sendfile implementation, except the one in sf_buf_mext(). This is weird, please move the code from sf_buf_mext() into static function and put it near sf_sync_ref(). While moving the code into sf_sync_syscall_wait(), please use the opportunity and replace the if (sfs->count !=3D 0) with the while () cv_wait(); loop, to avoid spurious wakeups. I do not see any use for sfs->flags. Also, I do not see any use for passing the flags masked with SF_SYNC, which is always set, to sf_sync_alloc(). If the flags are going to be useful later, it should be added when needed later. The sf_sync_* stuff in the compat32 code just duplicates the main syscall. It should be done in the common code. > My aim here is to move the sendfile_sync stuff out of uipc_syscalls.c > and out of sendfile-only code and into something that can be reused > elsewhere or replaced later on. I've created methods for all the > sendfile_sync stuff, I've moved it out of the do_sendfile() loop so > do_sendfile() doesn't specifically implement the completion behaviour. As is, the patch is sincere nop. Modulo the comments above, I do not object against it. --H55dy74Id38Dzf32 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQIcBAEBAgAGBQJSlvywAAoJEJDCuSvBvK1Bs9gP/iv7zcSWRf51kPbynKkWlM42 XZx4Tg6OnMkykU/G/M5091AyBgtM6NdJcalmtn5suFST5xa42gP0XXhWx7PhRdTQ m+tCqEqs19QNhElDCQ9a/BtemTuEN1ZIg4vYyXpG1qH4mWK+hJr+zJrQNcEMUXDo P3n4t7H5p4z9+20ETIQUGMqyuP0xHlAafvljGTuDzgDfk+y/3LN8Y8bM6VTGDm9B NzCbkP4cvMzuoeGiHOzpv13xd7jiLPJxaDbzUJQNQblWR1j9cWZe/z4AnAeZpm3z n5Pon2SeDxIi3i8p357J3H4UouDEwN52yt7DaXCUTSaRjI1T1ElDmMOnZAkl3YxX f8ZWP+YtJCaS+ZOW3OzV6pIqDVNkLpXuuNrsj2AISNNyz7SxwL7WEB3aRGaksj1f IVEBGs0CDboG0DYO49AFcEJCih1c6FvSvKyzkI6dvyX6L+1di3BnrpIIe7SKxUfb bxl6vxuGuFHSQhGKcb6eT9D6RaDIrQBhHzyvMWaWGFqdITOSH/7uHqk5twYfFBN8 8hd4NVEG8Q3zv6UHjWX+wYf3Vs/NcZ9ayEWjimdSsUqykuerDBUfTWm5t6uuupNp QxiHGI85nEGx2xxiQW22vLn1ESeaP0oY7qWXd3i9GcUP3h0p5uVnpB+ZYPbP+1s9 v991+RluwIIic34q+5I8 =MPo5 -----END PGP SIGNATURE----- --H55dy74Id38Dzf32--