Date: Mon, 6 Feb 2012 09:58:14 +0100 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: arch@freebsd.org Subject: Re: Prefaulting for i/o buffers Message-ID: <20120206085814.GB1324@garage.freebsd.pl> In-Reply-To: <20120205221259.GL3283@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>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Mon, Feb 06, 2012 at 12:12:59AM +0200, Konstantin Belousov wrote: > http://people.freebsd.org/~kib/misc/vm1.9.patch --- a/sys/fs/nfsclient/nfs_clbio.c +++ b/sys/fs/nfsclient/nfs_clbio.c @@ -820,7 +820,10 @@ do_sync: t_uio->uio_segflg = UIO_SYSSPACE; t_uio->uio_rw = UIO_WRITE; t_uio->uio_td = td; - bcopy(uiop->uio_iov->iov_base, t_iov->iov_base, size); + error = copyin(uiop->uio_iov->iov_base, + t_iov->iov_base, size); + if (error != 0) + goto err_free; 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? + if (lock->rl_currdep == TAILQ_FIRST(&lock->rl_waiters) && + lock->rl_currdep != NULL) + lock->rl_currdep = TAILQ_NEXT(lock->rl_currdep, rl_q_link); + for (entry = lock->rl_currdep; entry; Minor style inconsistency. Two lines earlier you compare pointer with NULL, which is nice, but here you treat pointer as boolean. +void +rangelock_unlock(struct rangelock *lock, void *cookie) +{ + struct rl_q_entry *entry; + + MPASS(lock != NULL && cookie != NULL); + + entry = cookie; + sleepq_lock(lock); + rangelock_unlock_locked(lock, entry); +} 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. +/* + * Add the lock request to the queue of the pending requests for + * rangelock. Sleeps until the request can be granted. s/request/lock/ ? @@ -216,6 +218,7 @@ thread_fini(void *mem, int size) td = (struct thread *)mem; EVENTHANDLER_INVOKE(thread_fini, td); + rlqentry_free(td->td_rlqe); 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 handle NULL pointers though. -- Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk8vliYACgkQForvXbEpPzTNGgCggyZ6rgH24Bq8z9gzYFDpf+gO PR8An3JeYbNqfilOEamCbLxTRA/D/Z3n =YHIV -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120206085814.GB1324>