Re: sysctl locking

From: Max Laier <max_at_love2party.net>
Date: Mon, 13 Dec 2004 09:13:13 +0100
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

Received on Mon Dec 13 2004 - 07:12:29 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:24 UTC