Message-ID: <20210210214053.GA610964@bjorn-Precision-5520>
Date:   Wed, 10 Feb 2021 15:40:53 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        linux-samsung-soc@...r.kernel.org, Jan Kara <jack@...e.cz>,
        Kees Cook <keescook@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-pci@...r.kernel.org,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        linux-mm@...ck.org, Jason Gunthorpe <jgg@...pe.ca>,
        Jérôme Glisse <jglisse@...hat.com>,
        John Hubbard <jhubbard@...dia.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org
Subject: Re: [PATCH] PCI: Also set up legacy files only after sysfs init

On Fri, Feb 05, 2021 at 02:36:32PM +0100, Daniel Vetter wrote:
> We are already doing this for all the regular sysfs files on PCI
> devices, but not yet on the legacy io files on the PCI buses. Thus far
> no problem, but in the next patch I want to wire up iomem revoke
> support. That needs the vfs up and running already to make sure that
> iomem_get_mapping() works.
> 
> Wire it up exactly like the existing code in
> pci_create_sysfs_dev_files(). Note that pci_remove_legacy_files()
> doesn't need a check since the one for pci_bus->legacy_io is
> sufficient.
> 
> An alternative solution would be to implement a callback in sysfs to
> set up the address space from iomem_get_mapping() when userspace calls
> mmap(). This also works, but Greg didn't really like that just to work
> around an ordering issue when the kernel loads initially.
> 
> v2: Improve commit message (Bjorn)
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>

Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>

I wish we weren't extending a known-racy mechanism to do this, but at
least we're not *adding* a brand new race.

> Cc: Stephen Rothwell <sfr@...b.auug.org.au>
> Cc: Jason Gunthorpe <jgg@...pe.ca>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: John Hubbard <jhubbard@...dia.com>
> Cc: Jérôme Glisse <jglisse@...hat.com>
> Cc: Jan Kara <jack@...e.cz>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: linux-mm@...ck.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-samsung-soc@...r.kernel.org
> Cc: linux-media@...r.kernel.org
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: linux-pci@...r.kernel.org
> ---
>  drivers/pci/pci-sysfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index fb072f4b3176..0c45b4f7b214 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -927,6 +927,9 @@ void pci_create_legacy_files(struct pci_bus *b)
>  {
>  	int error;
>  
> +	if (!sysfs_initialized)
> +		return;
> +
>  	b->legacy_io = kcalloc(2, sizeof(struct bin_attribute),
>  			       GFP_ATOMIC);
>  	if (!b->legacy_io)
> @@ -1448,6 +1451,7 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
>  static int __init pci_sysfs_init(void)
>  {
>  	struct pci_dev *pdev = NULL;
> +	struct pci_bus *pbus = NULL;
>  	int retval;
>  
>  	sysfs_initialized = 1;
> @@ -1459,6 +1463,9 @@ static int __init pci_sysfs_init(void)
>  		}
>  	}
>  
> +	while ((pbus = pci_find_next_bus(pbus)))
> +		pci_create_legacy_files(pbus);
> +
>  	return 0;
>  }
>  late_initcall(pci_sysfs_init);
> -- 
> 2.30.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Powered by blists - more mailing lists