sysctl locking

Max Laier max at love2party.net
Mon Dec 13 00:12:30 PST 2004


On Monday 13 December 2004 08:14, Suleiman Souhlal wrote:
> Hello,
>
> The patch at http://people.freebsd.org/~ssouhlal/sysctl-locking.diff
> removes Giant from the sysctl subsystem.
> I tested it on i386 and powerpc, where it appears to work perfectly.
> However, I have not been able to test it on an SMP box, as I don't have
> access to any.
> So I would appreciate it if someone would test it, and report any
> problems.
> I will commit it in about week, unless, of course, there are
> objections/problems.

Wait a minute ... you can't just assert that all sysctl handler are MPSAFE. 
It's a good idea to introduce "real" locking for the sysctl-tree handling in 
order to be able to lose Giant at a later point, but I *strongly* suggest 
that you keep on grabing Giant before calling oid_handler() in sysctl_root(). 
It doesn't seem like you do so, right now. 

You have identified two places where Giant is explicitly asserted, but I am 
afraid that there are much more handlers that don't assert Giant but need it. 
Moreover, the "simple" handler might also write to memory that is implicitly 
protected by Giant and should not be modified without it.

As a transition step I suggest that we extend the API in the way the callout 
API works. So that you can ask for a Giant-free handler call by setting an 
MPSAFE flag.

On a side note, I am not sure if I like the string copy thing - while I 
understand the intention. Neither am I sure if it is a good idea to introduce 
"yet another sleep lock/reference count thingy"[tm] before sitting down and 
giving some attention to the existing sx(9) implementation. I haven't fully 
read/understand your ref-count there, hence I can not tell if sx(9) will 
really work - and I know (very well) that sx(9) isn't the optimal answer 
sometimes (most of the time). But I am suggesting that we give that a closer 
look before reinventing the wheel over and over again. Oh, and last but 
*definitely* least, your patch could use some style(9) facelifting. e.g. tab 
after #define.

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20041213/8586f753/attachment.bin


More information about the freebsd-current mailing list