From: Frederic Barrat <fbarrat@linux.ibm.com>
To: Sasha Levin <sashal@kernel.org>,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, Andrew Donnellan <ajd@linux.ibm.com>
Subject: Re: [PATCH AUTOSEL 5.5 096/542] powerpc/powernv/ioda: Fix ref count for devices with their own PE
Date: Mon, 17 Feb 2020 09:49:41 +0100 [thread overview]
Message-ID: <0867167a-73b8-0735-78ce-0d984f7a80b5@linux.ibm.com> (raw)
In-Reply-To: <20200214154854.6746-96-sashal@kernel.org>
Le 14/02/2020 à 16:41, Sasha Levin a écrit :
> From: Frederic Barrat <fbarrat@linux.ibm.com>
>
> [ Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 ]
Hi,
Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 doesn't really
need to go to stable (any of 4.19, 5.4 and 5.5). While it's probably
safe, the patch replaces a refcount leak by another one, which makes
sense as part of the full series merged in 5.6-rc1, but isn't terribly
useful standalone on the current stable branches.
Fred
> The pci_dn structure used to store a pointer to the struct pci_dev, so
> taking a reference on the device was required. However, the pci_dev
> pointer was later removed from the pci_dn structure, but the reference
> was kept for the npu device.
> See commit 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary
> pcidev from pci_dn").
>
> We don't need to take a reference on the device when assigning the PE
> as the struct pnv_ioda_pe is cleaned up at the same time as
> the (physical) device is released. Doing so prevents the device from
> being released, which is a problem for opencapi devices, since we want
> to be able to remove them through PCI hotplug.
>
> Now the ugly part: nvlink npu devices are not meant to be
> released. Because of the above, we've always leaked a reference and
> simply removing it now is dangerous and would likely require more
> work. There's currently no release device callback for nvlink devices
> for example. So to be safe, this patch leaks a reference on the npu
> device, but only for nvlink and not opencapi.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Link: https://lore.kernel.org/r/20191121134918.7155-2-fbarrat@linux.ibm.com
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 4374836b033b4..67b836f102402 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
> return NULL;
> }
>
> - /* NOTE: We get only one ref to the pci_dev for the pdn, not for the
> - * pointer in the PE data structure, both should be destroyed at the
> - * same time. However, this needs to be looked at more closely again
> - * once we actually start removing things (Hotplug, SR-IOV, ...)
> + /* NOTE: We don't get a reference for the pointer in the PE
> + * data structure, both the device and PE structures should be
> + * destroyed at the same time. However, removing nvlink
> + * devices will need some work.
> *
> * At some point we want to remove the PDN completely anyways
> */
> - pci_dev_get(dev);
> pdn->pe_number = pe->pe_number;
> pe->flags = PNV_IODA_PE_DEV;
> pe->pdev = dev;
> @@ -1084,7 +1083,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
> pnv_ioda_free_pe(pe);
> pdn->pe_number = IODA_INVALID_PE;
> pe->pdev = NULL;
> - pci_dev_put(dev);
> return NULL;
> }
>
> @@ -1205,6 +1203,14 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
> struct pci_controller *hose = pci_bus_to_host(npu_pdev->bus);
> struct pnv_phb *phb = hose->private_data;
>
> + /*
> + * Intentionally leak a reference on the npu device (for
> + * nvlink only; this is not an opencapi path) to make sure it
> + * never goes away, as it's been the case all along and some
> + * work is needed otherwise.
> + */
> + pci_dev_get(npu_pdev);
> +
> /*
> * Due to a hardware errata PE#0 on the NPU is reserved for
> * error handling. This means we only have three PEs remaining
> @@ -1228,7 +1234,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
> */
> dev_info(&npu_pdev->dev,
> "Associating to existing PE %x\n", pe_num);
> - pci_dev_get(npu_pdev);
> npu_pdn = pci_get_pdn(npu_pdev);
> rid = npu_pdev->bus->number << 8 | npu_pdn->devfn;
> npu_pdn->pe_number = pe_num;
>
next prev parent reply other threads:[~2020-02-17 8:51 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200214154854.6746-1-sashal@kernel.org>
2020-02-14 15:40 ` [PATCH AUTOSEL 5.5 012/542] soc: fsl: qe: change return type of cpm_muram_alloc() to s32 Sasha Levin
2020-02-14 15:40 ` [PATCH AUTOSEL 5.5 048/542] powerpc/powernv/iov: Ensure the pdn for VFs always contains a valid PE number Sasha Levin
2020-02-14 15:41 ` [PATCH AUTOSEL 5.5 096/542] powerpc/powernv/ioda: Fix ref count for devices with their own PE Sasha Levin
2020-02-17 8:49 ` Frederic Barrat [this message]
2020-02-20 16:12 ` Sasha Levin
2020-02-14 15:41 ` [PATCH AUTOSEL 5.5 097/542] powerpc/iov: Move VF pdev fixup into pcibios_fixup_iov() Sasha Levin
2020-02-14 15:41 ` [PATCH AUTOSEL 5.5 103/542] powerpc/papr_scm: Fix leaking 'bus_desc.provider_name' in some paths Sasha Levin
2020-02-14 15:41 ` [PATCH AUTOSEL 5.5 104/542] powerpc/pseries/vio: Fix iommu_table use-after-free refcount warning Sasha Levin
2020-02-14 15:42 ` [PATCH AUTOSEL 5.5 177/542] net/wan/fsl_ucc_hdlc: reject muram offsets above 64K Sasha Levin
2020-02-14 15:44 ` [PATCH AUTOSEL 5.5 266/542] net/wan/fsl_ucc_hdlc: remove set but not used variables 'ut_info' and 'ret' Sasha Levin
2020-02-14 15:44 ` [PATCH AUTOSEL 5.5 302/542] soc: fsl: qe: remove set but not used variable 'mm_gc' Sasha Levin
2020-02-14 15:46 ` [PATCH AUTOSEL 5.5 395/542] KVM: PPC: Remove set but not used variable 'ra', 'rs', 'rt' Sasha Levin
2020-02-14 15:46 ` [PATCH AUTOSEL 5.5 414/542] ide: remove set but not used variable 'hwif' Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 444/542] powerpc/ptdump: Fix W+X verification call in mark_rodata_ro() Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 445/542] powerpc/ptdump: Only enable PPC_CHECK_WX with STRICT_KERNEL_RWX Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 446/542] powerpc/sriov: Remove VF eeh_dev state when disabling SR-IOV Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 457/542] PCI/AER: Initialize aer_fifo Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 464/542] powerpc/pseries/lparcfg: Fix display of Maximum Memory Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 465/542] selftests/eeh: Bump EEH wait time to 60s Sasha Levin
2020-02-14 15:47 ` [PATCH AUTOSEL 5.5 483/542] powerpc/mm: Don't log user reads to 0xffffffff Sasha Levin
2020-02-14 15:48 ` [PATCH AUTOSEL 5.5 493/542] KVM: PPC: Book3S HV: Release lock on page-out failure path Sasha Levin
2020-02-14 15:48 ` [PATCH AUTOSEL 5.5 502/542] powerpc: Do not consider weak unresolved symbol relocations as bad Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0867167a-73b8-0735-78ce-0d984f7a80b5@linux.ibm.com \
--to=fbarrat@linux.ibm.com \
--cc=ajd@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).