pcmcia: release_class patch concern

pcmcia: release_class patch concern

[Dmitry Torokhov]

Hi Dominik,

I noticed that Linus committed the patch from you that introduces waiting
for completion in module's exit routine. I believe it is a big no-no as
something like this will wedge the kernel:

	rmmod <module> < /sys/path/to/devices/attribute

Have you considered using Greg's class_create()/class_destroy() or maybe
bumping up module's refrerence count when registering class devices so
rmmod would fail if there are users of this module?

-- 
Dmitry

Re: pcmcia: release_class patch concern

[Dominik Brodowski]

Hi Dmitry,

On Mon, Jun 27, 2005 at 11:56:49PM -0500, Dmitry Torokhov wrote:
> Hi Dominik,
> 
> I noticed that Linus committed the patch from you that introduces waiting
> for completion in module's exit routine. I believe it is a big no-no

Is it really? Any PCI driver which calls pci_unregister_driver() waits for
completion (-> driver_unregister() -> wait_for_completion(&drv->unloaded) ).


> as something like this will wedge the kernel:
> 
> 	rmmod <module> < /sys/path/to/devices/attribute

Why would anybody issue such a command? But it even wouldn't succeed, as
the module usage count would be >0 if there are attributes below
/sys/class/pcmcia_socket/

> Have you considered using Greg's class_create()/class_destroy()

Oh, don't get me started... they're for class_device_create() -- and we
don't register char devices here -- and this interface has some severe 
limitations and shortcomings:
http://marc.theaimsgroup.com/?l=linux-usb-devel&m=111193575527911&w=2
and follow the thread...

> or maybe
> bumping up module's refrerence count when registering class devices so
> rmmod would fail if there are users of this module?

In fact, this is done already, even though indirectly: socket drivers will
always increase the reference count as they use exports from pcmcia_core.ko,
and their call to pcmcia_unregister_socket in the device's ->remove()
function only succeeds if the class device is actually freed. So I could
have left the other wait_for_completion out, as it should never actually
_wait_. Nonethteless, I consider it to be a safeguard.

Thanks,
	Dominik

Re: pcmcia: release_class patch concern

[Dmitry Torokhov]

On Tuesday 28 June 2005 01:14, Dominik Brodowski wrote:
> Hi Dmitry,
> 
> On Mon, Jun 27, 2005 at 11:56:49PM -0500, Dmitry Torokhov wrote:
> > Hi Dominik,
> > 
> > I noticed that Linus committed the patch from you that introduces waiting
> > for completion in module's exit routine. I believe it is a big no-no
> 
> Is it really? Any PCI driver which calls pci_unregister_driver() waits for
> completion (-> driver_unregister() -> wait_for_completion(&drv->unloaded) ).
>

Driver objects don't linger around - teardown is straightforward and
attribute access protected with bumping up module's reference count.
So it usually works out pretty well. 

> 
> > as something like this will wedge the kernel:
> > 
> > 	rmmod <module> < /sys/path/to/devices/attribute
> 
> Why would anybody issue such a command?

This is just the simpliest method to illustrate the problem. I am sure
someone could come up with a more realistic example. I think Al Viro
mentioned it some time ago, but I can't find his post...

> But it even wouldn't succeed, as 
> the module usage count would be >0 if there are attributes below
> /sys/class/pcmcia_socket/
...
> So I could have left the other wait_for_completion out, as it should never
> actually _wait_. Nonethteless, I consider it to be a safeguard.

Since the completion will never be actually used I'd rather not have it
at all - I believe it sets bad example.
 
-- 
Dmitry