linux-kernel - Re: [PATCH] PCI/P2PDMA: Match interface changes to devm_memremap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181206204643.GC247703@google.com>
Date:   Thu, 6 Dec 2018 14:46:43 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>, linux-pci@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH] PCI/P2PDMA: Match interface changes to
 devm_memremap_pages()

On Fri, Nov 30, 2018 at 03:59:11PM -0700, Logan Gunthorpe wrote:
> "mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in the
> mm tree breaks p2pdma. The patch was written and reviewed before p2pdma
> was merged so the necessary changes were not done to the call site in
> that code.
> 
> Without this patch, all drivers will fail to register P2P resources
> because devm_memremap_pages() will return -EINVAL due to the 'kill'
> member of the pagemap structure not yet being set.
> 
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>

Applied with Dan's reviewed-by to pci/peer-to-peer for v4.21, thanks!

If the mm patch you mention gets merged for v4.20, let me know and I can
promote this to for-linus so v4.20 doesn't end up broken.

> ---
> 
> Ideally this patch should be squashed with the one mentioned above to
> avoid a bisect regression point.
> 
> drivers/pci/p2pdma.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index ae3c5b25dcc7..a2eb25271c96 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -82,10 +82,8 @@ static void pci_p2pdma_percpu_release(struct percpu_ref *ref)
>  	complete_all(&p2p->devmap_ref_done);
>  }
> 
> -static void pci_p2pdma_percpu_kill(void *data)
> +static void pci_p2pdma_percpu_kill(struct percpu_ref *ref)
>  {
> -	struct percpu_ref *ref = data;
> -
>  	/*
>  	 * pci_p2pdma_add_resource() may be called multiple times
>  	 * by a driver and may register the percpu_kill devm action multiple
> @@ -198,6 +196,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
>  	pgmap->type = MEMORY_DEVICE_PCI_P2PDMA;
>  	pgmap->pci_p2pdma_bus_offset = pci_bus_address(pdev, bar) -
>  		pci_resource_start(pdev, bar);
> +	pgmap->kill = pci_p2pdma_percpu_kill;
> 
>  	addr = devm_memremap_pages(&pdev->dev, pgmap);
>  	if (IS_ERR(addr)) {
> @@ -211,11 +210,6 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
>  	if (error)
>  		goto pgmap_free;
> 
> -	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_percpu_kill,
> -					  &pdev->p2pdma->devmap_ref);
> -	if (error)
> -		goto pgmap_free;
> -
>  	pci_info(pdev, "added peer-to-peer DMA memory %pR\n",
>  		 &pgmap->res);
> 
> --
> 2.19.0