lkml.org 
[lkml]   [2012]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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



    \
     
     \ /
      Last update: 2012-01-18 18:57    [W:2.547 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site