Re: [PATCH] mce: fix warning messages about static struct mce_device

[Alan Stern]

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

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