Date: Sun, 29 Apr 2001 01:35:33 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: Jake Burkholder <jburkholder0829@home.com> Cc: smp@FreeBSD.ORG Subject: Re: that vm diff now makes it into single user mode. Message-ID: <20010429013533.D18676@fw.wintelcom.net> In-Reply-To: <20010429061303.6B821223A@k7.rchrd1.on.wave.home.com>; from jburkholder0829@home.com on Sun, Apr 29, 2001 at 02:13:03AM -0400 References: <bright@wintelcom.net> <20010429061303.6B821223A@k7.rchrd1.on.wave.home.com>
next in thread | previous in thread | raw e-mail | index | archive | help
* Jake Burkholder <jburkholder0829@home.com> [010428 23:10] wrote: > > * Alfred Perlstein <bright@wintelcom.net> [010427 04:26] wrote: > > > If you're booting diskless you get to mounting/using the md0 disk > > > and you panic because I haven't made ufs_readwrite safe. > > > > > > http://people.freebsd.org/~alfred/vm.diff > > > > I'm now building world on my p100 over NFS with the latest version > > of this patch. I'm pretty sure UFS isn't going to work all that > > well but the changes required to fix it should be minor. > > > > Review (of code and locking methods, but not style) and fixes would > > be appreciated. > > Ok. I've just looked at what you've changed by following the patch. > I started to point out what I thought were bugs, but I guess its > just stuff you haven't got to yet (sendfile). Yes, still making world over NFS here, p100 is slow. :) > i386/i386/vm_machdep.c: > > the mtx_trylock in vm_page_zero_idle is unnecessary, the lock is > already held. This whole thing needs to be non-blocking if its > going to be called from the idle loop, but I'm not sure that > that's still possible. Its currently commented out. Ok, should be fixed. I guess we now know where it could have been useful to be able to spin on a sleeplock, ie not worry about idle getting stuck on a runqueue/sleepqueue. > kern/sysv_shm.c: > > shm_deallocate_segment calls vm_object_deallocate and is called outside > of vm_mtx. Does vm_object_deallocate need vm_mtx? Noted, I'll take a look at that. Not fixed yet.. > vfs_bio.c: > > I don't see why vfs_vmio_release needs to unlock the vm_mtx, wakeup > doesn't reschedule and brelvp doesn't sleep. I'd rather it not > release the lock if it doesn't have to. That code can go several function levels deep, it was a minor optimization and I thought the comments would make it clear, however I've reverted it to not unlock and put the unlock in the caller. > > Should the second mtx_lock after vm_object_pip_wakeupn be an unlock? > Probably a typo. It is. > > vm_mmap.c: > > munmap will return with the vm_mtx held if the protection check fails. Fixed. > > vm_pageout.c: > > vm_daemon() needs an mtx_lock(vm_mtxp) at the bottom of the infinite loop. nod. > The following functions have a comment about needing the vm_mtx, but > don't have the corresponding assertions: > > MA_OWNED: vfs_page_set_valid, swp_pager_meta_build, swapout_procs > MA_NOTOWNED: brelse, vfs_unbusy_pages, vfs_busy_pages > > You might consider this style, but I'm not crazy about this kind of thing: > > gotlock = mtx_owned(vm_mtxp); > if (!gotlock) > mtx_lock(vm_mtxp); > ... > if (!gotlock) > mtx_unlock(vm_mtxp); These are for functions that can be called with or without the lock but if called with the lock must not release it, it looks like the code you have is for dealing with when code can be called without but may need to pick up Giant along the way. > I'd rather you just recurse on vm_mtx and make sure that the function > doesn't change the recursion level overall. I've been screwed more > then once by this same thing with the old got_mplock flag in > i386/i386/trap.c. Making the mutex recursive doesn't work well because of things like INTERLOCK and lockmgr. > About page faults not needing Giant, have you tried not acquiring > Giant in trap where the page faults come in? I've pushed Giant down > far enough in trap() for this to happe for i386 at least, I think Doug's > done the same for alpha. foo, i didn't see the Giant wrapper around trap_pfault, I thought I was already doing that. Thanks a ton for your input! -- -Alfred Perlstein - [alfred@freebsd.org] http://www.egr.unlv.edu/~slumos/on-netbsd.html To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010429013533.D18676>