Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Thomas Gleixner wrote:
> On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
>> Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
>> adds a preempt_disable but no preempt_enable().
>>
>> Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
>
> Good catch. Applied.
Thomas could you apply this instead?
After discussions w/RMK and validation of my own on OMAP2 (ARMv6), it
seems that these disable/enable sections aren't necessary.
Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
reverted:
--- linux-2.6.21/include/asm-arm/tlbflush.h
+++ linux-2.6.21.orig/include/asm-arm/tlbflush.h
@@ -246,7 +246,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_WB))
dsb();
@@ -258,7 +257,6 @@
asm("mcr p15, 0, %0, c8, c6, 0" : : "r" (zero) : "cc");
if (tlb_flag(TLB_V4_I_FULL | TLB_V6_I_FULL))
asm("mcr p15, 0, %0, c8, c5, 0" : : "r" (zero) : "cc");
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -276,7 +274,6 @@
const int asid = ASID(mm);
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_WB))
dsb();
@@ -297,7 +294,6 @@
asm("mcr p15, 0, %0, c8, c6, 2" : : "r" (asid) : "cc");
if (tlb_flag(TLB_V6_I_ASID))
asm("mcr p15, 0, %0, c8, c5, 2" : : "r" (asid) : "cc");
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -314,7 +310,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
uaddr = (uaddr & PAGE_MASK) | ASID(vma->vm_mm);
if (tlb_flag(TLB_WB))
@@ -339,7 +334,6 @@
asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (uaddr) : "cc");
if (tlb_flag(TLB_V6_I_PAGE))
asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (uaddr) : "cc");
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -355,7 +349,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
kaddr &= PAGE_MASK;
if (tlb_flag(TLB_WB))
@@ -406,13 +399,11 @@
{
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_DCLEAN))
asm("mcrp15, 0, %0, c7, c10, 1 @ flush_pmd"
: : "r" (pmd) : "cc");
if (tlb_flag(TLB_WB))
dsb();
- preempt_enable();
}
static inline void clean_pmd_entry(pmd_t *pmd)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Russell King wrote: > On Wed, May 23, 2007 at 09:13:57AM -0700, Kevin Hilman wrote: >> On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote: >>> In which case shouldn't it be at the end of the function so it includes >>> the write buffer handling as well? >>> >>> However, I think I agree with Daniel on this one. I don't see the point >>> of the preempt_disable() here. >> Note that my patch simply adds an enable to match the disable added by >> the -rt patch. I'm not sure where the disable originally came from, but >> there are disable/enable pairs scattered throughout tlbflush.h in the >> -rt patch. >> >> If this one isn't necessary, then the others probably are not either. >> In most cases there are 2 mcr instructions inside the critical section. >> One for the dsb() and the other for the actual function. >> >> Russell, is there a reason any of these sections should be atomic? > > I don't see any reason for them to be - when switching to another process > we'll generally do a full TLB flush anyway, so what's the point in making > these flushes atomic? OK, I've removed the locally and will be doing some testing on OMAP2 (ARMv6.) I'll submit a patch to Ingo if things look good. In the meantime, my previous fix is still necessary for -rt to even work on ARM. Kevin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, 2007-05-23 at 09:13 -0700, Kevin Hilman wrote: > Note that my patch simply adds an enable to match the disable added by > the -rt patch. I'm not sure where the disable originally came from, but > there are disable/enable pairs scattered throughout tlbflush.h in the > -rt patch. > > If this one isn't necessary, then the others probably are not either. > In most cases there are 2 mcr instructions inside the critical section. > One for the dsb() and the other for the actual function. > > Russell, is there a reason any of these sections should be atomic? Under normal preempt modes those functions would end running with preemption disabled , but with PREEMPT_RT enabled they become preemptive .. I may have been mistaken , but my impression was that one mcr instruction was executed and it was atomic , so there was no need for additional protection there.. If there are two mcr instructions then you could be preempted between the two, which may be unsafe depending on what those instructions are doing .. Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, May 23, 2007 at 09:13:57AM -0700, Kevin Hilman wrote: > On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote: > > In which case shouldn't it be at the end of the function so it includes > > the write buffer handling as well? > > > > However, I think I agree with Daniel on this one. I don't see the point > > of the preempt_disable() here. > > Note that my patch simply adds an enable to match the disable added by > the -rt patch. I'm not sure where the disable originally came from, but > there are disable/enable pairs scattered throughout tlbflush.h in the > -rt patch. > > If this one isn't necessary, then the others probably are not either. > In most cases there are 2 mcr instructions inside the critical section. > One for the dsb() and the other for the actual function. > > Russell, is there a reason any of these sections should be atomic? I don't see any reason for them to be - when switching to another process we'll generally do a full TLB flush anyway, so what's the point in making these flushes atomic? Consider: flush_tlb_page() first mcr - invalidates tlb single entry --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- dsb That context switch is harmless - we end up with the entire TLB being invalidated and a DSB following. Now consider: flush_tlb_page() --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- preempt_disable() first mcr - invalidates tlb single entry dsb preempt_enable() Any difference? No. Without the preempt disable/enable fiddling? No. flush_tlb_page() preempt_disable() first mcr - invalidates tlb single entry dsb preempt_enable() --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- Any difference? No. Without the preempt disable/enable fiddling? No. In every case of a preemption occuring in the middle of a tlb operation, the ultimate result is identical irrespective of preempt control sprinkling. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote:
> On Tue, May 22, 2007 at 04:41:36PM -0700, Kevin Hilman wrote:
> > On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote:
> > > On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
> > > > Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
> > > > adds a preempt_disable but no preempt_enable().
> > > >
> > > > Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
> > > >
> > > >
> > > > ---
> > > > include/asm-arm/tlbflush.h |1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > Index: linux-2.6.21/include/asm-arm/tlbflush.h
> > > > ===
> > > > --- linux-2.6.21.orig/include/asm-arm/tlbflush.h
> > > > +++ linux-2.6.21/include/asm-arm/tlbflush.h
> > > > @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne
> > > > asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (kaddr) : "cc");
> > > > if (tlb_flag(TLB_V6_I_PAGE))
> > > > asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (kaddr) : "cc");
> > >
> > > Aren't these mcr operations atomic?
> > >
> >
> > Individually, yes. But the point of the preempt_disable/enable is to
> > make the whole sequence atomic.
>
> In which case shouldn't it be at the end of the function so it includes
> the write buffer handling as well?
>
> However, I think I agree with Daniel on this one. I don't see the point
> of the preempt_disable() here.
Note that my patch simply adds an enable to match the disable added by
the -rt patch. I'm not sure where the disable originally came from, but
there are disable/enable pairs scattered throughout tlbflush.h in the
-rt patch.
If this one isn't necessary, then the others probably are not either.
In most cases there are 2 mcr instructions inside the critical section.
One for the dsb() and the other for the actual function.
Russell, is there a reason any of these sections should be atomic?
Kevin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, May 22, 2007 at 04:41:36PM -0700, Kevin Hilman wrote:
> On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote:
> > On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
> > > Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
> > > adds a preempt_disable but no preempt_enable().
> > >
> > > Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
> > >
> > >
> > > ---
> > > include/asm-arm/tlbflush.h |1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > Index: linux-2.6.21/include/asm-arm/tlbflush.h
> > > ===
> > > --- linux-2.6.21.orig/include/asm-arm/tlbflush.h
> > > +++ linux-2.6.21/include/asm-arm/tlbflush.h
> > > @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne
> > > asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (kaddr) : "cc");
> > > if (tlb_flag(TLB_V6_I_PAGE))
> > > asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (kaddr) : "cc");
> >
> > Aren't these mcr operations atomic?
> >
>
> Individually, yes. But the point of the preempt_disable/enable is to
> make the whole sequence atomic.
In which case shouldn't it be at the end of the function so it includes
the write buffer handling as well?
However, I think I agree with Daniel on this one. I don't see the point
of the preempt_disable() here.
--
Russell King
Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, May 22, 2007 at 04:41:36PM -0700, Kevin Hilman wrote: On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote: On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] --- include/asm-arm/tlbflush.h |1 + 1 file changed, 1 insertion(+) Index: linux-2.6.21/include/asm-arm/tlbflush.h === --- linux-2.6.21.orig/include/asm-arm/tlbflush.h +++ linux-2.6.21/include/asm-arm/tlbflush.h @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne asm(mcr p15, 0, %0, c8, c6, 1 : : r (kaddr) : cc); if (tlb_flag(TLB_V6_I_PAGE)) asm(mcr p15, 0, %0, c8, c5, 1 : : r (kaddr) : cc); Aren't these mcr operations atomic? Individually, yes. But the point of the preempt_disable/enable is to make the whole sequence atomic. In which case shouldn't it be at the end of the function so it includes the write buffer handling as well? However, I think I agree with Daniel on this one. I don't see the point of the preempt_disable() here. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote: On Tue, May 22, 2007 at 04:41:36PM -0700, Kevin Hilman wrote: On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote: On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] --- include/asm-arm/tlbflush.h |1 + 1 file changed, 1 insertion(+) Index: linux-2.6.21/include/asm-arm/tlbflush.h === --- linux-2.6.21.orig/include/asm-arm/tlbflush.h +++ linux-2.6.21/include/asm-arm/tlbflush.h @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne asm(mcr p15, 0, %0, c8, c6, 1 : : r (kaddr) : cc); if (tlb_flag(TLB_V6_I_PAGE)) asm(mcr p15, 0, %0, c8, c5, 1 : : r (kaddr) : cc); Aren't these mcr operations atomic? Individually, yes. But the point of the preempt_disable/enable is to make the whole sequence atomic. In which case shouldn't it be at the end of the function so it includes the write buffer handling as well? However, I think I agree with Daniel on this one. I don't see the point of the preempt_disable() here. Note that my patch simply adds an enable to match the disable added by the -rt patch. I'm not sure where the disable originally came from, but there are disable/enable pairs scattered throughout tlbflush.h in the -rt patch. If this one isn't necessary, then the others probably are not either. In most cases there are 2 mcr instructions inside the critical section. One for the dsb() and the other for the actual function. Russell, is there a reason any of these sections should be atomic? Kevin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, May 23, 2007 at 09:13:57AM -0700, Kevin Hilman wrote: On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote: In which case shouldn't it be at the end of the function so it includes the write buffer handling as well? However, I think I agree with Daniel on this one. I don't see the point of the preempt_disable() here. Note that my patch simply adds an enable to match the disable added by the -rt patch. I'm not sure where the disable originally came from, but there are disable/enable pairs scattered throughout tlbflush.h in the -rt patch. If this one isn't necessary, then the others probably are not either. In most cases there are 2 mcr instructions inside the critical section. One for the dsb() and the other for the actual function. Russell, is there a reason any of these sections should be atomic? I don't see any reason for them to be - when switching to another process we'll generally do a full TLB flush anyway, so what's the point in making these flushes atomic? Consider: flush_tlb_page() first mcr - invalidates tlb single entry --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- dsb That context switch is harmless - we end up with the entire TLB being invalidated and a DSB following. Now consider: flush_tlb_page() --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- preempt_disable() first mcr - invalidates tlb single entry dsb preempt_enable() Any difference? No. Without the preempt disable/enable fiddling? No. flush_tlb_page() preempt_disable() first mcr - invalidates tlb single entry dsb preempt_enable() --- context switch, invalidates entire tlb, inc dsb --- something else runs --- context switch, invalidates entire tlb, inc dsb, again --- Any difference? No. Without the preempt disable/enable fiddling? No. In every case of a preemption occuring in the middle of a tlb operation, the ultimate result is identical irrespective of preempt control sprinkling. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Wed, 2007-05-23 at 09:13 -0700, Kevin Hilman wrote: Note that my patch simply adds an enable to match the disable added by the -rt patch. I'm not sure where the disable originally came from, but there are disable/enable pairs scattered throughout tlbflush.h in the -rt patch. If this one isn't necessary, then the others probably are not either. In most cases there are 2 mcr instructions inside the critical section. One for the dsb() and the other for the actual function. Russell, is there a reason any of these sections should be atomic? Under normal preempt modes those functions would end running with preemption disabled , but with PREEMPT_RT enabled they become preemptive .. I may have been mistaken , but my impression was that one mcr instruction was executed and it was atomic , so there was no need for additional protection there.. If there are two mcr instructions then you could be preempted between the two, which may be unsafe depending on what those instructions are doing .. Daniel - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Russell King wrote: On Wed, May 23, 2007 at 09:13:57AM -0700, Kevin Hilman wrote: On Wed, 2007-05-23 at 10:22 +0100, Russell King wrote: In which case shouldn't it be at the end of the function so it includes the write buffer handling as well? However, I think I agree with Daniel on this one. I don't see the point of the preempt_disable() here. Note that my patch simply adds an enable to match the disable added by the -rt patch. I'm not sure where the disable originally came from, but there are disable/enable pairs scattered throughout tlbflush.h in the -rt patch. If this one isn't necessary, then the others probably are not either. In most cases there are 2 mcr instructions inside the critical section. One for the dsb() and the other for the actual function. Russell, is there a reason any of these sections should be atomic? I don't see any reason for them to be - when switching to another process we'll generally do a full TLB flush anyway, so what's the point in making these flushes atomic? OK, I've removed the locally and will be doing some testing on OMAP2 (ARMv6.) I'll submit a patch to Ingo if things look good. In the meantime, my previous fix is still necessary for -rt to even work on ARM. Kevin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Thomas Gleixner wrote:
On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
adds a preempt_disable but no preempt_enable().
Signed-off-by: Kevin Hilman [EMAIL PROTECTED]
Good catch. Applied.
Thomas could you apply this instead?
After discussions w/RMK and validation of my own on OMAP2 (ARMv6), it
seems that these disable/enable sections aren't necessary.
Signed-off-by: Kevin Hilman [EMAIL PROTECTED]
reverted:
--- linux-2.6.21/include/asm-arm/tlbflush.h
+++ linux-2.6.21.orig/include/asm-arm/tlbflush.h
@@ -246,7 +246,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_WB))
dsb();
@@ -258,7 +257,6 @@
asm(mcr p15, 0, %0, c8, c6, 0 : : r (zero) : cc);
if (tlb_flag(TLB_V4_I_FULL | TLB_V6_I_FULL))
asm(mcr p15, 0, %0, c8, c5, 0 : : r (zero) : cc);
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -276,7 +274,6 @@
const int asid = ASID(mm);
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_WB))
dsb();
@@ -297,7 +294,6 @@
asm(mcr p15, 0, %0, c8, c6, 2 : : r (asid) : cc);
if (tlb_flag(TLB_V6_I_ASID))
asm(mcr p15, 0, %0, c8, c5, 2 : : r (asid) : cc);
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -314,7 +310,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
uaddr = (uaddr PAGE_MASK) | ASID(vma-vm_mm);
if (tlb_flag(TLB_WB))
@@ -339,7 +334,6 @@
asm(mcr p15, 0, %0, c8, c6, 1 : : r (uaddr) : cc);
if (tlb_flag(TLB_V6_I_PAGE))
asm(mcr p15, 0, %0, c8, c5, 1 : : r (uaddr) : cc);
- preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
@@ -355,7 +349,6 @@
const int zero = 0;
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
kaddr = PAGE_MASK;
if (tlb_flag(TLB_WB))
@@ -406,13 +399,11 @@
{
const unsigned int __tlb_flag = __cpu_tlb_flags;
- preempt_disable();
if (tlb_flag(TLB_DCLEAN))
asm(mcrp15, 0, %0, c7, c10, 1 @ flush_pmd
: : r (pmd) : cc);
if (tlb_flag(TLB_WB))
dsb();
- preempt_enable();
}
static inline void clean_pmd_entry(pmd_t *pmd)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: > Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch > adds a preempt_disable but no preempt_enable(). > > Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]> Good catch. Applied. Thanks, tglx - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:41 -0700, Kevin Hilman wrote: > > Individually, yes. But the point of the preempt_disable/enable is to > make the whole sequence atomic. I was under the impression that only one of those mcr lines is called per board type, the rest just compile away? Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote:
> On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
> > Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
> > adds a preempt_disable but no preempt_enable().
> >
> > Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
> >
> >
> > ---
> > include/asm-arm/tlbflush.h |1 +
> > 1 file changed, 1 insertion(+)
> >
> > Index: linux-2.6.21/include/asm-arm/tlbflush.h
> > ===
> > --- linux-2.6.21.orig/include/asm-arm/tlbflush.h
> > +++ linux-2.6.21/include/asm-arm/tlbflush.h
> > @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne
> > asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (kaddr) : "cc");
> > if (tlb_flag(TLB_V6_I_PAGE))
> > asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (kaddr) : "cc");
>
> Aren't these mcr operations atomic?
>
Individually, yes. But the point of the preempt_disable/enable is to
make the whole sequence atomic.
Kevin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote:
> Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
> adds a preempt_disable but no preempt_enable().
>
> Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
>
>
> ---
> include/asm-arm/tlbflush.h |1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux-2.6.21/include/asm-arm/tlbflush.h
> ===
> --- linux-2.6.21.orig/include/asm-arm/tlbflush.h
> +++ linux-2.6.21/include/asm-arm/tlbflush.h
> @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne
> asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (kaddr) : "cc");
> if (tlb_flag(TLB_V6_I_PAGE))
> asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (kaddr) : "cc");
Aren't these mcr operations atomic?
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch
adds a preempt_disable but no preempt_enable().
Signed-off-by: Kevin Hilman <[EMAIL PROTECTED]>
---
include/asm-arm/tlbflush.h |1 +
1 file changed, 1 insertion(+)
Index: linux-2.6.21/include/asm-arm/tlbflush.h
===
--- linux-2.6.21.orig/include/asm-arm/tlbflush.h
+++ linux-2.6.21/include/asm-arm/tlbflush.h
@@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne
asm("mcr p15, 0, %0, c8, c6, 1" : : "r" (kaddr) : "cc");
if (tlb_flag(TLB_V6_I_PAGE))
asm("mcr p15, 0, %0, c8, c5, 1" : : "r" (kaddr) : "cc");
+ preempt_enable();
if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL |
TLB_V6_I_PAGE | TLB_V6_D_PAGE |
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] --- include/asm-arm/tlbflush.h |1 + 1 file changed, 1 insertion(+) Index: linux-2.6.21/include/asm-arm/tlbflush.h === --- linux-2.6.21.orig/include/asm-arm/tlbflush.h +++ linux-2.6.21/include/asm-arm/tlbflush.h @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne asm(mcr p15, 0, %0, c8, c6, 1 : : r (kaddr) : cc); if (tlb_flag(TLB_V6_I_PAGE)) asm(mcr p15, 0, %0, c8, c5, 1 : : r (kaddr) : cc); + preempt_enable(); if (tlb_flag(TLB_V6_I_FULL | TLB_V6_D_FULL | TLB_V6_I_PAGE | TLB_V6_D_PAGE | -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] --- include/asm-arm/tlbflush.h |1 + 1 file changed, 1 insertion(+) Index: linux-2.6.21/include/asm-arm/tlbflush.h === --- linux-2.6.21.orig/include/asm-arm/tlbflush.h +++ linux-2.6.21/include/asm-arm/tlbflush.h @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne asm(mcr p15, 0, %0, c8, c6, 1 : : r (kaddr) : cc); if (tlb_flag(TLB_V6_I_PAGE)) asm(mcr p15, 0, %0, c8, c5, 1 : : r (kaddr) : cc); Aren't these mcr operations atomic? Daniel - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:25 -0700, Daniel Walker wrote: On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] --- include/asm-arm/tlbflush.h |1 + 1 file changed, 1 insertion(+) Index: linux-2.6.21/include/asm-arm/tlbflush.h === --- linux-2.6.21.orig/include/asm-arm/tlbflush.h +++ linux-2.6.21/include/asm-arm/tlbflush.h @@ -378,6 +378,7 @@ static inline void local_flush_tlb_kerne asm(mcr p15, 0, %0, c8, c6, 1 : : r (kaddr) : cc); if (tlb_flag(TLB_V6_I_PAGE)) asm(mcr p15, 0, %0, c8, c5, 1 : : r (kaddr) : cc); Aren't these mcr operations atomic? Individually, yes. But the point of the preempt_disable/enable is to make the whole sequence atomic. Kevin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:41 -0700, Kevin Hilman wrote: Individually, yes. But the point of the preempt_disable/enable is to make the whole sequence atomic. I was under the impression that only one of those mcr lines is called per board type, the rest just compile away? Daniel - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -rt] ARM TLB flush fix: don't forget to re-enable preemption
On Tue, 2007-05-22 at 16:01 -0700, Kevin Hilman wrote: Add a preempt_enable() to flush_tlb_kernel_page() since -rt4 patch adds a preempt_disable but no preempt_enable(). Signed-off-by: Kevin Hilman [EMAIL PROTECTED] Good catch. Applied. Thanks, tglx - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
