Re: [PATCH v3 2/4] PCI: Allow driver-specific data in host bridge

[Arnd Bergmann <arnd@arndb.de>]

On Friday, August 19, 2016 5:55:06 PM CEST Lorenzo Pieralisi wrote:
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 93583b389058..ecf543014da3 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -521,16 +521,19 @@ static void pci_release_host_bridge_dev(struct device *dev)
> >       kfree(bridge);
> >  }
> >  
> > -static struct pci_host_bridge *pci_alloc_host_bridge(void)
> > +static struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
> >  {
> >       struct pci_host_bridge *bridge;
> >  
> > -     bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
> > +     bridge = kzalloc(sizeof(*bridge) + priv, GFP_KERNEL);
> >       if (!bridge)
> >               return NULL;
> >  
> >       INIT_LIST_HEAD(&bridge->windows);
> >  
> > +     if (priv)
> > +             bridge->private = &bridge[1];
> 
> How about making private a zero length array ?

Right, the member can actually be removed here if we want to, this
was just meant as a shorthand so we can avoid the cast or function
call in device drivers. I think either way makes sense.

Also, someone commented on a related issue in the previous version, and I
recommended to align it to ARCH_SLAB_MINALIGN. This might be done more
easily with the zero-length array using

struct pci_host_bridge {
	...

	unsigned long private[0] __attribute__((aligned(ARCH_SLAB_MINALIGN)));
};

Without this, we need a bit more computation, like
	
	bridge = kzalloc(ALIGN(sizeof(*bridge), ARCH_SLAB_MINALIGN) + priv, GFP_KERNEL);
	bridge->private = (void *)bridge + ALIGN(sizeof(*bridge);

or

static inline void *pci_bridge_private(struct pci_host_bridge *bridge)
{
	return (void *)bridge + ALIGN(sizeof(*bridge);
}
static inline struct pci_host_bridge *pci_bridge_from_private(void *priv)
{
	return priv - ALIGN(sizeof(*bridge);
}

The alignment is needed if the private structure contains any data that
we may want to transfer using DMA.

	Arnd