* [PATCH v3 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback
2021-07-29 23:32 [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Krzysztof Wilczyński
@ 2021-07-29 23:32 ` Krzysztof Wilczyński
2021-07-30 19:44 ` Bjorn Helgaas
2021-07-29 23:32 ` [PATCH v3 2/2] sysfs: Rename struct bin_attribute member to f_mapping Krzysztof Wilczyński
2021-07-30 5:06 ` [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Greg Kroah-Hartman
2 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2021-07-29 23:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dan Williams, Greg Kroah-Hartman, Daniel Vetter, Kees Cook,
Pali Rohár, Oliver O'Halloran, linux-pci
Defer invocation of the iomem_get_mapping() to the sysfs open callback
so that it can be executed as needed when the binary sysfs object has
been accessed.
To do that, convert the "mapping" member of the struct bin_attribute
from a pointer to the struct address_space into a function pointer with
a signature that requires the same return type, and then updates the
sysfs_kf_bin_open() to invoke provided function should the function
pointer be valid.
Also, convert every invocation of iomem_get_mapping() into a function
pointer assignment, therefore allowing for the iomem_get_mapping()
invocation to be deferred to when the sysfs open callback runs.
Thus, this change removes the need for the fs_initcalls to complete
before any other sub-system that uses the iomem_get_mapping() would be
able to invoke it safely without leading to a failure and an Oops
related to an invalid iomem_get_mapping() access.
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/pci/pci-sysfs.c | 6 +++---
fs/sysfs/file.c | 2 +-
include/linux/sysfs.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5d63df7c1820..76e5545d0e73 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -965,7 +965,7 @@ void pci_create_legacy_files(struct pci_bus *b)
b->legacy_io->read = pci_read_legacy_io;
b->legacy_io->write = pci_write_legacy_io;
b->legacy_io->mmap = pci_mmap_legacy_io;
- b->legacy_io->mapping = iomem_get_mapping();
+ b->legacy_io->mapping = iomem_get_mapping;
pci_adjust_legacy_attr(b, pci_mmap_io);
error = device_create_bin_file(&b->dev, b->legacy_io);
if (error)
@@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
b->legacy_mem->size = 1024*1024;
b->legacy_mem->attr.mode = 0600;
b->legacy_mem->mmap = pci_mmap_legacy_mem;
- b->legacy_io->mapping = iomem_get_mapping();
+ b->legacy_io->mapping = iomem_get_mapping;
pci_adjust_legacy_attr(b, pci_mmap_mem);
error = device_create_bin_file(&b->dev, b->legacy_mem);
if (error)
@@ -1195,7 +1195,7 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
}
}
if (res_attr->mmap)
- res_attr->mapping = iomem_get_mapping();
+ res_attr->mapping = iomem_get_mapping;
res_attr->attr.name = res_attr_name;
res_attr->attr.mode = 0600;
res_attr->size = pci_resource_len(pdev, num);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 9aefa7779b29..a3ee4c32a264 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -175,7 +175,7 @@ static int sysfs_kf_bin_open(struct kernfs_open_file *of)
struct bin_attribute *battr = of->kn->priv;
if (battr->mapping)
- of->file->f_mapping = battr->mapping;
+ of->file->f_mapping = battr->mapping();
return 0;
}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index a12556a4b93a..d5bcc897583c 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -176,7 +176,7 @@ struct bin_attribute {
struct attribute attr;
size_t size;
void *private;
- struct address_space *mapping;
+ struct address_space *(*mapping)(void);
ssize_t (*read)(struct file *, struct kobject *, struct bin_attribute *,
char *, loff_t, size_t);
ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback
2021-07-29 23:32 ` [PATCH v3 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback Krzysztof Wilczyński
@ 2021-07-30 19:44 ` Bjorn Helgaas
0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2021-07-30 19:44 UTC (permalink / raw)
To: Krzysztof Wilczyński
Cc: Bjorn Helgaas, Dan Williams, Greg Kroah-Hartman, Daniel Vetter,
Kees Cook, Pali Rohár, Oliver O'Halloran, linux-pci
On Thu, Jul 29, 2021 at 11:32:34PM +0000, Krzysztof Wilczyński wrote:
> Defer invocation of the iomem_get_mapping() to the sysfs open callback
> so that it can be executed as needed when the binary sysfs object has
> been accessed.
>
> To do that, convert the "mapping" member of the struct bin_attribute
> from a pointer to the struct address_space into a function pointer with
> a signature that requires the same return type, and then updates the
> sysfs_kf_bin_open() to invoke provided function should the function
> pointer be valid.
>
> Also, convert every invocation of iomem_get_mapping() into a function
> pointer assignment, therefore allowing for the iomem_get_mapping()
> invocation to be deferred to when the sysfs open callback runs.
>
> Thus, this change removes the need for the fs_initcalls to complete
> before any other sub-system that uses the iomem_get_mapping() would be
> able to invoke it safely without leading to a failure and an Oops
> related to an invalid iomem_get_mapping() access.
>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/pci-sysfs.c | 6 +++---
> fs/sysfs/file.c | 2 +-
> include/linux/sysfs.h | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 5d63df7c1820..76e5545d0e73 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -965,7 +965,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_io->read = pci_read_legacy_io;
> b->legacy_io->write = pci_write_legacy_io;
> b->legacy_io->mmap = pci_mmap_legacy_io;
> - b->legacy_io->mapping = iomem_get_mapping();
> + b->legacy_io->mapping = iomem_get_mapping;
> pci_adjust_legacy_attr(b, pci_mmap_io);
> error = device_create_bin_file(&b->dev, b->legacy_io);
> if (error)
> @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_mem->size = 1024*1024;
> b->legacy_mem->attr.mode = 0600;
> b->legacy_mem->mmap = pci_mmap_legacy_mem;
> - b->legacy_io->mapping = iomem_get_mapping();
> + b->legacy_io->mapping = iomem_get_mapping;
> pci_adjust_legacy_attr(b, pci_mmap_mem);
> error = device_create_bin_file(&b->dev, b->legacy_mem);
> if (error)
> @@ -1195,7 +1195,7 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
> }
> }
> if (res_attr->mmap)
> - res_attr->mapping = iomem_get_mapping();
> + res_attr->mapping = iomem_get_mapping;
> res_attr->attr.name = res_attr_name;
> res_attr->attr.mode = 0600;
> res_attr->size = pci_resource_len(pdev, num);
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index 9aefa7779b29..a3ee4c32a264 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -175,7 +175,7 @@ static int sysfs_kf_bin_open(struct kernfs_open_file *of)
> struct bin_attribute *battr = of->kn->priv;
>
> if (battr->mapping)
> - of->file->f_mapping = battr->mapping;
> + of->file->f_mapping = battr->mapping();
>
> return 0;
> }
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index a12556a4b93a..d5bcc897583c 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -176,7 +176,7 @@ struct bin_attribute {
> struct attribute attr;
> size_t size;
> void *private;
> - struct address_space *mapping;
> + struct address_space *(*mapping)(void);
> ssize_t (*read)(struct file *, struct kobject *, struct bin_attribute *,
> char *, loff_t, size_t);
> ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] sysfs: Rename struct bin_attribute member to f_mapping
2021-07-29 23:32 [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Krzysztof Wilczyński
2021-07-29 23:32 ` [PATCH v3 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback Krzysztof Wilczyński
@ 2021-07-29 23:32 ` Krzysztof Wilczyński
2021-07-30 19:45 ` Bjorn Helgaas
2021-07-30 5:06 ` [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Greg Kroah-Hartman
2 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2021-07-29 23:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dan Williams, Greg Kroah-Hartman, Daniel Vetter, Kees Cook,
Pali Rohár, Oliver O'Halloran, linux-pci
There are two users of iomem_get_mapping(), the struct file and struct
bin_attribute. The former has a member called "f_mapping" and the
latter has a member called "mapping", and both are poniters to struct
address_space.
Rename struct bin_attribute member to "f_mapping" to keep both meaning
and the usage consistent with other users of iomem_get_mapping().
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
drivers/pci/pci-sysfs.c | 6 +++---
fs/sysfs/file.c | 4 ++--
include/linux/sysfs.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 76e5545d0e73..f65382915f01 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -965,7 +965,7 @@ void pci_create_legacy_files(struct pci_bus *b)
b->legacy_io->read = pci_read_legacy_io;
b->legacy_io->write = pci_write_legacy_io;
b->legacy_io->mmap = pci_mmap_legacy_io;
- b->legacy_io->mapping = iomem_get_mapping;
+ b->legacy_io->f_mapping = iomem_get_mapping;
pci_adjust_legacy_attr(b, pci_mmap_io);
error = device_create_bin_file(&b->dev, b->legacy_io);
if (error)
@@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
b->legacy_mem->size = 1024*1024;
b->legacy_mem->attr.mode = 0600;
b->legacy_mem->mmap = pci_mmap_legacy_mem;
- b->legacy_io->mapping = iomem_get_mapping;
+ b->legacy_io->f_mapping = iomem_get_mapping;
pci_adjust_legacy_attr(b, pci_mmap_mem);
error = device_create_bin_file(&b->dev, b->legacy_mem);
if (error)
@@ -1195,7 +1195,7 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
}
}
if (res_attr->mmap)
- res_attr->mapping = iomem_get_mapping;
+ res_attr->f_mapping = iomem_get_mapping;
res_attr->attr.name = res_attr_name;
res_attr->attr.mode = 0600;
res_attr->size = pci_resource_len(pdev, num);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a3ee4c32a264..d019d6ac6ad0 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -174,8 +174,8 @@ static int sysfs_kf_bin_open(struct kernfs_open_file *of)
{
struct bin_attribute *battr = of->kn->priv;
- if (battr->mapping)
- of->file->f_mapping = battr->mapping();
+ if (battr->f_mapping)
+ of->file->f_mapping = battr->f_mapping();
return 0;
}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index d5bcc897583c..e3f1e8ac1f85 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -176,7 +176,7 @@ struct bin_attribute {
struct attribute attr;
size_t size;
void *private;
- struct address_space *(*mapping)(void);
+ struct address_space *(*f_mapping)(void);
ssize_t (*read)(struct file *, struct kobject *, struct bin_attribute *,
char *, loff_t, size_t);
ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 2/2] sysfs: Rename struct bin_attribute member to f_mapping
2021-07-29 23:32 ` [PATCH v3 2/2] sysfs: Rename struct bin_attribute member to f_mapping Krzysztof Wilczyński
@ 2021-07-30 19:45 ` Bjorn Helgaas
0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2021-07-30 19:45 UTC (permalink / raw)
To: Krzysztof Wilczyński
Cc: Bjorn Helgaas, Dan Williams, Greg Kroah-Hartman, Daniel Vetter,
Kees Cook, Pali Rohár, Oliver O'Halloran, linux-pci
On Thu, Jul 29, 2021 at 11:32:35PM +0000, Krzysztof Wilczyński wrote:
> There are two users of iomem_get_mapping(), the struct file and struct
> bin_attribute. The former has a member called "f_mapping" and the
> latter has a member called "mapping", and both are poniters to struct
> address_space.
Nit: s/poniters/pointers/
> Rename struct bin_attribute member to "f_mapping" to keep both meaning
> and the usage consistent with other users of iomem_get_mapping().
>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/pci-sysfs.c | 6 +++---
> fs/sysfs/file.c | 4 ++--
> include/linux/sysfs.h | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 76e5545d0e73..f65382915f01 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -965,7 +965,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_io->read = pci_read_legacy_io;
> b->legacy_io->write = pci_write_legacy_io;
> b->legacy_io->mmap = pci_mmap_legacy_io;
> - b->legacy_io->mapping = iomem_get_mapping;
> + b->legacy_io->f_mapping = iomem_get_mapping;
> pci_adjust_legacy_attr(b, pci_mmap_io);
> error = device_create_bin_file(&b->dev, b->legacy_io);
> if (error)
> @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> b->legacy_mem->size = 1024*1024;
> b->legacy_mem->attr.mode = 0600;
> b->legacy_mem->mmap = pci_mmap_legacy_mem;
> - b->legacy_io->mapping = iomem_get_mapping;
> + b->legacy_io->f_mapping = iomem_get_mapping;
> pci_adjust_legacy_attr(b, pci_mmap_mem);
> error = device_create_bin_file(&b->dev, b->legacy_mem);
> if (error)
> @@ -1195,7 +1195,7 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
> }
> }
> if (res_attr->mmap)
> - res_attr->mapping = iomem_get_mapping;
> + res_attr->f_mapping = iomem_get_mapping;
> res_attr->attr.name = res_attr_name;
> res_attr->attr.mode = 0600;
> res_attr->size = pci_resource_len(pdev, num);
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index a3ee4c32a264..d019d6ac6ad0 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -174,8 +174,8 @@ static int sysfs_kf_bin_open(struct kernfs_open_file *of)
> {
> struct bin_attribute *battr = of->kn->priv;
>
> - if (battr->mapping)
> - of->file->f_mapping = battr->mapping();
> + if (battr->f_mapping)
> + of->file->f_mapping = battr->f_mapping();
>
> return 0;
> }
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index d5bcc897583c..e3f1e8ac1f85 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -176,7 +176,7 @@ struct bin_attribute {
> struct attribute attr;
> size_t size;
> void *private;
> - struct address_space *(*mapping)(void);
> + struct address_space *(*f_mapping)(void);
> ssize_t (*read)(struct file *, struct kobject *, struct bin_attribute *,
> char *, loff_t, size_t);
> ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping()
2021-07-29 23:32 [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Krzysztof Wilczyński
2021-07-29 23:32 ` [PATCH v3 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback Krzysztof Wilczyński
2021-07-29 23:32 ` [PATCH v3 2/2] sysfs: Rename struct bin_attribute member to f_mapping Krzysztof Wilczyński
@ 2021-07-30 5:06 ` Greg Kroah-Hartman
2021-07-30 19:50 ` Bjorn Helgaas
2 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-30 5:06 UTC (permalink / raw)
To: Krzysztof Wilczyński
Cc: Bjorn Helgaas, Dan Williams, Daniel Vetter, Kees Cook,
Pali Rohár, Oliver O'Halloran, linux-pci
On Thu, Jul 29, 2021 at 11:32:33PM +0000, Krzysztof Wilczyński wrote:
> Hello,
>
> At the moment, the dependency on iomem_get_mapping() that is currently
> used in the pci_create_resource_files() and pci_create_legacy_files()
> stops us from completely retiring the late_initcall() that is used to
> invoke pci_sysfs_init() when creating sysfs object for PCI devices.
>
> This dependency on iomem_get_mapping() stops us from retiring the
> late_initcall at the moment as when we convert dynamically added sysfs
> objects, that are primarily added in the pci_create_resource_files() and
> pci_create_legacy_files(), as these attributes are added before the VFS
> completes its initialisation, and since most of the PCI devices are
> typically enumerated in subsys_initcall this leads to a failure and an
> Oops related to iomem_get_mapping() access.
>
> See relevant conversations:
> https://lore.kernel.org/linux-pci/20210204165831.2703772-1-daniel.vetter@ffwll.ch/
> https://lore.kernel.org/linux-pci/20210313215747.GA2394467@bjorn-Precision-5520/
>
> After some deliberation about the problem at hand, Dan Williams
> suggested a solution to the problem, as per:
> https://lore.kernel.org/linux-pci/CAPcyv4i0y_4cMGEpNVShLUyUk3nyWH203Ry3S87BqnDJE0Rmxg@mail.gmail.com/
>
> The idea is to defer execution of the iomem_get_mapping() to only when
> the sysfs open callback is run, and thus removing the reliance of
> fs_initcalls to complete before any other sub-system that uses the
> iomem_get_mapping().
>
> Currently, the PCI sub-system will benefit the most from this change
> allowing for it to complete the transition from dynamically created to
> static sysfs objects.
>
> This series aims to take Dan Williams' idea through the finish line.
>
> Related to:
> https://lore.kernel.org/linux-pci/20210527205845.GA1421476@bjorn-Precision-5520/
> https://lore.kernel.org/linux-pci/20210507102706.7658-1-danijel.slivka@amd.com/
> https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/
>
> Krzysztof
>
No objection from me on these, Bjorn, mind if I take them through my
driver core tree?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping()
2021-07-30 5:06 ` [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping() Greg Kroah-Hartman
@ 2021-07-30 19:50 ` Bjorn Helgaas
2021-08-05 12:43 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2021-07-30 19:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Krzysztof Wilczyński, Bjorn Helgaas, Dan Williams,
Daniel Vetter, Kees Cook, Pali Rohár, Oliver O'Halloran,
linux-pci
On Fri, Jul 30, 2021 at 07:06:26AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 29, 2021 at 11:32:33PM +0000, Krzysztof Wilczyński wrote:
> > Hello,
> >
> > At the moment, the dependency on iomem_get_mapping() that is currently
> > used in the pci_create_resource_files() and pci_create_legacy_files()
> > stops us from completely retiring the late_initcall() that is used to
> > invoke pci_sysfs_init() when creating sysfs object for PCI devices.
> >
> > This dependency on iomem_get_mapping() stops us from retiring the
> > late_initcall at the moment as when we convert dynamically added sysfs
> > objects, that are primarily added in the pci_create_resource_files() and
> > pci_create_legacy_files(), as these attributes are added before the VFS
> > completes its initialisation, and since most of the PCI devices are
> > typically enumerated in subsys_initcall this leads to a failure and an
> > Oops related to iomem_get_mapping() access.
> >
> > See relevant conversations:
> > https://lore.kernel.org/linux-pci/20210204165831.2703772-1-daniel.vetter@ffwll.ch/
> > https://lore.kernel.org/linux-pci/20210313215747.GA2394467@bjorn-Precision-5520/
> >
> > After some deliberation about the problem at hand, Dan Williams
> > suggested a solution to the problem, as per:
> > https://lore.kernel.org/linux-pci/CAPcyv4i0y_4cMGEpNVShLUyUk3nyWH203Ry3S87BqnDJE0Rmxg@mail.gmail.com/
> >
> > The idea is to defer execution of the iomem_get_mapping() to only when
> > the sysfs open callback is run, and thus removing the reliance of
> > fs_initcalls to complete before any other sub-system that uses the
> > iomem_get_mapping().
> >
> > Currently, the PCI sub-system will benefit the most from this change
> > allowing for it to complete the transition from dynamically created to
> > static sysfs objects.
> >
> > This series aims to take Dan Williams' idea through the finish line.
> >
> > Related to:
> > https://lore.kernel.org/linux-pci/20210527205845.GA1421476@bjorn-Precision-5520/
> > https://lore.kernel.org/linux-pci/20210507102706.7658-1-danijel.slivka@amd.com/
> > https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/
> >
> > Krzysztof
> >
>
> No objection from me on these, Bjorn, mind if I take them through my
> driver core tree?
That'd be great!
Is your tree immutable? I'm hoping we can take advantage of this to
get rid of pci_sysfs_init(), which will fix a race that people seem to
be hitting frequently now [1]. I think Krzysztof is getting pretty
close.
[1] https://lore.kernel.org/r/m3eebg9puj.fsf@t19.piap.pl
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping()
2021-07-30 19:50 ` Bjorn Helgaas
@ 2021-08-05 12:43 ` Greg Kroah-Hartman
2021-08-06 11:05 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-05 12:43 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Krzysztof Wilczyński, Bjorn Helgaas, Dan Williams,
Daniel Vetter, Kees Cook, Pali Rohár, Oliver O'Halloran,
linux-pci
On Fri, Jul 30, 2021 at 02:50:13PM -0500, Bjorn Helgaas wrote:
> On Fri, Jul 30, 2021 at 07:06:26AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Jul 29, 2021 at 11:32:33PM +0000, Krzysztof Wilczyński wrote:
> > > Hello,
> > >
> > > At the moment, the dependency on iomem_get_mapping() that is currently
> > > used in the pci_create_resource_files() and pci_create_legacy_files()
> > > stops us from completely retiring the late_initcall() that is used to
> > > invoke pci_sysfs_init() when creating sysfs object for PCI devices.
> > >
> > > This dependency on iomem_get_mapping() stops us from retiring the
> > > late_initcall at the moment as when we convert dynamically added sysfs
> > > objects, that are primarily added in the pci_create_resource_files() and
> > > pci_create_legacy_files(), as these attributes are added before the VFS
> > > completes its initialisation, and since most of the PCI devices are
> > > typically enumerated in subsys_initcall this leads to a failure and an
> > > Oops related to iomem_get_mapping() access.
> > >
> > > See relevant conversations:
> > > https://lore.kernel.org/linux-pci/20210204165831.2703772-1-daniel.vetter@ffwll.ch/
> > > https://lore.kernel.org/linux-pci/20210313215747.GA2394467@bjorn-Precision-5520/
> > >
> > > After some deliberation about the problem at hand, Dan Williams
> > > suggested a solution to the problem, as per:
> > > https://lore.kernel.org/linux-pci/CAPcyv4i0y_4cMGEpNVShLUyUk3nyWH203Ry3S87BqnDJE0Rmxg@mail.gmail.com/
> > >
> > > The idea is to defer execution of the iomem_get_mapping() to only when
> > > the sysfs open callback is run, and thus removing the reliance of
> > > fs_initcalls to complete before any other sub-system that uses the
> > > iomem_get_mapping().
> > >
> > > Currently, the PCI sub-system will benefit the most from this change
> > > allowing for it to complete the transition from dynamically created to
> > > static sysfs objects.
> > >
> > > This series aims to take Dan Williams' idea through the finish line.
> > >
> > > Related to:
> > > https://lore.kernel.org/linux-pci/20210527205845.GA1421476@bjorn-Precision-5520/
> > > https://lore.kernel.org/linux-pci/20210507102706.7658-1-danijel.slivka@amd.com/
> > > https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/
> > >
> > > Krzysztof
> > >
> >
> > No objection from me on these, Bjorn, mind if I take them through my
> > driver core tree?
>
> That'd be great!
>
> Is your tree immutable? I'm hoping we can take advantage of this to
> get rid of pci_sysfs_init(), which will fix a race that people seem to
> be hitting frequently now [1]. I think Krzysztof is getting pretty
> close.
>
> [1] https://lore.kernel.org/r/m3eebg9puj.fsf@t19.piap.pl
Yes, my tree is immutable. Let me create a branch just for this that
you can pull from after it passes 0-day...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] Allow deferred execution of iomem_get_mapping()
2021-08-05 12:43 ` Greg Kroah-Hartman
@ 2021-08-06 11:05 ` Greg Kroah-Hartman
0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-06 11:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Krzysztof Wilczyński, Bjorn Helgaas, Dan Williams,
Daniel Vetter, Kees Cook, Pali Rohár, Oliver O'Halloran,
linux-pci
On Thu, Aug 05, 2021 at 02:43:46PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 30, 2021 at 02:50:13PM -0500, Bjorn Helgaas wrote:
> > On Fri, Jul 30, 2021 at 07:06:26AM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Jul 29, 2021 at 11:32:33PM +0000, Krzysztof Wilczyński wrote:
> > > > Hello,
> > > >
> > > > At the moment, the dependency on iomem_get_mapping() that is currently
> > > > used in the pci_create_resource_files() and pci_create_legacy_files()
> > > > stops us from completely retiring the late_initcall() that is used to
> > > > invoke pci_sysfs_init() when creating sysfs object for PCI devices.
> > > >
> > > > This dependency on iomem_get_mapping() stops us from retiring the
> > > > late_initcall at the moment as when we convert dynamically added sysfs
> > > > objects, that are primarily added in the pci_create_resource_files() and
> > > > pci_create_legacy_files(), as these attributes are added before the VFS
> > > > completes its initialisation, and since most of the PCI devices are
> > > > typically enumerated in subsys_initcall this leads to a failure and an
> > > > Oops related to iomem_get_mapping() access.
> > > >
> > > > See relevant conversations:
> > > > https://lore.kernel.org/linux-pci/20210204165831.2703772-1-daniel.vetter@ffwll.ch/
> > > > https://lore.kernel.org/linux-pci/20210313215747.GA2394467@bjorn-Precision-5520/
> > > >
> > > > After some deliberation about the problem at hand, Dan Williams
> > > > suggested a solution to the problem, as per:
> > > > https://lore.kernel.org/linux-pci/CAPcyv4i0y_4cMGEpNVShLUyUk3nyWH203Ry3S87BqnDJE0Rmxg@mail.gmail.com/
> > > >
> > > > The idea is to defer execution of the iomem_get_mapping() to only when
> > > > the sysfs open callback is run, and thus removing the reliance of
> > > > fs_initcalls to complete before any other sub-system that uses the
> > > > iomem_get_mapping().
> > > >
> > > > Currently, the PCI sub-system will benefit the most from this change
> > > > allowing for it to complete the transition from dynamically created to
> > > > static sysfs objects.
> > > >
> > > > This series aims to take Dan Williams' idea through the finish line.
> > > >
> > > > Related to:
> > > > https://lore.kernel.org/linux-pci/20210527205845.GA1421476@bjorn-Precision-5520/
> > > > https://lore.kernel.org/linux-pci/20210507102706.7658-1-danijel.slivka@amd.com/
> > > > https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/
> > > >
> > > > Krzysztof
> > > >
> > >
> > > No objection from me on these, Bjorn, mind if I take them through my
> > > driver core tree?
> >
> > That'd be great!
> >
> > Is your tree immutable? I'm hoping we can take advantage of this to
> > get rid of pci_sysfs_init(), which will fix a race that people seem to
> > be hitting frequently now [1]. I think Krzysztof is getting pretty
> > close.
> >
> > [1] https://lore.kernel.org/r/m3eebg9puj.fsf@t19.piap.pl
>
> Yes, my tree is immutable. Let me create a branch just for this that
> you can pull from after it passes 0-day...
Ok, here you go, a signed tag for you to pull from:
The following changes since commit ff1176468d368232b684f75e82563369208bc371:
Linux 5.14-rc3 (2021-07-25 15:35:14 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/sysfs_defferred_iomem_get_mapping-5.15
for you to fetch changes up to f06aff924f975881a6abf91d2af0078fc8cd37bf:
sysfs: Rename struct bin_attribute member to f_mapping (2021-08-05 14:47:31 +0200)
----------------------------------------------------------------
sysfs: Allow deferred execution of iomem_get_mapping()
Tag for toerh trees/branches to pull from in order to have a stable base
to build off of for the "Allow deferred execution of
iomem_get_mapping()" set of sysfs changes
Link: https://lore.kernel.org/r/20210729233235.1508920-1-kw@linux.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Krzysztof Wilczyński (2):
sysfs: Invoke iomem_get_mapping() from the sysfs open callback
sysfs: Rename struct bin_attribute member to f_mapping
drivers/pci/pci-sysfs.c | 6 +++---
fs/sysfs/file.c | 4 ++--
include/linux/sysfs.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread