On Tue, Nov 25, 2008 at 04:40:19PM +0000, Andrew Doran wrote:
> On Tue, Nov 25, 2008 at 02:40:36AM +0000, Quentin Garnier wrote:
>
> > Module Name: src
> > Committed By: cube
> > Date: Tue Nov 25 02:40:36 UTC 2008
> >
> > Modified Files:
> > src/sys/net: bsd-comp.c if_ppp.c if_ppp.h ppp-comp.h ppp-deflate.c
> >
> > Log Message:
> > Rework the way PPP compmressors are handled and allow them to be
> > automatically loaded when needed.
>
> Thanks, however there are problems with this:
>
> - The code using module_hold() has race conditions. It would be better to
> reference count the compressors individually.
The two facts, while true, seem unrelated to me. I used module_hold
because, well, it was already available and that way I had a userland
interface to know the refcount from userland easily at no additional
cost for my tests.
Is there any other reason not to use module_{hold,rele}? I'm a bit
suprised that modstat lists a lot of modules with a refcount of 0 when
they're obviously in use (I haven't tried, but I'd expect modunload to
fail for them). What is it supposed to represent?
Another thing that I found odd while playing with that stuff is that
some module can be auto;atically unloaded but I'm not quite sure why.
The reason why it came up here is that when asked for bsdcomp, pppd will
try deflate instead if it is available, so it ends up getting both
modules loaded. However, the reference count on bsdcomp drops to 0
almost immediately as it replaced by deflate, and it does get unloaded.
But when the refcount on deflate drops to 0 later after the ppp session
is ended, it doesn't get unloaded.
[... other stuff I have to fix ...]
> - There is no explicit locking in pppioctl(). Calls made when manipluating
> compressors can sleep and lead to race conditions, as kernel_lock is
> dropped automatically when sleeping.
I'm not getting that one. I don't see what I should be protecting WRT
modules in pppioctl().
Thanks for reviewing that. To me it was more of an exercise to play
with modules, and I guess I failed the written exam, but there is little
documentation material for that class.
--
Quentin Garnier - cube%cubidou.net@localhost - cube%NetBSD.org@localhost
"See the look on my face from staying too long in one place
[...] every time the morning breaks I know I'm closer to falling"
KT Tunstall, Saving My Face, Drastic Fantastic, 2007.
Attachment:
pgpUOVJoz7xH_.pgp
Description: PGP signature