Fix the empty pci_iounmap() implementation that is causing memory leaks on
!CONFIG_GENERIC_IOMAP configs relying on asm-generic/io.h.
A small tweak is required on sparc32 to pull in some declarations,
hopefully nothing problematic, subject to changes as requested.
Previous tentatives:
https://lore.kernel.org/lkml/[email protected]
https://lore.kernel.org/lkml/[email protected]
Cc: Bjorn Helgaas <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: George Cherian <[email protected]>
Cc: Yang Yingliang <[email protected]>
Lorenzo Pieralisi (2):
sparc32: Move ioremap/iounmap declaration before asm-generic/io.h
include
asm-generic/io.h: Fix !CONFIG_GENERIC_IOMAP pci_iounmap()
implementation
arch/sparc/include/asm/io_32.h | 16 ++++++++------
include/asm-generic/io.h | 39 +++++++++++++++++++++++-----------
2 files changed, 37 insertions(+), 18 deletions(-)
--
2.26.1
Move the ioremap/iounmap declaration before asm-generic/io.h is
included so that it is visible within it.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
arch/sparc/include/asm/io_32.h | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
index 9a52d9506f80..042201c79ad1 100644
--- a/arch/sparc/include/asm/io_32.h
+++ b/arch/sparc/include/asm/io_32.h
@@ -11,6 +11,16 @@
#define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz)
#define memcpy_toio(d,s,sz) _memcpy_toio(d,s,sz)
+#ifdef __KERNEL__
+
+/*
+ * Bus number may be embedded in the higher bits of the physical address.
+ * This is why we have no bus number argument to ioremap().
+ */
+void __iomem *ioremap(phys_addr_t offset, size_t size);
+void iounmap(volatile void __iomem *addr);
+#endif
+
#include <asm-generic/io.h>
static inline void _memset_io(volatile void __iomem *dst,
@@ -123,12 +133,6 @@ static inline void sbus_memcpy_toio(volatile void __iomem *dst,
#ifdef __KERNEL__
-/*
- * Bus number may be embedded in the higher bits of the physical address.
- * This is why we have no bus number argument to ioremap().
- */
-void __iomem *ioremap(phys_addr_t offset, size_t size);
-void iounmap(volatile void __iomem *addr);
/* Create a virtual mapping cookie for an IO port range */
void __iomem *ioport_map(unsigned long port, unsigned int nr);
void ioport_unmap(void __iomem *);
--
2.26.1
For arches that do not select CONFIG_GENERIC_IOMAP, the current
pci_iounmap() function does nothing causing obvious memory leaks
for mapped regions that are backed by MMIO physical space.
In order to detect if a mapped pointer is IO vs MMIO, a check must made
available to the pci_iounmap() function so that it can actually detect
whether the pointer has to be unmapped.
In configurations where CONFIG_HAS_IOPORT_MAP && !CONFIG_GENERIC_IOMAP,
a mapped port is detected using an ioport_map() stub defined in
asm-generic/io.h.
Use the same logic to implement a stub (ie __pci_ioport_unmap()) that
detects if the passed in pointer in pci_iounmap() is IO vs MMIO to
iounmap conditionally and call it in pci_iounmap() fixing the issue.
Leave __pci_ioport_unmap() as a NOP for all other config options.
Reported-by: George Cherian <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Link: https://lore.kernel.org/lkml/[email protected]
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: George Cherian <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Yang Yingliang <[email protected]>
---
include/asm-generic/io.h | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index dabf8cb7203b..9ea83d80eb6f 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -911,18 +911,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
#include <linux/vmalloc.h>
#define __io_virt(x) ((void __force *)(x))
-#ifndef CONFIG_GENERIC_IOMAP
-struct pci_dev;
-extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
-
-#ifndef pci_iounmap
-#define pci_iounmap pci_iounmap
-static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
-{
-}
-#endif
-#endif /* CONFIG_GENERIC_IOMAP */
-
/*
* Change virtual addresses to physical addresses and vv.
* These are pretty trivial
@@ -1016,6 +1004,16 @@ static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
port &= IO_SPACE_LIMIT;
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
}
+#define __pci_ioport_unmap __pci_ioport_unmap
+static inline void __pci_ioport_unmap(void __iomem *p)
+{
+ uintptr_t start = (uintptr_t) PCI_IOBASE;
+ uintptr_t addr = (uintptr_t) p;
+
+ if (addr >= start && addr < start + IO_SPACE_LIMIT)
+ return;
+ iounmap(p);
+}
#endif
#ifndef ioport_unmap
@@ -1030,6 +1028,23 @@ extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
#endif /* CONFIG_HAS_IOPORT_MAP */
+#ifndef CONFIG_GENERIC_IOMAP
+struct pci_dev;
+extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+
+#ifndef __pci_ioport_unmap
+static inline void __pci_ioport_unmap(void __iomem *p) {}
+#endif
+
+#ifndef pci_iounmap
+#define pci_iounmap pci_iounmap
+static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
+{
+ __pci_ioport_unmap(p);
+}
+#endif
+#endif /* CONFIG_GENERIC_IOMAP */
+
/*
* Convert a virtual cached pointer to an uncached pointer
*/
--
2.26.1
From: Lorenzo Pieralisi <[email protected]>
Date: Tue, 15 Sep 2020 10:32:02 +0100
> Move the ioremap/iounmap declaration before asm-generic/io.h is
> included so that it is visible within it.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
Acked-by: David S. Miller <[email protected]>
On Tue, Sep 15, 2020 at 03:14:52PM +0100, Christoph Hellwig wrote:
> > diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
> > index 9a52d9506f80..042201c79ad1 100644
> > --- a/arch/sparc/include/asm/io_32.h
> > +++ b/arch/sparc/include/asm/io_32.h
> > @@ -11,6 +11,16 @@
> > #define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz)
> > #define memcpy_toio(d,s,sz) _memcpy_toio(d,s,sz)
> >
> > +#ifdef __KERNEL__
> > +
> > +/*
> > + * Bus number may be embedded in the higher bits of the physical address.
> > + * This is why we have no bus number argument to ioremap().
> > + */
> > +void __iomem *ioremap(phys_addr_t offset, size_t size);
> > +void iounmap(volatile void __iomem *addr);
> > +#endif
>
> No need for an __KERNEL__ in non-uapi headers.
Sure, just kept the same preproc guard as current code, will add a patch
to remove the guard first before this one then.
Thanks,
Lorenzo
> diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
> index 9a52d9506f80..042201c79ad1 100644
> --- a/arch/sparc/include/asm/io_32.h
> +++ b/arch/sparc/include/asm/io_32.h
> @@ -11,6 +11,16 @@
> #define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz)
> #define memcpy_toio(d,s,sz) _memcpy_toio(d,s,sz)
>
> +#ifdef __KERNEL__
> +
> +/*
> + * Bus number may be embedded in the higher bits of the physical address.
> + * This is why we have no bus number argument to ioremap().
> + */
> +void __iomem *ioremap(phys_addr_t offset, size_t size);
> +void iounmap(volatile void __iomem *addr);
> +#endif
No need for an __KERNEL__ in non-uapi headers.
v2 of a previous posting.
v1->v2:
- Added additional patch to remove sparc32 useless __KERNEL__
guard
v1: https://lore.kernel.org/lkml/[email protected]
Original cover letter
---
Fix the empty pci_iounmap() implementation that is causing memory leaks on
!CONFIG_GENERIC_IOMAP configs relying on asm-generic/io.h.
A small tweak is required on sparc32 to pull in some declarations,
hopefully nothing problematic, subject to changes as requested.
Previous tentatives:
https://lore.kernel.org/lkml/[email protected]
https://lore.kernel.org/lkml/[email protected]
Cc: Bjorn Helgaas <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: George Cherian <[email protected]>
Cc: Yang Yingliang <[email protected]>
Lorenzo Pieralisi (3):
sparc32: Remove useless io_32.h __KERNEL__ preprocessor guard
sparc32: Move ioremap/iounmap declaration before asm-generic/io.h
include
asm-generic/io.h: Fix !CONFIG_GENERIC_IOMAP pci_iounmap()
implementation
arch/sparc/include/asm/io_32.h | 17 ++++++---------
include/asm-generic/io.h | 39 +++++++++++++++++++++++-----------
2 files changed, 34 insertions(+), 22 deletions(-)
--
2.26.1
Move the ioremap/iounmap declaration before asm-generic/io.h is
included so that it is visible within it.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
arch/sparc/include/asm/io_32.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
index 8179958e3ce1..549f0a72280d 100644
--- a/arch/sparc/include/asm/io_32.h
+++ b/arch/sparc/include/asm/io_32.h
@@ -11,6 +11,13 @@
#define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz)
#define memcpy_toio(d,s,sz) _memcpy_toio(d,s,sz)
+/*
+ * Bus number may be embedded in the higher bits of the physical address.
+ * This is why we have no bus number argument to ioremap().
+ */
+void __iomem *ioremap(phys_addr_t offset, size_t size);
+void iounmap(volatile void __iomem *addr);
+
#include <asm-generic/io.h>
static inline void _memset_io(volatile void __iomem *dst,
@@ -121,12 +128,6 @@ static inline void sbus_memcpy_toio(volatile void __iomem *dst,
}
}
-/*
- * Bus number may be embedded in the higher bits of the physical address.
- * This is why we have no bus number argument to ioremap().
- */
-void __iomem *ioremap(phys_addr_t offset, size_t size);
-void iounmap(volatile void __iomem *addr);
/* Create a virtual mapping cookie for an IO port range */
void __iomem *ioport_map(unsigned long port, unsigned int nr);
void ioport_unmap(void __iomem *);
--
2.26.1
For arches that do not select CONFIG_GENERIC_IOMAP, the current
pci_iounmap() function does nothing causing obvious memory leaks
for mapped regions that are backed by MMIO physical space.
In order to detect if a mapped pointer is IO vs MMIO, a check must made
available to the pci_iounmap() function so that it can actually detect
whether the pointer has to be unmapped.
In configurations where CONFIG_HAS_IOPORT_MAP && !CONFIG_GENERIC_IOMAP,
a mapped port is detected using an ioport_map() stub defined in
asm-generic/io.h.
Use the same logic to implement a stub (ie __pci_ioport_unmap()) that
detects if the passed in pointer in pci_iounmap() is IO vs MMIO to
iounmap conditionally and call it in pci_iounmap() fixing the issue.
Leave __pci_ioport_unmap() as a NOP for all other config options.
Reported-by: George Cherian <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Link: https://lore.kernel.org/lkml/[email protected]
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: George Cherian <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Yang Yingliang <[email protected]>
---
include/asm-generic/io.h | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index dabf8cb7203b..9ea83d80eb6f 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -911,18 +911,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
#include <linux/vmalloc.h>
#define __io_virt(x) ((void __force *)(x))
-#ifndef CONFIG_GENERIC_IOMAP
-struct pci_dev;
-extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
-
-#ifndef pci_iounmap
-#define pci_iounmap pci_iounmap
-static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
-{
-}
-#endif
-#endif /* CONFIG_GENERIC_IOMAP */
-
/*
* Change virtual addresses to physical addresses and vv.
* These are pretty trivial
@@ -1016,6 +1004,16 @@ static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
port &= IO_SPACE_LIMIT;
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
}
+#define __pci_ioport_unmap __pci_ioport_unmap
+static inline void __pci_ioport_unmap(void __iomem *p)
+{
+ uintptr_t start = (uintptr_t) PCI_IOBASE;
+ uintptr_t addr = (uintptr_t) p;
+
+ if (addr >= start && addr < start + IO_SPACE_LIMIT)
+ return;
+ iounmap(p);
+}
#endif
#ifndef ioport_unmap
@@ -1030,6 +1028,23 @@ extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
#endif /* CONFIG_HAS_IOPORT_MAP */
+#ifndef CONFIG_GENERIC_IOMAP
+struct pci_dev;
+extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+
+#ifndef __pci_ioport_unmap
+static inline void __pci_ioport_unmap(void __iomem *p) {}
+#endif
+
+#ifndef pci_iounmap
+#define pci_iounmap pci_iounmap
+static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
+{
+ __pci_ioport_unmap(p);
+}
+#endif
+#endif /* CONFIG_GENERIC_IOMAP */
+
/*
* Convert a virtual cached pointer to an uncached pointer
*/
--
2.26.1
On Wed, Sep 16, 2020 at 12:06:58PM +0100, Lorenzo Pieralisi wrote:
> For arches that do not select CONFIG_GENERIC_IOMAP, the current
> pci_iounmap() function does nothing causing obvious memory leaks
> for mapped regions that are backed by MMIO physical space.
>
> In order to detect if a mapped pointer is IO vs MMIO, a check must made
> available to the pci_iounmap() function so that it can actually detect
> whether the pointer has to be unmapped.
>
> In configurations where CONFIG_HAS_IOPORT_MAP && !CONFIG_GENERIC_IOMAP,
> a mapped port is detected using an ioport_map() stub defined in
> asm-generic/io.h.
>
> Use the same logic to implement a stub (ie __pci_ioport_unmap()) that
> detects if the passed in pointer in pci_iounmap() is IO vs MMIO to
> iounmap conditionally and call it in pci_iounmap() fixing the issue.
>
> Leave __pci_ioport_unmap() as a NOP for all other config options.
>
> Reported-by: George Cherian <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: George Cherian <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Yang Yingliang <[email protected]>
> ---
> include/asm-generic/io.h | 39 +++++++++++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 12 deletions(-)
This works for me. The only question I have is whether pci_iomap.h is
better than io.h for __pci_ioport_unmap(). These headers are really
confusing.
Either way:
Reviewed-by: Catalin Marinas <[email protected]>
The __KERNEL_ preprocessor guard is useless in non-uapi headers.
Remove it.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: David S. Miller <[email protected]>
---
arch/sparc/include/asm/io_32.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h
index 9a52d9506f80..8179958e3ce1 100644
--- a/arch/sparc/include/asm/io_32.h
+++ b/arch/sparc/include/asm/io_32.h
@@ -121,8 +121,6 @@ static inline void sbus_memcpy_toio(volatile void __iomem *dst,
}
}
-#ifdef __KERNEL__
-
/*
* Bus number may be embedded in the higher bits of the physical address.
* This is why we have no bus number argument to ioremap().
@@ -148,8 +146,6 @@ static inline int sbus_can_burst64(void)
struct device;
void sbus_set_sbus64(struct device *, int);
-#endif
-
#define __ARCH_HAS_NO_PAGE_ZERO_MAPPED 1
--
2.26.1
On Wed, Sep 16, 2020 at 03:51:11PM +0100, Catalin Marinas wrote:
> On Wed, Sep 16, 2020 at 12:06:58PM +0100, Lorenzo Pieralisi wrote:
> > For arches that do not select CONFIG_GENERIC_IOMAP, the current
> > pci_iounmap() function does nothing causing obvious memory leaks
> > for mapped regions that are backed by MMIO physical space.
> >
> > In order to detect if a mapped pointer is IO vs MMIO, a check must made
> > available to the pci_iounmap() function so that it can actually detect
> > whether the pointer has to be unmapped.
> >
> > In configurations where CONFIG_HAS_IOPORT_MAP && !CONFIG_GENERIC_IOMAP,
> > a mapped port is detected using an ioport_map() stub defined in
> > asm-generic/io.h.
> >
> > Use the same logic to implement a stub (ie __pci_ioport_unmap()) that
> > detects if the passed in pointer in pci_iounmap() is IO vs MMIO to
> > iounmap conditionally and call it in pci_iounmap() fixing the issue.
> >
> > Leave __pci_ioport_unmap() as a NOP for all other config options.
> >
> > Reported-by: George Cherian <[email protected]>
> > Link: https://lore.kernel.org/lkml/[email protected]
> > Link: https://lore.kernel.org/lkml/[email protected]
> > Signed-off-by: Lorenzo Pieralisi <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: George Cherian <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Yang Yingliang <[email protected]>
> > ---
> > include/asm-generic/io.h | 39 +++++++++++++++++++++++++++------------
> > 1 file changed, 27 insertions(+), 12 deletions(-)
>
> This works for me. The only question I have is whether pci_iomap.h is
> better than io.h for __pci_ioport_unmap(). These headers are really
> confusing.
Yes they are, in total honesty there is much more to do to make them
sane, this patch is just a band-aid.
I thought about moving this stuff into pci_iomap.h, though that
file is included _independently_ from io.h from some arches so
I tried to keep everything in io.h to minimize disruption.
We can merge this patch - since it is a fix after all - and then I can
try to improve the whole pci_iounmap() includes.
> Either way:
>
> Reviewed-by: Catalin Marinas <[email protected]>
Thanks a lot. I'd appreciate a tested-by from the George as he is
the one who reported the problem.
Lorenzo
> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: Thursday, September 17, 2020 3:00 PM
> To: Catalin Marinas <[email protected]>
> Cc: [email protected]; George Cherian <[email protected]>;
> Arnd Bergmann <[email protected]>; Will Deacon <[email protected]>; Bjorn
> Helgaas <[email protected]>; Yang Yingliang
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; David S. Miller
> <[email protected]>
> Subject: Re: [PATCH v2 3/3] asm-generic/io.h: Fix
> !CONFIG_GENERIC_IOMAP pci_iounmap() implementation
>
>
> ----------------------------------------------------------------------
> On Wed, Sep 16, 2020 at 03:51:11PM +0100, Catalin Marinas wrote:
> > On Wed, Sep 16, 2020 at 12:06:58PM +0100, Lorenzo Pieralisi wrote:
> > > For arches that do not select CONFIG_GENERIC_IOMAP, the current
> > > pci_iounmap() function does nothing causing obvious memory leaks for
> > > mapped regions that are backed by MMIO physical space.
> > >
> > > In order to detect if a mapped pointer is IO vs MMIO, a check must
> > > made available to the pci_iounmap() function so that it can actually
> > > detect whether the pointer has to be unmapped.
> > >
> > > In configurations where CONFIG_HAS_IOPORT_MAP &&
> > > !CONFIG_GENERIC_IOMAP, a mapped port is detected using an
> > > ioport_map() stub defined in asm-generic/io.h.
> > >
> > > Use the same logic to implement a stub (ie __pci_ioport_unmap())
> > > that detects if the passed in pointer in pci_iounmap() is IO vs MMIO
> > > to iounmap conditionally and call it in pci_iounmap() fixing the issue.
> > >
> > > Leave __pci_ioport_unmap() as a NOP for all other config options.
> > >
> > > Reported-by: George Cherian <[email protected]>
> > > Link:
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org
> > > _lkml_20200905024811.74701-2D1-2Dyangyingliang-
> 40huawei.com&d=DwIBAg
> > >
> &c=nKjWec2b6R0mOyPaz7xtfQ&r=TjMsEFPc7dirkF6u2D3eSIS0cA8FeYpzRkk
> Mzr4a
> > > Cbk&m=UO5qU5LtNtCn6_gnT0rCkBxIm-w8jCaxHO6v7oK-U-
> I&s=CSGHQpKoVdNiqb1e
> > > DFuRUhka_Xv5o2PosWZ1rR8oOD4&e=
> > > Link:
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org
> > > _lkml_20200824132046.3114383-2D1-2Dgeorge.cherian-
> 40marvell.com&d=Dw
> > >
> IBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=TjMsEFPc7dirkF6u2D3eSIS0cA8FeYpz
> RkkM
> > > zr4aCbk&m=UO5qU5LtNtCn6_gnT0rCkBxIm-w8jCaxHO6v7oK-U-
> I&s=3B83oan7i1g3
> > > KaPgQmFK6PudR9GzvAPk33Z5Yyv-CMI&e=
> > > Signed-off-by: Lorenzo Pieralisi <[email protected]>
> > > Cc: Arnd Bergmann <[email protected]>
> > > Cc: George Cherian <[email protected]>
> > > Cc: Will Deacon <[email protected]>
> > > Cc: Bjorn Helgaas <[email protected]>
> > > Cc: Catalin Marinas <[email protected]>
> > > Cc: Yang Yingliang <[email protected]>
> > > ---
> > > include/asm-generic/io.h | 39
> > > +++++++++++++++++++++++++++------------
> > > 1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > This works for me. The only question I have is whether pci_iomap.h is
> > better than io.h for __pci_ioport_unmap(). These headers are really
> > confusing.
>
> Yes they are, in total honesty there is much more to do to make them sane,
> this patch is just a band-aid.
>
> I thought about moving this stuff into pci_iomap.h, though that file is
> included _independently_ from io.h from some arches so I tried to keep
> everything in io.h to minimize disruption.
>
> We can merge this patch - since it is a fix after all - and then I can try to
> improve the whole pci_iounmap() includes.
>
> > Either way:
> >
> > Reviewed-by: Catalin Marinas <[email protected]>
>
> Thanks a lot. I'd appreciate a tested-by from the George as he is the one who
> reported the problem.
Verified this patch and it works as expected.
Tested-by: George Cherian <[email protected]>
> Lorenzo
On Wed, Sep 16, 2020 at 12:06:55PM +0100, Lorenzo Pieralisi wrote:
> v2 of a previous posting.
>
> v1->v2:
> - Added additional patch to remove sparc32 useless __KERNEL__
> guard
>
> v1: https://lore.kernel.org/lkml/[email protected]
>
> Original cover letter
> ---
>
> Fix the empty pci_iounmap() implementation that is causing memory leaks on
> !CONFIG_GENERIC_IOMAP configs relying on asm-generic/io.h.
>
> A small tweak is required on sparc32 to pull in some declarations,
> hopefully nothing problematic, subject to changes as requested.
>
> Previous tentatives:
> https://lore.kernel.org/lkml/[email protected]
> https://lore.kernel.org/lkml/[email protected]
>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: George Cherian <[email protected]>
> Cc: Yang Yingliang <[email protected]>
>
> Lorenzo Pieralisi (3):
> sparc32: Remove useless io_32.h __KERNEL__ preprocessor guard
> sparc32: Move ioremap/iounmap declaration before asm-generic/io.h
> include
> asm-generic/io.h: Fix !CONFIG_GENERIC_IOMAP pci_iounmap()
> implementation
>
> arch/sparc/include/asm/io_32.h | 17 ++++++---------
> include/asm-generic/io.h | 39 +++++++++++++++++++++++-----------
> 2 files changed, 34 insertions(+), 22 deletions(-)
Arnd, David, Bjorn,
I have got review/test tags, is it OK if we merge this series please ?
Can we pull it in the PCI tree or you want it to go via a different
route upstream ?
Please let me know.
Thanks,
Lorenzo
On Fri, Sep 18, 2020 at 1:45 PM Lorenzo Pieralisi
<[email protected]> wrote:
> >
> > Lorenzo Pieralisi (3):
> > sparc32: Remove useless io_32.h __KERNEL__ preprocessor guard
> > sparc32: Move ioremap/iounmap declaration before asm-generic/io.h
> > include
> > asm-generic/io.h: Fix !CONFIG_GENERIC_IOMAP pci_iounmap()
> > implementation
> >
> > arch/sparc/include/asm/io_32.h | 17 ++++++---------
> > include/asm-generic/io.h | 39 +++++++++++++++++++++++-----------
> > 2 files changed, 34 insertions(+), 22 deletions(-)
>
> Arnd, David, Bjorn,
>
> I have got review/test tags, is it OK if we merge this series please ?
>
> Can we pull it in the PCI tree or you want it to go via a different
> route upstream ?
>
> Please let me know.
Going through the PCI tree sounds good to me, but I can
take it through the asm-generic tree if Bjorn doesn't want to
pick it up there.
Arnd
On Fri, Sep 18, 2020 at 09:58:51PM +0200, Arnd Bergmann wrote:
> On Fri, Sep 18, 2020 at 1:45 PM Lorenzo Pieralisi
> <[email protected]> wrote:
> > >
> > > Lorenzo Pieralisi (3):
> > > sparc32: Remove useless io_32.h __KERNEL__ preprocessor guard
> > > sparc32: Move ioremap/iounmap declaration before asm-generic/io.h
> > > include
> > > asm-generic/io.h: Fix !CONFIG_GENERIC_IOMAP pci_iounmap()
> > > implementation
> > >
> > > arch/sparc/include/asm/io_32.h | 17 ++++++---------
> > > include/asm-generic/io.h | 39 +++++++++++++++++++++++-----------
> > > 2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > Arnd, David, Bjorn,
> >
> > I have got review/test tags, is it OK if we merge this series please ?
> >
> > Can we pull it in the PCI tree or you want it to go via a different
> > route upstream ?
> >
> > Please let me know.
>
> Going through the PCI tree sounds good to me, but I can
> take it through the asm-generic tree if Bjorn doesn't want to
> pick it up there.
Bjorn, can we pull this series into PCI tree please - if that's OK
with you ?
Thanks,
Lorenzo
On Tue, Sep 15, 2020 at 01:11:21PM -0700, David Miller wrote:
> From: Lorenzo Pieralisi <[email protected]>
> Date: Tue, 15 Sep 2020 10:32:02 +0100
>
> > Move the ioremap/iounmap declaration before asm-generic/io.h is
> > included so that it is visible within it.
> >
> > Signed-off-by: Lorenzo Pieralisi <[email protected]>
>
> Acked-by: David S. Miller <[email protected]>
Hi David,
can I apply your Acked-by to v2 (where I had to split this patch in 2):
https://lore.kernel.org/lkml/[email protected]
I am about to merge it - please let me know.
Thanks,
Lorenzo
From: Lorenzo Pieralisi <[email protected]>
Date: Fri, 2 Oct 2020 15:50:29 +0100
> On Tue, Sep 15, 2020 at 01:11:21PM -0700, David Miller wrote:
>> From: Lorenzo Pieralisi <[email protected]>
>> Date: Tue, 15 Sep 2020 10:32:02 +0100
>>
>> > Move the ioremap/iounmap declaration before asm-generic/io.h is
>> > included so that it is visible within it.
>> >
>> > Signed-off-by: Lorenzo Pieralisi <[email protected]>
>>
>> Acked-by: David S. Miller <[email protected]>
>
> Hi David,
>
> can I apply your Acked-by to v2 (where I had to split this patch in 2):
>
> https://lore.kernel.org/lkml/[email protected]
> I am about to merge it - please let me know.
Yes, you can.