From owner-freebsd-arch@FreeBSD.ORG Mon Feb 6 09:54:24 2012 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B0ED5106566B; Mon, 6 Feb 2012 09:54:24 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 3F97C8FC12; Mon, 6 Feb 2012 09:54:23 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q169sGSe024524 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 6 Feb 2012 11:54:16 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q169sFHJ058022; Mon, 6 Feb 2012 11:54:15 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q169sFD0058021; Mon, 6 Feb 2012 11:54:15 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 6 Feb 2012 11:54:15 +0200 From: Konstantin Belousov To: Pawel Jakub Dawidek Message-ID: <20120206095415.GM3283@deviant.kiev.zoral.com.ua> References: <20120203193719.GB3283@deviant.kiev.zoral.com.ua> <20120205114345.GE30033@garage.freebsd.pl> <20120205164729.GH3283@deviant.kiev.zoral.com.ua> <20120205192045.GH30033@garage.freebsd.pl> <20120205221259.GL3283@deviant.kiev.zoral.com.ua> <20120206085814.GB1324@garage.freebsd.pl> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="T4pX28sySFCT/oEO" Content-Disposition: inline In-Reply-To: <20120206085814.GB1324@garage.freebsd.pl> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.9 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: arch@freebsd.org Subject: Re: Prefaulting for i/o buffers X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Feb 2012 09:54:24 -0000 --T4pX28sySFCT/oEO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 06, 2012 at 09:58:14AM +0100, Pawel Jakub Dawidek wrote: > On Mon, Feb 06, 2012 at 12:12:59AM +0200, Konstantin Belousov wrote: > > http://people.freebsd.org/~kib/misc/vm1.9.patch >=20 > --- a/sys/fs/nfsclient/nfs_clbio.c > +++ b/sys/fs/nfsclient/nfs_clbio.c > @@ -820,7 +820,10 @@ do_sync: > t_uio->uio_segflg =3D UIO_SYSSPACE; > t_uio->uio_rw =3D UIO_WRITE; > t_uio->uio_td =3D td; > - bcopy(uiop->uio_iov->iov_base, t_iov->iov_base, size); > + error =3D copyin(uiop->uio_iov->iov_base, > + t_iov->iov_base, size); > + if (error !=3D 0) > + goto err_free; >=20 > Good catch. It makes me wonder if uiop will always point at userland > buffer. What if we eg. AUDIT subsystem writing from within the kernel to > a file stored on NFS file system? Hm, I committed to the wrong branch. Anyway, speaking about this (unrelated) change, I thought that directio path cannot happen for UIO_SYSSPACE. But since both you and Rick think that handling UIO_SYSSPACE there makes sense, so be it. >=20 > + if (lock->rl_currdep =3D=3D TAILQ_FIRST(&lock->rl_waiters) && > + lock->rl_currdep !=3D NULL) > + lock->rl_currdep =3D TAILQ_NEXT(lock->rl_currdep, rl_q_link); > + for (entry =3D lock->rl_currdep; entry; >=20 > Minor style inconsistency. Two lines earlier you compare pointer with > NULL, which is nice, but here you treat pointer as boolean. Fixed. >=20 > +void > +rangelock_unlock(struct rangelock *lock, void *cookie) > +{ > + struct rl_q_entry *entry; > + > + MPASS(lock !=3D NULL && cookie !=3D NULL); > + > + entry =3D cookie; > + sleepq_lock(lock); > + rangelock_unlock_locked(lock, entry); > +} >=20 > You could drop using addtional 'entry' variable and just pass 'cookie' > directly. Although current code is self-explaining what 'cookie' > actually is, so I kinda like it as-is. Dropped entry. >=20 > +/* > + * Add the lock request to the queue of the pending requests for > + * rangelock. Sleeps until the request can be granted. >=20 > s/request/lock/ ? No, I think request is more precise there. >=20 > @@ -216,6 +218,7 @@ thread_fini(void *mem, int size) > =20 > td =3D (struct thread *)mem; > EVENTHANDLER_INVOKE(thread_fini, td); > + rlqentry_free(td->td_rlqe); >=20 > Not sure if it was intended or not, but td_rlqe can be NULL now, so it > might be worth checking that. It is not a big deal, as uma_zfree() can ha= ndle > NULL pointers though. Yes, it is intended. As you noted yourself, uma_zfree() handles NULL. For typical thread, td_rlqe is not NULL, so it is slightly better to not check for NULL. --T4pX28sySFCT/oEO Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk8vo0cACgkQC3+MBN1Mb4h1DQCfefzc3n6Z3mj0Ia2dp8hk4RMR 2YwAoOWHv8a6dXehFKAGpB7glFSIV2oI =JAkA -----END PGP SIGNATURE----- --T4pX28sySFCT/oEO--