Date:	Wed, 18 Jan 2012 23:24:33 +0530
From:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To:	"Luck, Tony" <tony.luck@...el.com>
CC:	Alan Stern <stern@...land.harvard.edu>, Greg KH <gregkh@...e.de>,
	Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Sergei Trofimovich <slyich@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kay Sievers <kay.sievers@...y.org>,
	Linux PM mailing list <linux-pm@...r.kernel.org>,
	Borislav Petkov <bp@...64.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"prasad@...ux.vnet.ibm.com" <prasad@...ux.vnet.ibm.com>,
	Ming Lei <tom.leiming@...il.com>,
	Djalal Harouni <tixxdz@...ndz.org>,
	Borislav Petkov <borislav.petkov@....com>,
	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>,
	Andi Kleen <ak@...ux.intel.com>,
	"gouders@...bocholt.fh-gelsenkirchen.de" 
	<gouders@...bocholt.fh-gelsenkirchen.de>,
	Marcos Souza <marcos.mage@...il.com>,
	"justinmattock@...il.com" <justinmattock@...il.com>,
	Jeff Chua <jeff.chua.linux@...il.com>
Subject: Re: [PATCH] mce: fix warning messages about static struct mce_device

On 01/18/2012 10:58 PM, Luck, Tony wrote:

> Greg said:
>> It was already fixed that way, but the problem is that you can not have
>> statically allocated 'struct device' objects in the system.
> 
> and then Alan said:
>> There's an additional requirement: Device structures may not be reused.  
>> Not even if the caller clears all the fields to 0 in between.  That was
>> the real bug in the original code -- and adding a dummy release routine
>> wouldn't fix it.
> 
> There seems to be some curious logic happening here which I don't understand
> at all. How can the code that deals with 'struct device' tell whether it was
> statically declared or dynamically allocated? Why would it care?
> 
> What happens if we play by the rules using a dynamic structure and do
> 
>  device_register() + device_create_file(dev)
>    ...
>  device_remove_file(dev) + device_unregister()
> 
> then later come back to re-add this and by pure random fluke kzalloc()
> gives us back the exact same block of memory that we used for dev before?
> 
> By Alan's logic we are screwed - we are re-using the same device structure
> (unless kfree() + kzalloc() does some magic pixie dust thing so that this
> same block of memory is now not the same device structure we had before, even
> though it has the same address).
> 
> In summary - I can totally buy the argument that you must start with a zeroed
> struct device (and that it is just fine that device_unregister() doesn't waste
> cpu cycles cleaning up the structure just in case someone will re-use it, because
> that isn't going to be the common case).
> 
> I just don't understand the magical difference between a static structure that
> has been memset() to all zero, and a dynamic block returned from kzalloc().
> 


I am in total agreement with what Tony said above. We have already seen that
my patch did a memset of the device structure and solved the suspend issue.
So, the suspend issue is no longer haunting us, which demonstrates that there
is really no difference between using a zeroed struct device vs using a
structure which is dynamically allocated using zalloc().

What Greg is trying to do with this patch is - get rid of the "Machinecheck
doesn't have release() function" warning in a proper way - something better
than having a dummy release function. Functionality-wise, that patch is not
fixing anything!

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists