Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Greg KH]

> From: Jeff Dike <jdike@addtoit.com>, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> Define a release method for the ubd and network driver so that sysfs doesn't
> complain when one is removed via:

What?  No.  The kernel is complaining for a reason, don't try to
out-smart it.

> 
> host $ uml_mconsole <umid> remove <dev>
> 
> Done by Jeff around January for ubd only, later lost, then restored in his tree
> - however I'm merging it now since there's no reason to leave this here.
> 
> We don't need to do any cleanup in the new added method, because when hot-unplug
> is done by uml_mconsole we already handle cleanup in mconsole infrastructure,
> i.e.  mc_device->remove (net_remove/ubd_remove), which is also the calling
> method.

Huh?  You have 2 different release functions for the same object?  And
how do you know which one is correct?  That does not sound right at all.

Please fix this correctly.

thanks,

greg k-h

Re: [uml-devel] Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Blaisorblade]

On Wednesday 18 January 2006 02:12, Greg KH wrote:
> > From: Jeff Dike <jdike@addtoit.com>, Paolo 'Blaisorblade' Giarrusso
> > <blaisorblade@yahoo.it>
> >
> > Define a release method for the ubd and network driver so that sysfs
> > doesn't complain when one is removed via:

> What?  No.  The kernel is complaining for a reason, don't try to
> out-smart it.

I'm not trying to ignore the warning.

> > host $ uml_mconsole <umid> remove <dev>

> > Done by Jeff around January for ubd only, later lost, then restored in
> > his tree - however I'm merging it now since there's no reason to leave
> > this here.

> > We don't need to do any cleanup in the new added method, because when
> > hot-unplug is done by uml_mconsole we already handle cleanup in mconsole
> > infrastructure, i.e.  mc_device->remove (net_remove/ubd_remove), which is
> > also the calling method.

> Huh?  You have 2 different release functions for the same object?

Not sure which ones you refer. net_remove and ubd_remove are for different 
devices; mc_device->remove and the sysfs release are in different layers and 
for different things.

> And 
> how do you know which one is correct?  That does not sound right at all.

No, but since we don't have plugs, we get configuration requests to remove 
devices.

And currently, since this existed in already 2.4, the handler of the 
configuration request(mc_device->remove) does all the work to shutdown the 
interface, and then calls into the kobject stuff (with 
platform_device_unregister()); and this complains for the missing ->remove.

If it's better, we may try to move things inside the sysfs ->remove event, 
with care because different stuff has been done.

However, there are a lot of things to look at here... is there anything in 
ubd_remove() smelling awful? Because we have a crash which is caused by us 
harming sysfs data structures, which triggers on ubd_remove().

Bug report:
http://marc.theaimsgroup.com/?l=user-mode-linux-devel&m=113537479514679&w=2

investigations:
http://marc.theaimsgroup.com/?l=user-mode-linux-devel&m=113538805406552&w=2

> Please fix this correctly.

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it

Re: [uml-devel] Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Greg KH]

On Wed, Jan 18, 2006 at 12:53:54PM +0100, Blaisorblade wrote:
> On Wednesday 18 January 2006 02:12, Greg KH wrote:
> > > From: Jeff Dike <jdike@addtoit.com>, Paolo 'Blaisorblade' Giarrusso
> > > <blaisorblade@yahoo.it>
> > >
> > > Define a release method for the ubd and network driver so that sysfs
> > > doesn't complain when one is removed via:
> 
> > What?  No.  The kernel is complaining for a reason, don't try to
> > out-smart it.
> 
> I'm not trying to ignore the warning.

By providing a release function that does nothing, all you are doing is
shutting the kernel up.  You are not doing anything to "fix" the real
problem at all.

> > > host $ uml_mconsole <umid> remove <dev>
> 
> > > Done by Jeff around January for ubd only, later lost, then restored in
> > > his tree - however I'm merging it now since there's no reason to leave
> > > this here.
> 
> > > We don't need to do any cleanup in the new added method, because when
> > > hot-unplug is done by uml_mconsole we already handle cleanup in mconsole
> > > infrastructure, i.e.  mc_device->remove (net_remove/ubd_remove), which is
> > > also the calling method.
> 
> > Huh?  You have 2 different release functions for the same object?
> 
> Not sure which ones you refer. net_remove and ubd_remove are for different 
> devices; mc_device->remove and the sysfs release are in different layers and 
> for different things.

Ok, but you are still adding 2 empty release functions.

Please do not do that, if you think you need this, your code logic is
wrong.

As for your specific bug report, sorry, I really don't know.

good luck,

greg k-h

Re: [uml-devel] Re: [PATCH 1/9] uml: avoid sysfs warning on hot-unplug

[Blaisorblade]

On Wednesday 18 January 2006 18:02, Greg KH wrote:
> On Wed, Jan 18, 2006 at 12:53:54PM +0100, Blaisorblade wrote:

> As for your specific bug report, sorry, I really don't know.

I know it's not something easy to find, but nor me nor Jeff master deeply the 
kobject API. And we're not short of work.

Is there anybody who while reviewing various drivers could review our ones on 
this aspect (the same way people like you use to review and fix bugs all over 
the tree)?

I know this may seem unkind, but it's a "specialization thing" - 
don't let an engineer heal somebody nor a doctor build a house, and don't 
request them doing such things - defer the thing to more competent people, if 
possible.

Alternatively, documentation is accepted. Is LDD v3 up-to-date enough? Guess 
not.

A question: looking at ide driver I saw that they don't use the 
platform_device API, because they're already in the block category.

Could we be causing trouble by using both block API (with gendisk stuff) and 
platform_device? Can platform_device

Alternatively, is drivers/input/serio/i8042.c a likely correct API example?
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

		
___________________________________ 
Yahoo! Messenger with Voice: chiama da PC a telefono a tariffe esclusive 
http://it.messenger.yahoo.com