Date: Thu, 16 Aug 2001 11:45:40 +0200 From: Andrzej Bialecki <abial@webgiro.com> To: Peter Pentchev <roam@ringlet.net> Cc: arch@FreeBSD.org, audit@FreeBSD.org Subject: Re: sysctl_register_oid() breakage at unload [PATCH] Message-ID: <3B7B9643.9CA874D9@webgiro.com> References: <20010811233452.A510@ringworld.oblivion.bg>
next in thread | previous in thread | raw e-mail | index | archive | help
Peter Pentchev wrote: > > Hi, > > Well, it seems that I broke things with the panic at attempts to > register oid's higher than the first dynamic oid. Specifically, > this broke the case of unregistering sysctl's, esp. at module unload. > The algorithm described in the sysctl_ctx_free(9) manpage is indeed > so very weird (I won't go so far as calling it 'stupid', because > I cannot really suggest any way to improve it right now). I hear you :-) Actually, I agree with your statement - the current algorithm is a brute force approach. The "proper" way to do it would be to keep a ref count - however, at the time I implemented this I ran out of time. Feel free to finish it. Also, IMHO the border between static and dynamic sysctls should be a much higher number (say, 32767). > So, if a sysctl context is freed, most of the time the first pass > of freeing will fail, and sysctl_ctx_free() will attempt to reregister > the sysctls with the same oid's; this, of course, causes a panic, > because sysctl_register_oid() does not like so high a "static" oid :( > > I just noticed that on my -stable laptop, when I tried to MFC > the patch - my sound driver is only available as a module, and > the kernel panicked at shutdown after attempting to unload it. > For various reasons I cannot run -current on this laptop (not least > because this is the machine I'm using for developing an application > that is supposed to run under -stable), and my -current box did not > really have any need for loadable modules, so that's how this slipped > in unnoticed :( > > So here's a proposed fix: add a "this is actually a re-registering, > stay cool" flag to sysctl_register_oid(), and update all the calls > to it that I could find under src/sys. This flag needs only be set > in sysctl_ctx_free(), all the other callers put a 0. ...having in mind that this is just a bandaid around a suboptimal design. Well, come to that, I'm not sure if we should further uglify the code with this patch... It really should use ref counts instead. Depending on my daytime job, I may try to fix it - otherwise feel free to do it yourself. -- Andrzej // ---------------------------------------------------------------- // Andrzej Bialecki <abial@webgiro.com>, Chief System Architect // WebGiro AB, Sweden (http://www.webgiro.com) // ---------------------------------------------------------------- // <abial@freebsd.org> FreeBSD developer (http://www.freebsd.org) To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3B7B9643.9CA874D9>