diff mbox series

[v1,07/10] powerpc/mm: move iounmap() into ioremap.c and drop __iounmap()

Message ID 005a2b9321c10e23ca399e6ff4b19960009561f5.1565726867.git.christophe.leroy@c-s.fr (mailing list archive)
State Superseded
Headers show
Series [v1,01/10] powerpc/mm: drop ppc_md.iounmap() | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (da206bd46848568e1aaf35f00e2d78bf9bc94f95)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 0 checks, 129 lines checked

Commit Message

Christophe Leroy Aug. 13, 2019, 8:11 p.m. UTC
On PPC64 iounmap() does nothing else than calling __iounmap()
and is the only user of __iounmap().
__iounmap() is almost similar to PPC32 iounmap().

Lets define a common iounmap() and drop __iounmap().

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/book3s/32/pgtable.h |  2 ++
 arch/powerpc/include/asm/io.h                |  5 -----
 arch/powerpc/include/asm/nohash/32/pgtable.h |  2 ++
 arch/powerpc/mm/ioremap.c                    | 31 ++++++++++++++++++++++++++++
 arch/powerpc/mm/pgtable_32.c                 | 14 -------------
 arch/powerpc/mm/pgtable_64.c                 | 28 -------------------------
 6 files changed, 35 insertions(+), 47 deletions(-)

Comments

Michael Ellerman Aug. 19, 2019, 12:55 p.m. UTC | #1
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
> index 0c23660522ca..57d742509cec 100644
> --- a/arch/powerpc/mm/ioremap.c
> +++ b/arch/powerpc/mm/ioremap.c
> @@ -72,3 +75,31 @@ void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long f
>  	return __ioremap_caller(addr, size, pte_pgprot(pte), caller);
>  }
>  EXPORT_SYMBOL(ioremap_prot);
> +
> +/*
> + * Unmap an IO region and remove it from vmalloc'd list.
> + * Access to IO memory should be serialized by driver.
> + */
> +void iounmap(volatile void __iomem *token)
> +{
> +	void *addr;
> +
> +	/*
> +	 * If mapped by BATs then there is nothing to do.
> +	 */
> +	if (v_block_mapped((unsigned long)token))
> +		return;
> +
> +	if (!slab_is_available())
> +		return;
> +
> +	addr = (void *)((unsigned long __force)PCI_FIX_ADDR(token) & PAGE_MASK);
> +	if (WARN_ON((unsigned long)addr < IOREMAP_BASE))
> +		return;

This pops a bunch, as we seem to have various places that want to call
iounmap(NULL) in error paths, much like kfree().

One example:

[   85.062269] WARNING: CPU: 6 PID: 3643 at arch/powerpc/mm/ioremap.c:97 .iounmap+0x58/0xb0
[   85.062276] Modules linked in: snd_powermac(+) snd_pcm snd_timer snd soundcore
[   85.062314] CPU: 6 PID: 3643 Comm: modprobe Tainted: G        W         5.3.0-rc2-gcc-8.2.0-00051-ga8e8d67f314c #655
[   85.062325] NIP:  c000000000078e08 LR: c000000000078dd0 CTR: c000000000078db0
[   85.062335] REGS: c0000000f44f6e40 TRAP: 0700   Tainted: G        W          (5.3.0-rc2-gcc-8.2.0-00051-ga8e8d67f314c)
[   85.062342] MSR:  8000000002029032 <SF,VEC,EE,ME,IR,DR,RI>  CR: 24228884  XER: 00000000
[   85.062377] CFAR: c000000000339650 IRQMASK: 0 
               GPR00: c000000000078dd0 c0000000f44f70d0 c000000001a2ff00 0000000000000000 
               GPR04: c008000000336518 0000000000000000 c000000001a66b80 0000000000000001 
               GPR08: c0000000013296c8 c00a000080000000 0000000000000001 c00800000032ba08 
               GPR12: c000000000078db0 c00000003fff8e80 0000000000000004 c008000000350000 
               GPR16: c000000002637730 c000000000d69868 0000000000000000 c000000002637740 
               GPR20: c00000000197ad08 0000000000000000 0000000000000100 0000000000000028 
               GPR24: c0000000f736eac0 c0000000f44f7370 c0000000f5065000 c008000000336510 
               GPR28: c0000000026aafc8 ffffffffffffffed 0000000000000000 0000000000000000 
[   85.062554] NIP [c000000000078e08] .iounmap+0x58/0xb0
[   85.062564] LR [c000000000078dd0] .iounmap+0x20/0xb0
[   85.062572] Call Trace:
[   85.062591] [c0000000f44f70d0] [c000000000078dd0] .iounmap+0x20/0xb0 (unreliable)
[   85.062623] [c0000000f44f7150] [c008000000321e24] .snd_pmac_free+0x164/0x270 [snd_powermac]
[   85.062709] [c0000000f44f71e0] [c008000000322fa4] .snd_pmac_new+0x884/0xf30 [snd_powermac]
[   85.062798] [c0000000f44f72f0] [c00800000032015c] .snd_pmac_probe+0x7c/0x450 [snd_powermac]
[   85.062849] [c0000000f44f73a0] [c0000000007b0628] .platform_drv_probe+0x68/0x100
[   85.062863] [c0000000f44f7430] [c0000000007aca94] .really_probe+0x144/0x3c0
[   85.062880] [c0000000f44f74d0] [c0000000007acfe0] .driver_probe_device+0x80/0x170
[   85.062899] [c0000000f44f7560] [c0000000007a9be4] .bus_for_each_drv+0xb4/0x130
[   85.062922] [c0000000f44f7610] [c0000000007ac89c] .__device_attach+0x11c/0x1a0
[   85.062941] [c0000000f44f76c0] [c0000000007ab3d8] .bus_probe_device+0xe8/0x100
[   85.062961] [c0000000f44f7750] [c0000000007a6164] .device_add+0x504/0x7d0
[   85.062978] [c0000000f44f7820] [c0000000007b02fc] .platform_device_add+0x14c/0x310
[   85.062994] [c0000000f44f78c0] [c0000000007b14c0] .platform_device_register_full+0x130/0x210
[   85.063084] [c0000000f44f7940] [c00800000032b850] .alsa_card_pmac_init+0x80/0xc4 [snd_powermac]
[   85.063106] [c0000000f44f7a10] [c000000000010e58] .do_one_initcall+0x88/0x448
[   85.063158] [c0000000f44f7b00] [c0000000002416e4] .do_init_module+0x74/0x2e0
[   85.063203] [c0000000f44f7ba0] [c000000000243bd4] .load_module+0x20b4/0x26d0
[   85.063219] [c0000000f44f7cf0] [c000000000244470] .__se_sys_finit_module+0xe0/0x140
[   85.063237] [c0000000f44f7e20] [c00000000000c46c] system_call+0x5c/0x70



I think we can just do:

void iounmap(volatile void __iomem *token)
{
	void *addr;

	if (!addr)
        	return;

	...

??

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index aa1bc5f8da90..af34554d19e8 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -165,6 +165,8 @@  int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
 #define IOREMAP_TOP	KVIRT_TOP
 #endif
 
+#define IOREMAP_BASE	VMALLOC_START
+
 /*
  * Just any arbitrary offset to the start of the vmalloc VM area: the
  * current 16MB value just means that there will be a 64MB "hole" after the
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 23e5d5d16c7e..02d6256fe1ea 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -712,9 +712,6 @@  static inline void iosync(void)
  * * __ioremap_caller is the same as above but takes an explicit caller
  *   reference rather than using __builtin_return_address(0)
  *
- * * __iounmap, is the low level implementation used by iounmap and cannot
- *   be hooked (but can be used by a hook on iounmap)
- *
  */
 extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
 extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
@@ -734,8 +731,6 @@  extern void __iomem *__ioremap(phys_addr_t, unsigned long size,
 extern void __iomem *__ioremap_caller(phys_addr_t, unsigned long size,
 				      pgprot_t prot, void *caller);
 
-extern void __iounmap(volatile void __iomem *addr);
-
 extern void __iomem * __ioremap_at(phys_addr_t pa, void *ea,
 				   unsigned long size, pgprot_t prot);
 extern void __iounmap_at(void *ea, unsigned long size);
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index 7ce2a7c9fade..09f2739ab556 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -93,6 +93,8 @@  int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
 #define IOREMAP_TOP	KVIRT_TOP
 #endif
 
+#define IOREMAP_BASE	VMALLOC_START
+
 /*
  * Just any arbitrary offset to the start of the vmalloc VM area: the
  * current 16MB value just means that there will be a 64MB "hole" after the
diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
index 0c23660522ca..57d742509cec 100644
--- a/arch/powerpc/mm/ioremap.c
+++ b/arch/powerpc/mm/ioremap.c
@@ -1,7 +1,10 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
 #include <asm/io-workarounds.h>
+#include <mm/mmu_decl.h>
 
 unsigned long ioremap_bot;
 EXPORT_SYMBOL(ioremap_bot);
@@ -72,3 +75,31 @@  void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long f
 	return __ioremap_caller(addr, size, pte_pgprot(pte), caller);
 }
 EXPORT_SYMBOL(ioremap_prot);
+
+/*
+ * Unmap an IO region and remove it from vmalloc'd list.
+ * Access to IO memory should be serialized by driver.
+ */
+void iounmap(volatile void __iomem *token)
+{
+	void *addr;
+
+	/*
+	 * If mapped by BATs then there is nothing to do.
+	 */
+	if (v_block_mapped((unsigned long)token))
+		return;
+
+	if (!slab_is_available())
+		return;
+
+	addr = (void *)((unsigned long __force)PCI_FIX_ADDR(token) & PAGE_MASK);
+	if (WARN_ON((unsigned long)addr < IOREMAP_BASE))
+		return;
+	if ((unsigned long)addr >= ioremap_bot) {
+		pr_warn("Attempt to %s early bolted mapping at 0x%p\n", __func__, addr);
+		return;
+	}
+	vunmap(addr);
+}
+EXPORT_SYMBOL(iounmap);
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 7efdb1dee19b..4597f45e4dc6 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -110,20 +110,6 @@  __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
 	return (void __iomem *) (v + ((unsigned long)addr & ~PAGE_MASK));
 }
 
-void iounmap(volatile void __iomem *addr)
-{
-	/*
-	 * If mapped by BATs then there is nothing to do.
-	 * Calling vfree() generates a benign warning.
-	 */
-	if (v_block_mapped((unsigned long)addr))
-		return;
-
-	if (addr > high_memory && (unsigned long) addr < ioremap_bot)
-		vunmap((void *) (PAGE_MASK & (unsigned long)addr));
-}
-EXPORT_SYMBOL(iounmap);
-
 static void __init *early_alloc_pgtable(unsigned long size)
 {
 	void *ptr = memblock_alloc(size, size);
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index d631659c8859..b50a53a0a42b 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -201,35 +201,7 @@  void __iomem * __ioremap_caller(phys_addr_t addr, unsigned long size,
 	return ret;
 }
 
-/*  
- * Unmap an IO region and remove it from imalloc'd list.
- * Access to IO memory should be serialized by driver.
- */
-void __iounmap(volatile void __iomem *token)
-{
-	void *addr;
-
-	if (!slab_is_available())
-		return;
-	
-	addr = (void *) ((unsigned long __force)
-			 PCI_FIX_ADDR(token) & PAGE_MASK);
-	if ((unsigned long)addr >= ioremap_bot) {
-		printk(KERN_WARNING "Attempt to iounmap early bolted mapping"
-		       " at 0x%p\n", addr);
-		return;
-	}
-	vunmap(addr);
-}
-
-void iounmap(volatile void __iomem *token)
-{
-	__iounmap(token);
-}
-
 EXPORT_SYMBOL(__ioremap_at);
-EXPORT_SYMBOL(iounmap);
-EXPORT_SYMBOL(__iounmap);
 EXPORT_SYMBOL(__iounmap_at);
 
 #ifndef __PAGETABLE_PUD_FOLDED