lkml.org 
[lkml]   [2012]   [Jan]   [17]   [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 Tue, Jan 17, 2012 at 06:06:35PM +0530, Srivatsa S. Bhat wrote:
    > On 01/17/2012 04:10 AM, Greg KH wrote:
    >
    > > From: Greg Kroah-Hartman <gregkh@suse.de>
    > >
    > > When suspending, there was a large list of warnings going something like:
    > >
    > > Device 'machinecheck1' does not have a release() function, it is broken and must be fixed
    > >
    > > This patch turns the static mce_devices into dynamically allocated, and
    > > properly frees them when they are removed from the system. It solves
    > > the warning messages on my laptop here.
    > >
    >
    > ...
    >
    > > /* Per cpu device init. All of the cpus still share the same ctrl bank: */
    > > static __cpuinit int mce_device_create(unsigned int cpu)
    > > {
    > > - struct device *dev = &per_cpu(mce_device, cpu);
    > > + struct device *dev;
    > > int err;
    > > int i, j;
    > >
    > > if (!mce_available(&boot_cpu_data))
    > > return -EIO;
    > >
    > > - memset(dev, 0, sizeof(struct device));
    > > + dev = kzalloc(sizeof *dev, GFP_KERNEL);
    > > + if (!dev)
    > > + return -ENOMEM;
    > > dev->id = cpu;
    > > dev->bus = &mce_subsys;
    > > + dev->release = &mce_device_release;
    > >
    > > err = device_register(dev);
    > > if (err)
    > > @@ -2030,6 +2038,7 @@ static __cpuinit int mce_device_create(unsigned int cpu)
    > > goto error2;
    > > }
    > > cpumask_set_cpu(cpu, mce_device_initialized);
    > > + mce_device[cpu] = dev;
    > >
    > > return 0;
    > > error2:
    > > @@ -2046,7 +2055,7 @@ error:
    > >
    > > /* Make sure there are no machine checks on offlined CPUs. */
    > > diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
    > > index ba0b94a..786e76a 100644
    > > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
    > > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
    > > @@ -523,6 +523,7 @@ static __cpuinit int threshold_create_bank(unsigned int cpu, unsigned int bank)
    > > {
    > > int i, err = 0;
    > > struct threshold_bank *b = NULL;
    > > + struct device *dev = mce_device[cpu];
    > > char name[32];
    > >
    > > sprintf(name, "threshold_bank%i", bank);
    > > @@ -543,8 +544,7 @@ static __cpuinit int threshold_create_bank(unsigned int cpu, unsigned int bank)
    > > if (!b)
    > > goto out;
    > >
    > > - err = sysfs_create_link(&per_cpu(mce_device, cpu).kobj,
    > > - b->kobj, name);
    > > + err = sysfs_create_link(&dev->kobj, b->kobj, name);
    >
    >
    > I don't think dereferencing 'dev' like this is safe when booting up.
    > (See below for another such instance of dereferencing dev.)
    >
    > Both mcheck_init_device() and threshold_init_device() are device_initcalls.
    > And the latter depends on the former, because the former dynamically
    > allocates and fills the 'mce_device' array of pointers.
    >
    > So, what guarantees that this ordering is preserved? IOW, what ensures that
    > mcheck_init_device() is completed before running threshold_init_device()?
    >
    > Or am I missing something?

    You must be, as this is the way it is always works, and somehow your
    machine boots properly, with the symlinks setup :)

    thanks,

    greg k-h


    \
     
     \ /
      Last update: 2012-01-17 16:55    [W:3.836 / U:0.472 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site