Date: Sun, 4 Mar 2007 15:11:36 +0100 From: Divacky Roman <xdivac02@stud.fit.vutbr.cz> To: hackers@freebsd.org Subject: investigation of Giant usage in kernel Message-ID: <20070304141136.GA4935@stud.fit.vutbr.cz>
next in thread | raw e-mail | index | archive | help
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 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 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? 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 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? 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. These are just my impressions on the code and I'd like to hear some comments if its correct and if its I'd like to see some fixes. thnx roman
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070304141136.GA4935>