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

From: Rafael J. Wysocki
Date: Thu Feb 28 2019 - 18:43:11 EST


On 2/28/2019 9:35 AM, Greg Kroah-Hartman wrote:
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.

ISTR a use case where it was needed in the IOMMU subsystem, but I'm not sure if it has been used that way eventually.

The only way to really find out would be to audit all of the device_link_add() callers I'm afraid.

Cheers,

Rafael