On Sun, Mar 04, 2007 at 03:11:36PM +0100, Divacky Roman wrote: > hi > > I looked at where Giant is held in the kernel and I found these interesting > things: > > 1) in fs/fifofs/fifo_vnops.c we lock Giant when calling sorecieve()/sosend() > this is a bandaid for fixing a race that doesnt have to exist anymore. ie. > it needs some testing and can be remvoed
I have removed this on my test machines and will see if the problem recurs. It is probably not necessary. > 2) in geom/geom_dev.c we lock Giant for: > > 2a) calling make_dev() which seems unecessary because make_dev protects > itself with devmtx > 2b) setting a flag in cdev which I think is unecessary as well I removed this in CVS. > 3) in kern/kern_descrip.c we lock Giant for the whole life of flock() I dont > see > a need for this protection becuase > > 3a) access to fp is protected with FILE_LOCK()ing > 3b) otherwise it operates on local variables > 3c) it also calls VOP_* functions but those dont require Giant, right? I am not sure about this one. It might be tied in to 4). > 4) in kern/kern_lockf.c we lock Giant for the whole life of lf_advlock() I > dont > think this is necessary because > > 4a) it operates on local variables > 4b) it calls various lf_*lock() functions which seems to either protect > themselves or not needed protection at all There is no synchronization between e.g. the tailq manipulation from different threads, so locking is needed. I have replaced this with a global lockf_mtx in my p4 branch but a finer grained solution is clearly desirable. This will probably involve rewriting or restructuring the code though. > 5) in kern/kern_time.c we lock Giant to protect > > 5a) tc_setclock() call - I dont know if this is necessary or not > 5b) lease_updatetime call which is a function pointer that seems to be > only assigned at one place (line 119 in kern_time.c) to a NOP > function. > can this be removed? I guess you are protecting against two threads setting the clock at once. This does not seem to be a big deal, I don't imagine this will ever be in the critical path of anything. > 6) in vm/device_pager.c we lock Giant to (also) protect cdevsw->d_mmap but > the device mmap > is either MPSAFE or assigned to giant_mmap (a wrapper that locks GIant and > calls the original > d_mmap) so this seems unecessary. I'm not sure about this one either. Kris _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
