| Message ID | 20190728202213.15550-2-efremov@linux.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | PCI: Convert pci_resource_to_user() to a weak function | expand |
Hi Denis, On Sun, Jul 28, 2019 at 11:22:09PM +0300, Denis Efremov wrote: > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 9e700d9f9f28..1a19d0151b0a 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1870,25 +1870,13 @@ static inline const char *pci_name(const struct pci_dev *pdev) > return dev_name(&pdev->dev); > } > > - > /* > * Some archs don't want to expose struct resource to userland as-is > * in sysfs and /proc > */ > -#ifdef HAVE_ARCH_PCI_RESOURCE_TO_USER > -void pci_resource_to_user(const struct pci_dev *dev, int bar, > - const struct resource *rsrc, > - resource_size_t *start, resource_size_t *end); > -#else > -static inline void pci_resource_to_user(const struct pci_dev *dev, int bar, > - const struct resource *rsrc, resource_size_t *start, > - resource_size_t *end) > -{ > - *start = rsrc->start; > - *end = rsrc->end; > -} > -#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */ > - > +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar, > + const struct resource *rsrc, > + resource_size_t *start, resource_size_t *end); > > /* > * The world is not perfect and supplies us with broken PCI devices. This is wrong - using __weak on the declaration in a header will cause the weak attribute to be applied to all implementations too (presuming the C files containing the implementations include the header). You then get whichever impleentation the linker chooses, which isn't necessarily the one you wanted. checkpatch.pl should produce an error about this - see the WEAK_DECLARATION error introduced in commit 619a908aa334 ("checkpatch: add error on use of attribute((weak)) or __weak declarations"). Thanks, Paul
On Sun, 2019-07-28 at 22:49 +0000, Paul Burton wrote: > Hi Denis, > > On Sun, Jul 28, 2019 at 11:22:09PM +0300, Denis Efremov wrote: > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 9e700d9f9f28..1a19d0151b0a 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -1870,25 +1870,13 @@ static inline const char *pci_name(const struct pci_dev *pdev) > > return dev_name(&pdev->dev); > > } > > > > - > > /* > > * Some archs don't want to expose struct resource to userland as-is > > * in sysfs and /proc > > */ > > -#ifdef HAVE_ARCH_PCI_RESOURCE_TO_USER > > -void pci_resource_to_user(const struct pci_dev *dev, int bar, > > - const struct resource *rsrc, > > - resource_size_t *start, resource_size_t *end); > > -#else > > -static inline void pci_resource_to_user(const struct pci_dev *dev, int bar, > > - const struct resource *rsrc, resource_size_t *start, > > - resource_size_t *end) > > -{ > > - *start = rsrc->start; > > - *end = rsrc->end; > > -} > > -#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */ > > - > > +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar, > > + const struct resource *rsrc, > > + resource_size_t *start, resource_size_t *end); > > > > /* > > * The world is not perfect and supplies us with broken PCI devices. > > This is wrong - using __weak on the declaration in a header will cause > the weak attribute to be applied to all implementations too (presuming > the C files containing the implementations include the header). You then > get whichever impleentation the linker chooses, which isn't necessarily > the one you wanted. > > checkpatch.pl should produce an error about this - see the > WEAK_DECLARATION error introduced in commit 619a908aa334 ("checkpatch: > add error on use of attribute((weak)) or __weak declarations"). Unfortunately, checkpatch is pretty stupid and only emits this on single line declarations.
Hi Paul, On 29.07.2019 01:49, Paul Burton wrote: > Hi Denis, > > This is wrong - using __weak on the declaration in a header will cause > the weak attribute to be applied to all implementations too (presuming > the C files containing the implementations include the header). You then > get whichever impleentation the linker chooses, which isn't necessarily > the one you wanted. Thank you for pointing me on that. I will prepare the v2.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 29ed5ec1ac27..f9dc7563a8b9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5932,6 +5932,14 @@ resource_size_t __weak pcibios_default_alignment(void) return 0; } +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar, + const struct resource *rsrc, resource_size_t *start, + resource_size_t *end) +{ + *start = rsrc->start; + *end = rsrc->end; +} + #define RESOURCE_ALIGNMENT_PARAM_SIZE COMMAND_LINE_SIZE static char resource_alignment_param[RESOURCE_ALIGNMENT_PARAM_SIZE] = {0}; static DEFINE_SPINLOCK(resource_alignment_lock); diff --git a/include/linux/pci.h b/include/linux/pci.h index 9e700d9f9f28..1a19d0151b0a 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1870,25 +1870,13 @@ static inline const char *pci_name(const struct pci_dev *pdev) return dev_name(&pdev->dev); } - /* * Some archs don't want to expose struct resource to userland as-is * in sysfs and /proc */ -#ifdef HAVE_ARCH_PCI_RESOURCE_TO_USER -void pci_resource_to_user(const struct pci_dev *dev, int bar, - const struct resource *rsrc, - resource_size_t *start, resource_size_t *end); -#else -static inline void pci_resource_to_user(const struct pci_dev *dev, int bar, - const struct resource *rsrc, resource_size_t *start, - resource_size_t *end) -{ - *start = rsrc->start; - *end = rsrc->end; -} -#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */ - +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar, + const struct resource *rsrc, + resource_size_t *start, resource_size_t *end); /* * The world is not perfect and supplies us with broken PCI devices.
The patch turns pci_resource_to_user() to a weak function. Thus, architecture-specific versions will automatically override the generic one. This allows to remove the HAVE_ARCH_PCI_RESOURCE_TO_USER macro and avoid the conditional compilation for this single function. Signed-off-by: Denis Efremov <efremov@linux.com> --- drivers/pci/pci.c | 8 ++++++++ include/linux/pci.h | 18 +++--------------- 2 files changed, 11 insertions(+), 15 deletions(-)