* [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
@ 2013-09-17 9:43 ` Markos Chandras
0 siblings, 0 replies; 7+ messages in thread
From: Markos Chandras @ 2013-09-17 9:43 UTC (permalink / raw
To: linux-mips; +Cc: Markos Chandras
The cache flushing code uses the current_cpu_data macro which
may cause problems in preemptive kernels because it relies on
smp_processor_id() to get the current cpu number. Per cpu-data
needs to be protected so we disable preemption around the flush
caching code. We enable it back when we are about to return.
Fixes the following problem:
BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
caller is blast_dcache32+0x30/0x254
Call Trace:
[<8047f02c>] dump_stack+0x8/0x34
[<802e7e40>] debug_smp_processor_id+0xe0/0xf0
[<80114d94>] blast_dcache32+0x30/0x254
[<80118484>] r4k_dma_cache_wback_inv+0x200/0x288
[<80110ff0>] mips_dma_map_sg+0x108/0x180
[<80355098>] ide_dma_prepare+0xf0/0x1b8
[<8034eaa4>] do_rw_taskfile+0x1e8/0x33c
[<8035951c>] ide_do_rw_disk+0x298/0x3e4
[<8034a3c4>] do_ide_request+0x2e0/0x704
[<802bb0dc>] __blk_run_queue+0x44/0x64
[<802be000>] queue_unplugged.isra.36+0x1c/0x54
[<802beb94>] blk_flush_plug_list+0x18c/0x24c
[<802bec6c>] blk_finish_plug+0x18/0x48
[<8026554c>] journal_commit_transaction+0x3b8/0x151c
[<80269648>] kjournald+0xec/0x238
[<8014ac00>] kthread+0xb8/0xc0
[<8010268c>] ret_from_kernel_thread+0x14/0x1c
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
This patch is for the upstream-sfr/mips-for-linux-next tree
---
arch/mips/include/asm/r4kcache.h | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/mips/include/asm/r4kcache.h b/arch/mips/include/asm/r4kcache.h
index a0b2650..8f1a6a1 100644
--- a/arch/mips/include/asm/r4kcache.h
+++ b/arch/mips/include/asm/r4kcache.h
@@ -344,18 +344,21 @@ static inline void invalidate_tcache_page(unsigned long addr)
static inline void blast_##pfx##cache##lsize(void) \
{ \
unsigned long start = INDEX_BASE; \
- unsigned long end = start + current_cpu_data.desc.waysize; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
- unsigned long ws, addr; \
+ unsigned long end, ws_inc, ws_end, ws, addr; \
\
__##pfx##flush_prologue \
+ preempt_disable(); \
+ \
+ end = start + current_cpu_data.desc.waysize; \
+ ws_inc = 1UL << current_cpu_data.desc.waybit; \
+ ws_end = current_cpu_data.desc.ways << \
+ current_cpu_data.desc.waybit; \
\
for (ws = 0; ws < ws_end; ws += ws_inc) \
for (addr = start; addr < end; addr += lsize * 32) \
cache##lsize##_unroll32(addr|ws, indexop); \
\
+ preempt_enable(); \
__##pfx##flush_epilogue \
} \
\
@@ -376,20 +379,23 @@ static inline void blast_##pfx##cache##lsize##_page(unsigned long page) \
\
static inline void blast_##pfx##cache##lsize##_page_indexed(unsigned long page) \
{ \
- unsigned long indexmask = current_cpu_data.desc.waysize - 1; \
- unsigned long start = INDEX_BASE + (page & indexmask); \
- unsigned long end = start + PAGE_SIZE; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
- unsigned long ws, addr; \
+ unsigned long indexmask, end, start, ws_inc, ws_end, ws, addr; \
\
__##pfx##flush_prologue \
+ preempt_disable(); \
+ \
+ indexmask = current_cpu_data.desc.waysize - 1; \
+ start = INDEX_BASE + (page & indexmask); \
+ end = start + PAGE_SIZE; \
+ ws_inc = 1UL << current_cpu_data.desc.waybit; \
+ ws_end = current_cpu_data.desc.ways << \
+ current_cpu_data.desc.waybit; \
\
for (ws = 0; ws < ws_end; ws += ws_inc) \
for (addr = start; addr < end; addr += lsize * 32) \
cache##lsize##_unroll32(addr|ws, indexop); \
\
+ preempt_enable(); \
__##pfx##flush_epilogue \
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
@ 2013-09-17 9:43 ` Markos Chandras
0 siblings, 0 replies; 7+ messages in thread
From: Markos Chandras @ 2013-09-17 9:43 UTC (permalink / raw
To: linux-mips; +Cc: Markos Chandras
The cache flushing code uses the current_cpu_data macro which
may cause problems in preemptive kernels because it relies on
smp_processor_id() to get the current cpu number. Per cpu-data
needs to be protected so we disable preemption around the flush
caching code. We enable it back when we are about to return.
Fixes the following problem:
BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
caller is blast_dcache32+0x30/0x254
Call Trace:
[<8047f02c>] dump_stack+0x8/0x34
[<802e7e40>] debug_smp_processor_id+0xe0/0xf0
[<80114d94>] blast_dcache32+0x30/0x254
[<80118484>] r4k_dma_cache_wback_inv+0x200/0x288
[<80110ff0>] mips_dma_map_sg+0x108/0x180
[<80355098>] ide_dma_prepare+0xf0/0x1b8
[<8034eaa4>] do_rw_taskfile+0x1e8/0x33c
[<8035951c>] ide_do_rw_disk+0x298/0x3e4
[<8034a3c4>] do_ide_request+0x2e0/0x704
[<802bb0dc>] __blk_run_queue+0x44/0x64
[<802be000>] queue_unplugged.isra.36+0x1c/0x54
[<802beb94>] blk_flush_plug_list+0x18c/0x24c
[<802bec6c>] blk_finish_plug+0x18/0x48
[<8026554c>] journal_commit_transaction+0x3b8/0x151c
[<80269648>] kjournald+0xec/0x238
[<8014ac00>] kthread+0xb8/0xc0
[<8010268c>] ret_from_kernel_thread+0x14/0x1c
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
This patch is for the upstream-sfr/mips-for-linux-next tree
---
arch/mips/include/asm/r4kcache.h | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/mips/include/asm/r4kcache.h b/arch/mips/include/asm/r4kcache.h
index a0b2650..8f1a6a1 100644
--- a/arch/mips/include/asm/r4kcache.h
+++ b/arch/mips/include/asm/r4kcache.h
@@ -344,18 +344,21 @@ static inline void invalidate_tcache_page(unsigned long addr)
static inline void blast_##pfx##cache##lsize(void) \
{ \
unsigned long start = INDEX_BASE; \
- unsigned long end = start + current_cpu_data.desc.waysize; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
- unsigned long ws, addr; \
+ unsigned long end, ws_inc, ws_end, ws, addr; \
\
__##pfx##flush_prologue \
+ preempt_disable(); \
+ \
+ end = start + current_cpu_data.desc.waysize; \
+ ws_inc = 1UL << current_cpu_data.desc.waybit; \
+ ws_end = current_cpu_data.desc.ways << \
+ current_cpu_data.desc.waybit; \
\
for (ws = 0; ws < ws_end; ws += ws_inc) \
for (addr = start; addr < end; addr += lsize * 32) \
cache##lsize##_unroll32(addr|ws, indexop); \
\
+ preempt_enable(); \
__##pfx##flush_epilogue \
} \
\
@@ -376,20 +379,23 @@ static inline void blast_##pfx##cache##lsize##_page(unsigned long page) \
\
static inline void blast_##pfx##cache##lsize##_page_indexed(unsigned long page) \
{ \
- unsigned long indexmask = current_cpu_data.desc.waysize - 1; \
- unsigned long start = INDEX_BASE + (page & indexmask); \
- unsigned long end = start + PAGE_SIZE; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
- unsigned long ws, addr; \
+ unsigned long indexmask, end, start, ws_inc, ws_end, ws, addr; \
\
__##pfx##flush_prologue \
+ preempt_disable(); \
+ \
+ indexmask = current_cpu_data.desc.waysize - 1; \
+ start = INDEX_BASE + (page & indexmask); \
+ end = start + PAGE_SIZE; \
+ ws_inc = 1UL << current_cpu_data.desc.waybit; \
+ ws_end = current_cpu_data.desc.ways << \
+ current_cpu_data.desc.waybit; \
\
for (ws = 0; ws < ws_end; ws += ws_inc) \
for (addr = start; addr < end; addr += lsize * 32) \
cache##lsize##_unroll32(addr|ws, indexop); \
\
+ preempt_enable(); \
__##pfx##flush_epilogue \
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
2013-09-17 9:43 ` Markos Chandras
(?)
@ 2013-09-17 10:44 ` Ralf Baechle
2013-09-17 10:55 ` Markos Chandras
-1 siblings, 1 reply; 7+ messages in thread
From: Ralf Baechle @ 2013-09-17 10:44 UTC (permalink / raw
To: Markos Chandras; +Cc: linux-mips
On Tue, Sep 17, 2013 at 10:43:25AM +0100, Markos Chandras wrote:
> The cache flushing code uses the current_cpu_data macro which
> may cause problems in preemptive kernels because it relies on
> smp_processor_id() to get the current cpu number. Per cpu-data
> needs to be protected so we disable preemption around the flush
> caching code. We enable it back when we are about to return.
>
> Fixes the following problem:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
> caller is blast_dcache32+0x30/0x254
Just what I feared - these messages popping out from all over the tree.
I'd prefer if we change the caller otherwise depending on the platform
a single cache flush might involve several preempt_disable/-enable
invocations. Something like below.
And it also keeps the header file more usable outside the core kernel
which Florian's recent zboot a little easier.
However maybe we'd be even better off to just switch to boot_cpu_data.
That should be fine since r4k_dma_cache_* are only being used on
uniprocessor systems anyway.
Ralf
arch/mips/mm/c-r4k.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 3ff2f74..73ca8c5 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -12,6 +12,7 @@
#include <linux/highmem.h>
#include <linux/kernel.h>
#include <linux/linkage.h>
+#include <linux/preempt.h>
#include <linux/sched.h>
#include <linux/smp.h>
#include <linux/mm.h>
@@ -602,6 +603,7 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)
/* Catch bad driver code */
BUG_ON(size == 0);
+ preempt_disable();
if (cpu_has_inclusive_pcaches) {
if (size >= scache_size)
r4k_blast_scache();
@@ -622,6 +624,7 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)
R4600_HIT_CACHEOP_WAR_IMPL;
blast_dcache_range(addr, addr + size);
}
+ preempt_enable();
bc_wback_inv(addr, size);
__sync();
@@ -632,6 +635,7 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size)
/* Catch bad driver code */
BUG_ON(size == 0);
+ preempt_disable();
if (cpu_has_inclusive_pcaches) {
if (size >= scache_size)
r4k_blast_scache();
@@ -656,6 +660,7 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size)
R4600_HIT_CACHEOP_WAR_IMPL;
blast_inv_dcache_range(addr, addr + size);
}
+ preempt_enable();
bc_inv(addr, size);
__sync();
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
@ 2013-09-17 10:55 ` Markos Chandras
0 siblings, 0 replies; 7+ messages in thread
From: Markos Chandras @ 2013-09-17 10:55 UTC (permalink / raw
To: Ralf Baechle; +Cc: linux-mips
On 09/17/13 11:44, Ralf Baechle wrote:
> On Tue, Sep 17, 2013 at 10:43:25AM +0100, Markos Chandras wrote:
>
>> The cache flushing code uses the current_cpu_data macro which
>> may cause problems in preemptive kernels because it relies on
>> smp_processor_id() to get the current cpu number. Per cpu-data
>> needs to be protected so we disable preemption around the flush
>> caching code. We enable it back when we are about to return.
>>
>> Fixes the following problem:
>>
>> BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
>> caller is blast_dcache32+0x30/0x254
>
> Just what I feared - these messages popping out from all over the tree.
>
> I'd prefer if we change the caller otherwise depending on the platform
> a single cache flush might involve several preempt_disable/-enable
> invocations. Something like below.
>
> And it also keeps the header file more usable outside the core kernel
> which Florian's recent zboot a little easier.
>
Hi Ralf,
Changing the caller instead of the function in the header file looks
good to me. Thanks for fixing it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
@ 2013-09-17 10:55 ` Markos Chandras
0 siblings, 0 replies; 7+ messages in thread
From: Markos Chandras @ 2013-09-17 10:55 UTC (permalink / raw
To: Ralf Baechle; +Cc: linux-mips
On 09/17/13 11:44, Ralf Baechle wrote:
> On Tue, Sep 17, 2013 at 10:43:25AM +0100, Markos Chandras wrote:
>
>> The cache flushing code uses the current_cpu_data macro which
>> may cause problems in preemptive kernels because it relies on
>> smp_processor_id() to get the current cpu number. Per cpu-data
>> needs to be protected so we disable preemption around the flush
>> caching code. We enable it back when we are about to return.
>>
>> Fixes the following problem:
>>
>> BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
>> caller is blast_dcache32+0x30/0x254
>
> Just what I feared - these messages popping out from all over the tree.
>
> I'd prefer if we change the caller otherwise depending on the platform
> a single cache flush might involve several preempt_disable/-enable
> invocations. Something like below.
>
> And it also keeps the header file more usable outside the core kernel
> which Florian's recent zboot a little easier.
>
Hi Ralf,
Changing the caller instead of the function in the header file looks
good to me. Thanks for fixing it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
2013-09-17 10:55 ` Markos Chandras
(?)
@ 2013-09-17 11:43 ` Ralf Baechle
2013-09-17 16:02 ` Ralf Baechle
-1 siblings, 1 reply; 7+ messages in thread
From: Ralf Baechle @ 2013-09-17 11:43 UTC (permalink / raw
To: Markos Chandras; +Cc: linux-mips
On Tue, Sep 17, 2013 at 11:55:55AM +0100, Markos Chandras wrote:
> On 09/17/13 11:44, Ralf Baechle wrote:
> >On Tue, Sep 17, 2013 at 10:43:25AM +0100, Markos Chandras wrote:
> >
> >>The cache flushing code uses the current_cpu_data macro which
> >>may cause problems in preemptive kernels because it relies on
> >>smp_processor_id() to get the current cpu number. Per cpu-data
> >>needs to be protected so we disable preemption around the flush
> >>caching code. We enable it back when we are about to return.
> >>
> >>Fixes the following problem:
> >>
> >>BUG: using smp_processor_id() in preemptible [00000000] code: kjournald/1761
> >>caller is blast_dcache32+0x30/0x254
> >
> >Just what I feared - these messages popping out from all over the tree.
> >
> >I'd prefer if we change the caller otherwise depending on the platform
> >a single cache flush might involve several preempt_disable/-enable
> >invocations. Something like below.
> >
> >And it also keeps the header file more usable outside the core kernel
> >which Florian's recent zboot a little easier.
> >
>
> Hi Ralf,
>
> Changing the caller instead of the function in the header file looks
> good to me. Thanks for fixing it.
I think in the end the patch below is the better way of fixing it.
Ralf
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
arch/mips/include/asm/cpu-info.h | 1 +
arch/mips/include/asm/r4kcache.h | 16 ++++++++--------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index 41401d8..21c8e29 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -84,6 +84,7 @@ struct cpuinfo_mips {
extern struct cpuinfo_mips cpu_data[];
#define current_cpu_data cpu_data[smp_processor_id()]
#define raw_current_cpu_data cpu_data[raw_smp_processor_id()]
+#define boot_cpu_data cpu_data[0]
extern void cpu_probe(void);
extern void cpu_report(void);
diff --git a/arch/mips/include/asm/r4kcache.h b/arch/mips/include/asm/r4kcache.h
index a0b2650..be52f27 100644
--- a/arch/mips/include/asm/r4kcache.h
+++ b/arch/mips/include/asm/r4kcache.h
@@ -344,10 +344,10 @@ static inline void invalidate_tcache_page(unsigned long addr)
static inline void blast_##pfx##cache##lsize(void) \
{ \
unsigned long start = INDEX_BASE; \
- unsigned long end = start + current_cpu_data.desc.waysize; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
+ unsigned long end = start + boot_cpu_data.desc.waysize; \
+ unsigned long ws_inc = 1UL << boot_cpu_data.desc.waybit; \
+ unsigned long ws_end = boot_cpu_data.desc.ways << \
+ boot_cpu_data.desc.waybit; \
unsigned long ws, addr; \
\
__##pfx##flush_prologue \
@@ -376,12 +376,12 @@ static inline void blast_##pfx##cache##lsize##_page(unsigned long page) \
\
static inline void blast_##pfx##cache##lsize##_page_indexed(unsigned long page) \
{ \
- unsigned long indexmask = current_cpu_data.desc.waysize - 1; \
+ unsigned long indexmask = boot_cpu_data.desc.waysize - 1; \
unsigned long start = INDEX_BASE + (page & indexmask); \
unsigned long end = start + PAGE_SIZE; \
- unsigned long ws_inc = 1UL << current_cpu_data.desc.waybit; \
- unsigned long ws_end = current_cpu_data.desc.ways << \
- current_cpu_data.desc.waybit; \
+ unsigned long ws_inc = 1UL << boot_cpu_data.desc.waybit; \
+ unsigned long ws_end = boot_cpu_data.desc.ways << \
+ boot_cpu_data.desc.waybit; \
unsigned long ws, addr; \
\
__##pfx##flush_prologue \
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache
2013-09-17 11:43 ` Ralf Baechle
@ 2013-09-17 16:02 ` Ralf Baechle
0 siblings, 0 replies; 7+ messages in thread
From: Ralf Baechle @ 2013-09-17 16:02 UTC (permalink / raw
To: Markos Chandras; +Cc: linux-mips
On Tue, Sep 17, 2013 at 01:43:56PM +0200, Ralf Baechle wrote:
> > >I'd prefer if we change the caller otherwise depending on the platform
> > >a single cache flush might involve several preempt_disable/-enable
> > >invocations. Something like below.
> > >
> > >And it also keeps the header file more usable outside the core kernel
> > >which Florian's recent zboot a little easier.
> > >
> >
> > Hi Ralf,
> >
> > Changing the caller instead of the function in the header file looks
> > good to me. Thanks for fixing it.
>
> I think in the end the patch below is the better way of fixing it.
No, it's not. Most systems have identical caches for all processors
in a system but there are exceptions, so my first patch is the right
one.
Ralf
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-17 16:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 9:43 [PATCH] MIPS: Fix accessing to per-cpu data when flushing the cache Markos Chandras
2013-09-17 9:43 ` Markos Chandras
2013-09-17 10:44 ` Ralf Baechle
2013-09-17 10:55 ` Markos Chandras
2013-09-17 10:55 ` Markos Chandras
2013-09-17 11:43 ` Ralf Baechle
2013-09-17 16:02 ` Ralf Baechle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.