Date: Mon, 13 Dec 2004 09:13:13 +0100 From: Max Laier <max@love2party.net> To: freebsd-current@freebsd.org Cc: Suleiman Souhlal <ssouhlal@freebsd.org> Subject: Re: sysctl locking Message-ID: <200412130913.20215.max@love2party.net> In-Reply-To: <94AE3F5A-4CD6-11D9-8BD6-000A95C4D7BC@FreeBSD.org> References: <94AE3F5A-4CD6-11D9-8BD6-000A95C4D7BC@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1589865.srFL6FdiD5 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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.= =20 It's a good idea to introduce "real" locking for the sysctl-tree handling i= n=20 order to be able to lose Giant at a later point, but I *strongly* suggest=20 that you keep on grabing Giant before calling oid_handler() in sysctl_root(= ).=20 It doesn't seem like you do so, right now.=20 You have identified two places where Giant is explicitly asserted, but I am= =20 afraid that there are much more handlers that don't assert Giant but need i= t.=20 Moreover, the "simple" handler might also write to memory that is implicitl= y=20 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 callou= t=20 API works. So that you can ask for a Giant-free handler call by setting an= =20 MPSAFE flag. On a side note, I am not sure if I like the string copy thing - while I=20 understand the intention. Neither am I sure if it is a good idea to introdu= ce=20 "yet another sleep lock/reference count thingy"[tm] before sitting down and= =20 giving some attention to the existing sx(9) implementation. I haven't fully= =20 read/understand your ref-count there, hence I can not tell if sx(9) will=20 really work - and I know (very well) that sx(9) isn't the optimal answer=20 sometimes (most of the time). But I am suggesting that we give that a close= r=20 look before reinventing the wheel over and over again. Oh, and last but=20 *definitely* least, your patch could use some style(9) facelifting. e.g. ta= b=20 after #define. =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --nextPart1589865.srFL6FdiD5 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQBBvU8gXyyEoT62BG0RAmyRAJ45zhEJXRM//CzYG7XdS+Edm6hU4gCeIRGh H/762Crh/wnujk4tz1fJXf8= =DojW -----END PGP SIGNATURE----- --nextPart1589865.srFL6FdiD5--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200412130913.20215.max>