Re: sysctl locking

Hi,

On Dec 13, 2004, at 3:13 AM, Max Laier wrote:

> 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.

Right.

> 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.

The patch at http://people.freebsd.org/~ssouhlal/sysctl-sx-locking.diff 
fixes most of your concerns. It unconditionally acquires Giant just 
before calling the handler, but I'm hoping that we'll find another 
solution. It also uses sx(9), and requires the following patch to be 
applied: http://people.freebsd.org/~ssouhlal/sx_xlocked.diff .
The latter creates a new function, int sx_xlocked(struct sx *sx), that 
returns non-zero when the exclusive lock is held by the current thread, 
and zero otherwise.

Bye.
--
Suleiman Souhlal     | ssouhlal_at_vt.edu
The FreeBSD Project  | ssouhlal_at_FreeBSD.org