Re: [PATCH 1/2] device.h: pack struct dev_links_info

From: Greg Kroah-Hartman
Date: Thu Feb 28 2019 - 03:35:39 EST


On Wed, Feb 27, 2019 at 02:32:26PM +0100, Johan Hovold wrote:
> On Wed, Feb 27, 2019 at 01:06:45PM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Feb 27, 2019 at 11:59:51AM +0100, Johan Hovold wrote:
>
> > Yeah, that is a good point, normally we use packed to keep padding from
> > the middle of the structure from happening.
> >
> > I just don't like that 4 bytes sitting there doing nothing :)
>
> You could perhaps put them directly in struct device if the
> dev_links_info struct is just used a separator there and this really
> bothers you. :)

True :)

> > But, in thinking about this, there is no real reason that I can see that
> > this structure even is in struct device. It should be able to be in the
> > private "internal" structure.
> >
> > The patch below moves it out of struct device entirely. Overall there
> > is no memory savings, but it could give us the chance to only create
> > this structure if we really need it later on, as very few things use
> > links at this point in time.
> >
> > Rafael, there is one logic change below, the link structure is not
> > initialized until device_add() happens, instead of device_initialize().
> > Will that affect anything that you can think of? Does anyone do
> > anything with links before device_add() is called?
>
> I think device_add() may be too late.
>
> The earliest point in time when device links can be added is
> after :c:func:`device_add()` has been called for the supplier
> and :c:func:`device_initialize()` has been called for the
> consumer.

That is true today due to the way the code is set up, but it would be
good to figure out if anyone actually does call it this early.

thanks,

greg k-h