Re: [PATCH] fixup loop blkdev, add module_get

[Rusty Russell]

In message <[e-mail]> you write:
> On Mon, Jan 13, 2003 at 11:55:47AM +1100, Rusty Russell wrote:
> > In message <[e-mail]> you write:
> > > Sometimes, we are absolutely certain that we have at least one module
> > > reference "locked open" for us.  Loop is an example of such a case:  the
> > > set-fd and clear-fd struct block_device_operations ioctls already have a
> > > module reference from simply the block device being opened.
> > > 
> > > Therefore, we can just unconditionally increment the module refcount.
> > > I added module_get to do this.
> > 
> > Hi Jeff,
> > 
> > 	We may yet want such a primitive, but I've been resisting it
> > for the moment.
> > 
> > 	Firstly, because it's a very specialized and rare case which
> > lends itself to being abused, and secondly because if I "rmmod --wait"
> > the module, then such operations which try to hold the module in place
> > *should* fail.  Not doing so is impolite, at least.
> 
> Eh...  You are trying to chase infinity with 'rmmod --wait'.

No, you are trying to remove something and you want to chase down and
kill the users, scripts, whatever.  It guarantees that no new users
will access the module.

> I disagree:
> 
> 1) we do not prevent root from shooting themselves in the foot,

I don't understand this point.

> 2) moreover we do not prevent them from doing something that may be
> perfectly reasonable,

Nor this one, which seems to bethe same.

> 3) and this kind of code just adds error handling for no reason, when
> _not_ handling the error keeps the code more clean.

No, the reason is simple: the admin has said they want the damn module
removed.  They've *told* you what they want.  Why do you want to
disobey them?  8)

> In general this is just caring way too much about an obscure corner
> case.  Is the increased complexity of error handling when we _know_ the
> refcnt is locked for worth it?

Is the increased complexity of another primitive for "you know you
have a refcount" worth it? 8)

If there were 10 of these cases, sure, a __try_module_get() makes
sense: IMHO this is one of those areas on which intelligent people can
disagree, I think.

> Note that Linus turned off the 'deprecated' warning because MOD.*COUNT
> users are just too frequent, still.

Note that I didn't put the damn thing in there 8)

Hope he turned them back into macros, so the __unsafe runtime warning
doesn't report "module.h".

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.