Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9a0af2fec80f1b46c3ad9d80c6424e168ca2e54e.camel@linux.ibm.com>
Subject: Re: [PATCH AUTOSEL 5.17 18/31] s390/pci: improve zpci_dev reference
 counting
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     Sasha Levin <sashal@kernel.org>, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
Cc:     Matthew Rosato <mjrosato@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        gerald.schaefer@linux.ibm.com, hca@linux.ibm.com,
        agordeev@linux.ibm.com, linux-s390@vger.kernel.org
Date:   Thu, 07 Apr 2022 10:27:37 +0200
In-Reply-To: <20220407011029.113321-18-sashal@kernel.org>
References: <20220407011029.113321-1-sashal@kernel.org>
         <20220407011029.113321-18-sashal@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2022-04-06 at 21:10 -0400, Sasha Levin wrote:
> From: Niklas Schnelle <schnelle@linux.ibm.com>
> 
> [ 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@linux.ibm.com>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---

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.