* [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush
@ 2017-10-10 10:16 Nicholas Piggin
2017-10-10 10:22 ` Aneesh Kumar K.V
0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Piggin @ 2017-10-10 10:16 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/tlb-radix.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index b3e849c4886e..de414460287a 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
unsigned long ap = mmu_get_ap(mmu_virtual_psize);
unsigned long pid, end;
-
+ preempt_disable();
pid = mm ? mm->context.id : 0;
if (unlikely(pid == MMU_NO_CONTEXT))
goto no_context;
@@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
/* 4k page size, just blow the world */
if (PAGE_SIZE == 0x1000) {
radix__flush_all_mm(mm);
+ preempt_enable();
return;
}
--
2.13.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush
2017-10-10 10:16 [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
@ 2017-10-10 10:22 ` Aneesh Kumar K.V
2017-10-10 10:32 ` Nicholas Piggin
0 siblings, 1 reply; 5+ messages in thread
From: Aneesh Kumar K.V @ 2017-10-10 10:22 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
On 10/10/2017 03:46 PM, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/tlb-radix.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index b3e849c4886e..de414460287a 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> unsigned long ap = mmu_get_ap(mmu_virtual_psize);
> unsigned long pid, end;
>
> -
> + preempt_disable();
> pid = mm ? mm->context.id : 0;
> if (unlikely(pid == MMU_NO_CONTEXT))
> goto no_context;
> @@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> /* 4k page size, just blow the world */
> if (PAGE_SIZE == 0x1000) {
> radix__flush_all_mm(mm);
> + preempt_enable();
> return;
> }
>
Can't we do a preempt_disable before the if (local) check?. That way we
don't need that prempt_enable in that PAGE_SIZE==0x1000 path.We already
do disable/enable correctly in radix__flush_all_mm(mm)
-aneesh
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush
2017-10-10 10:22 ` Aneesh Kumar K.V
@ 2017-10-10 10:32 ` Nicholas Piggin
2017-10-10 13:39 ` Aneesh Kumar K.V
0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Piggin @ 2017-10-10 10:32 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev
On Tue, 10 Oct 2017 15:52:02 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On 10/10/2017 03:46 PM, Nicholas Piggin wrote:
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/mm/tlb-radix.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> > index b3e849c4886e..de414460287a 100644
> > --- a/arch/powerpc/mm/tlb-radix.c
> > +++ b/arch/powerpc/mm/tlb-radix.c
> > @@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> > unsigned long ap = mmu_get_ap(mmu_virtual_psize);
> > unsigned long pid, end;
> >
> > -
> > + preempt_disable();
> > pid = mm ? mm->context.id : 0;
> > if (unlikely(pid == MMU_NO_CONTEXT))
> > goto no_context;
> > @@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> > /* 4k page size, just blow the world */
> > if (PAGE_SIZE == 0x1000) {
> > radix__flush_all_mm(mm);
> > + preempt_enable();
> > return;
> > }
> >
>
> Can't we do a preempt_disable before the if (local) check?. That way we
> don't need that prempt_enable in that PAGE_SIZE==0x1000 path.We already
> do disable/enable correctly in radix__flush_all_mm(mm)
Well this is just to fix the imbalance. Nested preempt doesn't matter
much, and these are all no-ops for !preempt kernel, unless you turn on
debugging.
I already proposed another patch to bring those local tests under
preempt disable but no response yet
https://patchwork.ozlabs.org/patch/811061/
Thanks,
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush
2017-10-10 10:32 ` Nicholas Piggin
@ 2017-10-10 13:39 ` Aneesh Kumar K.V
2017-10-10 14:09 ` Nicholas Piggin
0 siblings, 1 reply; 5+ messages in thread
From: Aneesh Kumar K.V @ 2017-10-10 13:39 UTC (permalink / raw)
To: linuxppc-dev, Nicholas Piggin
On 10/10/2017 04:02 PM, Nicholas Piggin wrote:
> On Tue, 10 Oct 2017 15:52:02 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> On 10/10/2017 03:46 PM, Nicholas Piggin wrote:
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> arch/powerpc/mm/tlb-radix.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
>>> index b3e849c4886e..de414460287a 100644
>>> --- a/arch/powerpc/mm/tlb-radix.c
>>> +++ b/arch/powerpc/mm/tlb-radix.c
>>> @@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
>>> unsigned long ap = mmu_get_ap(mmu_virtual_psize);
>>> unsigned long pid, end;
>>>
>>> -
>>> + preempt_disable();
>>> pid = mm ? mm->context.id : 0;
>>> if (unlikely(pid == MMU_NO_CONTEXT))
>>> goto no_context;
>>> @@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
>>> /* 4k page size, just blow the world */
>>> if (PAGE_SIZE == 0x1000) {
>>> radix__flush_all_mm(mm);
>>> + preempt_enable();
>>> return;
>>> }
>>>
>> Can't we do a preempt_disable before the if (local) check?. That way we
>> don't need that prempt_enable in that PAGE_SIZE==0x1000 path.We already
>> do disable/enable correctly in radix__flush_all_mm(mm)
> Well this is just to fix the imbalance. Nested preempt doesn't matter
> much, and these are all no-ops for !preempt kernel, unless you turn on
> debugging.
But this patch is still doing the mm_is_thread_local() test outside
preempt_disable() right?
> I already proposed another patch to bring those local tests under
> preempt disable but no response yet
>
> https://patchwork.ozlabs.org/patch/811061/
>
That is a much better patch?
-aneesh
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush
2017-10-10 13:39 ` Aneesh Kumar K.V
@ 2017-10-10 14:09 ` Nicholas Piggin
0 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2017-10-10 14:09 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev
On Tue, 10 Oct 2017 19:09:54 +0530
"Aneesh Kumar K.V" <aneeshkumar.opensource@gmail.com> wrote:
>
>
> On 10/10/2017 04:02 PM, Nicholas Piggin wrote:
> > On Tue, 10 Oct 2017 15:52:02 +0530
> > "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >
> >> On 10/10/2017 03:46 PM, Nicholas Piggin wrote:
> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>> ---
> >>> arch/powerpc/mm/tlb-radix.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> >>> index b3e849c4886e..de414460287a 100644
> >>> --- a/arch/powerpc/mm/tlb-radix.c
> >>> +++ b/arch/powerpc/mm/tlb-radix.c
> >>> @@ -358,7 +358,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> >>> unsigned long ap = mmu_get_ap(mmu_virtual_psize);
> >>> unsigned long pid, end;
> >>>
> >>> -
> >>> + preempt_disable();
> >>> pid = mm ? mm->context.id : 0;
> >>> if (unlikely(pid == MMU_NO_CONTEXT))
> >>> goto no_context;
> >>> @@ -366,6 +366,7 @@ void radix__flush_tlb_collapsed_pmd(struct mm_struct *mm, unsigned long addr)
> >>> /* 4k page size, just blow the world */
> >>> if (PAGE_SIZE == 0x1000) {
> >>> radix__flush_all_mm(mm);
> >>> + preempt_enable();
> >>> return;
> >>> }
> >>>
> >> Can't we do a preempt_disable before the if (local) check?. That way we
> >> don't need that prempt_enable in that PAGE_SIZE==0x1000 path.We already
> >> do disable/enable correctly in radix__flush_all_mm(mm)
> > Well this is just to fix the imbalance. Nested preempt doesn't matter
> > much, and these are all no-ops for !preempt kernel, unless you turn on
> > debugging.
>
> But this patch is still doing the mm_is_thread_local() test outside
> preempt_disable() right?
Yes. As it does in some other place too. It's just a minimal fix for
the imbalance issue as I said, because that's messing up debugging.
> > I already proposed another patch to bring those local tests under
> > preempt disable but no response yet
> >
> > https://patchwork.ozlabs.org/patch/811061/
> >
>
> That is a much better patch?
I'm planning to repost the series but have been side-tracked hitting
testing it due to hitting bugs (!preempt though, so this has not been
top priority).
Thanks,
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-10 14:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-10 10:16 [PATCH] powerpc/64s/radix: fix preempt imbalance in TLB flush Nicholas Piggin
2017-10-10 10:22 ` Aneesh Kumar K.V
2017-10-10 10:32 ` Nicholas Piggin
2017-10-10 13:39 ` Aneesh Kumar K.V
2017-10-10 14:09 ` Nicholas Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).