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 Wed, 18 Jan 2012, Greg KH wrote:

> > > > Minor nit, i don't think we have any other such [CONFIG_NR_CPUS]
> > > > pattern in the kernel.
> > > >
> > > > This should be something like:
> > > >
> > > > DECLARE_PER_CPU(struct device *, mce_device);
> > >
> > > That is what we used to have, but with just a static struct
> > > device. [...]

Ingo's suggestion is fine. The difference is that instead of a static
array of struct device's, this is a static pre-cpu pointer to a dynamic
struct device (effectively, an array of pointers rather than an array
of structures).

> > Which was fine in itself for a per CPU data structure - wouldnt
> > the warning be fixed by memset()-ing before registering the
> > device or such, if device registry absolutely needs a pre-zeroed
> > buffer?
>
> It was already fixed that way, but the problem is that you can not have
> statically allocated 'struct device' objects in the system. That's what
> my add-on patch fixed, also resolving the syslog messages saying there
> was no release function for the device as well.
>
> > I still think there must be some bug/assumption lurking in the
> > device layer - do you require all device allocations to be one
> > via zalloc()? Seems like a weird and unrobust requirement.
>
> Yes, that's always been the requirement.

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.

> > Amongst other things we use PER_CPU to have an array of just 2
> > elements on a dual core system, even if it boots a
> > CONFIG_NR_CPUS=512 distro kernel. That saves RAM, and with
> > higher CONFIG_NR_CPUS values it adds up quickly.
> >
> > > > Or the pointer should be attached to the CPU info structure.
> > >
> > > Ok, I have no objection to that, do you want me to make a
> > > patch doing that, now that this is already in Linus's tree?
> >
> > Would be nice if you could do that or some other equivalent
> > solution, i'd really not like to see the [CONFIG_NR_CPUS]
> > pattern to spread in the kernel, we spent a lot of time getting
> > rid of such uses ;-)
>
> Ok, I'll work on resolving this.

A static per-cpu pointer to struct device should work perfectly.

Alan Stern



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