On Wed, 2022-04-06 at 21:10 -0400, Sasha Levin wrote:
> From: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
>
> [ Upstream commit c122383d221dfa2f41cfe5e672540595de986fde ]
>
> Currently zpci_dev uses kref based reference counting but only accounts
> for one original reference plus one reference from an added pci_dev to
> its underlying zpci_dev. Counting just the original reference worked
> until the pci_dev reference was added in commit 2a671f77ee49 ("s390/pci:
> fix use after free of zpci_dev") because once a zpci_dev goes away, i.e.
> enters the reserved state, it would immediately get released. However
> with the pci_dev reference this is no longer the case and the zpci_dev
> may still appear in multiple availability events indicating that it was
> reserved. This was solved by detecting when the zpci_dev is already on
> its way out but still hanging around. This has however shown some light
> on how unusual our zpci_dev reference counting is.
>
> Improve upon this by modelling zpci_dev reference counting on pci_dev.
> Analogous to pci_get_slot() increment the reference count in
> get_zdev_by_fid(). Thus all users of get_zdev_by_fid() must drop the
> reference once they are done with the zpci_dev.
>
> Similar to pci_scan_single_device(), zpci_create_device() returns the
> device with an initial count of 1 and the device added to the zpci_list
> (analogous to the PCI bus' device_list). In turn users of
> zpci_create_device() must only drop the reference once the device is
> gone from the point of view of the zPCI subsystem, it might still be
> referenced by the common PCI subsystem though.
>
> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> Signed-off-by: Vasily Gorbik <gor@xxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
This isn't really a bug fix, as far as I'm aware the existing code
works correctly. It is just about making things more like PCI bus
reference counting and less weird. I also see some potential of the
state of things with just this commit added being confusing. That's why
there is a follow up commit 7dcfe50f58d2 ("s390/pci: rename
get_zdev_by_bus() to zdev_from_bus()") to make it more obvious when
zpci_zdev_put() is needed.
In short I'd propose to drop this patch from the stable queues.
