On Tue, Sep 25, 2018 at 11:13:42AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2018-09-18 at 17:58 -0600, Keith Busch wrote:
> > This uses the managed device resource allocations for the service data
> > so that the aer driver doesn't need to manage it, further simplifying
> > this driver.
>
> Just be careful (it migh be ok, I haven't audited everything, but I got
> bitten by something like that in the past) that the devm stuff will get
> disposed of in two cases:
>
> - The owner device going away (so far so good)
>
> - The owner device's driver being unbound
>
> The latter is something not completely obvious, ie, even if the owner
> device still has held references, the successful completion of
> ->remove() on the driver will be followed by a cleanup of the managed
> stuff.
>
> As I said, it might be ok in the AER case, but you might want to at
> least keep the set_service_data(dev, NULL) to make sure you don't leave
> a stale pointer there.
Yes, these resource methods should be considered carefully. I think
we're okay here, and didn't want to set service data to NULL for a
couple reasons:
1. The service data and its device are released together, so the device
is already out of scope before it could hold a stale pointer.
2. It is possible the IRQ handler may be invoked after 'remove', but
before the managed irq is torn down. Leaving the service data set
while it is allocated removes a requirement to check for NULL on
each interrupt.
