Date: Thu, 14 Mar 2002 13:43:59 -0500 From: Jake Burkholder <jake@locore.ca> To: "Andrew R. Reiter" <arr@FreeBSD.org> Cc: freebsd-smp@FreeBSD.org Subject: Re: Patch to lock down modules Message-ID: <20020314134359.C52298@locore.ca> In-Reply-To: <Pine.NEB.3.96L.1020314130824.97600D-100000@fledge.watson.org>; from arr@FreeBSD.org on Thu, Mar 14, 2002 at 01:25:24PM -0500 References: <20020314130843.B52298@locore.ca> <Pine.NEB.3.96L.1020314130824.97600D-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Thu, Mar 14, 2002 at 01:25:24PM -0500, Andrew R. Reiter said words to the effect of; > On Thu, 14 Mar 2002, Jake Burkholder wrote: > > : > :Couple problems: > : > :I think that you missed a MOD_SLOCK before the > : newmod = module_lookupbyname(data->name); > :in module_register. > > Actually, iirc, module_register only got called from one spot, and in that > spot, keeping SLOCK held seemed better than dropping it and reaquiring a > SLOCK in module_register. If we'd like to keep this more consisten with > some of the other funcitons, then I can change this no problem. Hum. Well, I don't like its side effect of releasing the lock. I think that the place where its called in kern_linker.c should not do a lookup by name itself, but leave it to module_register, it will return an error anyway. That way it doesn't need to acquire the lock before hand, and can leave it up to module_register. > > : > :Also in that function the MOD_XLOCK isn't needed until just before the > :TAILQ_INSERT_TAIL. The increment of nextid needs something, I would > :probably move it to just before the TAILQ_INSERT, under the XLOCK. An > :atomic_add_and_return_old_value would be useful here. Also, I think > :that you should do another module_lookupbyname after acquiring the > :XLOCK and before the TAILQ_INSERT, because conceivably another module > :with the same name could show up when you release the SLOCK. > > Seems reasonable... > > :Can you explain the wierd logic that was added to linker_file_unload? > > Ugh, yes, this is kinda ugly. Essentially this is a result of hacking > this back and forth between where we should be holding SLOCK over. This > is exactly what I wanted to discuss this b/c prior to making this change, > I had a much more simple strategy here (but was dropped due to changes to > a better strategy for kern_module). We discussed before about possibly > having two sets of lookup functions -- internal and external -- perhaps > this is a better solution? Hmm, I'll have to look at it more closely. > > :Thanks, > :Jake > : > > -- > Andrew R. Reiter > arr@watson.org > arr@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020314134359.C52298>