| Message ID | 20230706010816.72682-1-bgray@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 86582e6189dd8f9f52c25d46c70fe5d111da6345 |
| Headers | show |
| Series | powerpc/powermac: Use early_* IO variants in via_calibrate_decr | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
Le 06/07/2023 à 03:08, Benjamin Gray a écrit : > On a powermac platform, under the call path > > start_kernel > time_init > ppc_md.calibrate_decr (pmac_calibrate_decr) > via_calibrate_decr > > we run ioremap and iounmap. The unmap can enable interrupts > unexpectedly (cond_resched in vunmap_pmd_range), which is warned about > later in the boot sequence in start_kernel. > > Use the early_* variants of these IO functions to prevent this. > > The issue is pre-existing, but is surfaced by commit 721255b9826b > ("genirq: Use a maple tree for interrupt descriptor management"). > It's not clear to me why this causes it to surface. > > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/platforms/powermac/time.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/platforms/powermac/time.c b/arch/powerpc/platforms/powermac/time.c > index 4c5790aff1b5..8633891b7aa5 100644 > --- a/arch/powerpc/platforms/powermac/time.c > +++ b/arch/powerpc/platforms/powermac/time.c > @@ -26,8 +26,8 @@ > #include <linux/rtc.h> > #include <linux/of_address.h> > > +#include <asm/early_ioremap.h> > #include <asm/sections.h> > -#include <asm/io.h> > #include <asm/machdep.h> > #include <asm/time.h> > #include <asm/nvram.h> > @@ -182,7 +182,7 @@ static int __init via_calibrate_decr(void) > return 0; > } > of_node_put(vias); > - via = ioremap(rsrc.start, resource_size(&rsrc)); > + via = early_ioremap(rsrc.start, resource_size(&rsrc)); > if (via == NULL) { > printk(KERN_ERR "Failed to map VIA for timer calibration !\n"); > return 0; > @@ -207,7 +207,7 @@ static int __init via_calibrate_decr(void) > > ppc_tb_freq = (dstart - dend) * 100 / 6; > > - iounmap(via); > + early_iounmap((void *)via, resource_size(&rsrc)); > > return 1; > }
On Thu, 2023-07-06 at 11:08 +1000, Benjamin Gray wrote: > The issue is pre-existing, but is surfaced by commit 721255b9826b > ("genirq: Use a maple tree for interrupt descriptor management"). > It's not clear to me why this causes it to surface. From the thread chain in [1], it looks like the maple tree implementation just has different allocation behaviour, which follows a pre-existing code path to kmem_cache_alloc_bulk(), which unconditionally enables interrupts. (thanks Jordan Niethe for finding this thread) [1]: https://lore.kernel.org/all/87o7qdzfay.ffs@tglx/
Benjamin Gray <bgray@linux.ibm.com> writes: > On Thu, 2023-07-06 at 11:08 +1000, Benjamin Gray wrote: >> The issue is pre-existing, but is surfaced by commit 721255b9826b >> ("genirq: Use a maple tree for interrupt descriptor management"). >> It's not clear to me why this causes it to surface. > > From the thread chain in [1], it looks like the maple tree > implementation just has different allocation behaviour, which follows a > pre-existing code path to kmem_cache_alloc_bulk(), which > unconditionally enables interrupts. That was a bug that was fixed before the series was merged. See: f5451547b831 ("mm, slab/slub: Ensure kmem_cache_alloc_bulk() is available early") It looks like the trigger here is that the maple tree code uses call_rcu() in ma_free_rcu(), and call_rcu() can cause TIF_NEED_RESCHED to be set, which causes cond_resched() to actually reschedule, enabling interrupts. cheers > (thanks Jordan Niethe for finding this thread) > > [1]: https://lore.kernel.org/all/87o7qdzfay.ffs@tglx/
diff --git a/arch/powerpc/platforms/powermac/time.c b/arch/powerpc/platforms/powermac/time.c index 4c5790aff1b5..8633891b7aa5 100644 --- a/arch/powerpc/platforms/powermac/time.c +++ b/arch/powerpc/platforms/powermac/time.c @@ -26,8 +26,8 @@ #include <linux/rtc.h> #include <linux/of_address.h> +#include <asm/early_ioremap.h> #include <asm/sections.h> -#include <asm/io.h> #include <asm/machdep.h> #include <asm/time.h> #include <asm/nvram.h> @@ -182,7 +182,7 @@ static int __init via_calibrate_decr(void) return 0; } of_node_put(vias); - via = ioremap(rsrc.start, resource_size(&rsrc)); + via = early_ioremap(rsrc.start, resource_size(&rsrc)); if (via == NULL) { printk(KERN_ERR "Failed to map VIA for timer calibration !\n"); return 0; @@ -207,7 +207,7 @@ static int __init via_calibrate_decr(void) ppc_tb_freq = (dstart - dend) * 100 / 6; - iounmap(via); + early_iounmap((void *)via, resource_size(&rsrc)); return 1; }
On a powermac platform, under the call path start_kernel time_init ppc_md.calibrate_decr (pmac_calibrate_decr) via_calibrate_decr we run ioremap and iounmap. The unmap can enable interrupts unexpectedly (cond_resched in vunmap_pmd_range), which is warned about later in the boot sequence in start_kernel. Use the early_* variants of these IO functions to prevent this. The issue is pre-existing, but is surfaced by commit 721255b9826b ("genirq: Use a maple tree for interrupt descriptor management"). It's not clear to me why this causes it to surface. Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- arch/powerpc/platforms/powermac/time.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)