2020-01-31 15:38:53

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page

When flushing a range, the flushing function flushes all TLBs.

When the range is a single page, do a page flush instead.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/mm/book3s32/tlb.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 2fcd321040ff..8e0089065a7e 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -79,14 +79,17 @@ static void flush_range(struct mm_struct *mm, unsigned long start,
int count;
unsigned int ctx = mm->context.id;

+ start &= PAGE_MASK;
+ end = (end - 1) | ~PAGE_MASK;
if (!Hash) {
- _tlbia();
+ if (end - start == PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
return;
}
- start &= PAGE_MASK;
if (start >= end)
return;
- end = (end - 1) | ~PAGE_MASK;
pmd = pmd_offset(pud_offset(pgd_offset(mm, start), start), start);
for (;;) {
pmd_end = ((start + PGDIR_SIZE) & PGDIR_MASK) - 1;
--
2.25.0


2020-01-31 15:53:18

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page

On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
> When the range is a single page, do a page flush instead.

> + start &= PAGE_MASK;
> + end = (end - 1) | ~PAGE_MASK;
> if (!Hash) {
> - _tlbia();
> + if (end - start == PAGE_SIZE)
> + _tlbie(start);
> + else
> + _tlbia();
> return;
> }

For just one page, you get end - start == 0 actually?


Segher

2020-01-31 16:17:56

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page



Le 31/01/2020 à 16:51, Segher Boessenkool a écrit :
> On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
>> When the range is a single page, do a page flush instead.
>
>> + start &= PAGE_MASK;
>> + end = (end - 1) | ~PAGE_MASK;
>> if (!Hash) {
>> - _tlbia();
>> + if (end - start == PAGE_SIZE)
>> + _tlbie(start);
>> + else
>> + _tlbia();
>> return;
>> }
>
> For just one page, you get end - start == 0 actually?
>
>

Oops, good catch.

Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.

Christophe

2020-01-31 19:40:25

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page

On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
> Le 31/01/2020 ? 16:51, Segher Boessenkool a ?crit?:
> >On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
> >>When the range is a single page, do a page flush instead.
> >
> >>+ start &= PAGE_MASK;
> >>+ end = (end - 1) | ~PAGE_MASK;
> >> if (!Hash) {
> >>- _tlbia();
> >>+ if (end - start == PAGE_SIZE)
> >>+ _tlbie(start);
> >>+ else
> >>+ _tlbia();
> >> return;
> >> }
> >
> >For just one page, you get end - start == 0 actually?
>
> Oops, good catch.
>
> Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.

You have all low bits masked off in both start and end, so you get zero.
You could make the condion read "if (start == end)?

Maybe a nicer way to describe what you do is "if start and end are on the
same memory page, flush that page."


Segher

2020-02-01 07:28:17

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page



Le 31/01/2020 à 20:38, Segher Boessenkool a écrit :
> On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
>> Le 31/01/2020 à 16:51, Segher Boessenkool a écrit :
>>> On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
>>>> When the range is a single page, do a page flush instead.
>>>
>>>> + start &= PAGE_MASK;
>>>> + end = (end - 1) | ~PAGE_MASK;
>>>> if (!Hash) {
>>>> - _tlbia();
>>>> + if (end - start == PAGE_SIZE)
>>>> + _tlbie(start);
>>>> + else
>>>> + _tlbia();
>>>> return;
>>>> }
>>>
>>> For just one page, you get end - start == 0 actually?
>>
>> Oops, good catch.
>>
>> Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.
>
> You have all low bits masked off in both start and end, so you get zero.
> You could make the condion read "if (start == end)?

No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
sets all low bits to 1.

Christophe

2020-02-01 14:09:38

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page

On Sat, Feb 01, 2020 at 08:27:03AM +0100, Christophe Leroy wrote:
> Le 31/01/2020 ? 20:38, Segher Boessenkool a ?crit?:
> >On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
> >>Le 31/01/2020 ? 16:51, Segher Boessenkool a ?crit?:
> >>>On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
> >>>>When the range is a single page, do a page flush instead.
> >>>
> >>>>+ start &= PAGE_MASK;
> >>>>+ end = (end - 1) | ~PAGE_MASK;
> >>>> if (!Hash) {
> >>>>- _tlbia();
> >>>>+ if (end - start == PAGE_SIZE)
> >>>>+ _tlbie(start);
> >>>>+ else
> >>>>+ _tlbia();
> >>>> return;
> >>>> }
> >>>
> >>>For just one page, you get end - start == 0 actually?
> >>
> >>Oops, good catch.
> >>
> >>Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.
> >
> >You have all low bits masked off in both start and end, so you get zero.
> >You could make the condion read "if (start == end)?
>
> No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
> sets all low bits to 1.

Oh, wow, yes, I cannot read apparently.

Maybe there are some ROUND_DOWN and ROUND_UP macros you could use?


Segher

2020-02-01 14:55:25

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page



Le 01/02/2020 à 15:06, Segher Boessenkool a écrit :
> On Sat, Feb 01, 2020 at 08:27:03AM +0100, Christophe Leroy wrote:
>> Le 31/01/2020 à 20:38, Segher Boessenkool a écrit :
>>> On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
>>>> Le 31/01/2020 à 16:51, Segher Boessenkool a écrit :
>>>>> On Fri, Jan 31, 2020 at 03:37:34PM +0000, Christophe Leroy wrote:
>>>>>> When the range is a single page, do a page flush instead.
>>>>>
>>>>>> + start &= PAGE_MASK;
>>>>>> + end = (end - 1) | ~PAGE_MASK;
>>>>>> if (!Hash) {
>>>>>> - _tlbia();
>>>>>> + if (end - start == PAGE_SIZE)
>>>>>> + _tlbie(start);
>>>>>> + else
>>>>>> + _tlbia();
>>>>>> return;
>>>>>> }
>>>>>
>>>>> For just one page, you get end - start == 0 actually?
>>>>
>>>> Oops, good catch.
>>>>
>>>> Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.
>>>
>>> You have all low bits masked off in both start and end, so you get zero.
>>> You could make the condion read "if (start == end)?
>>
>> No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
>> sets all low bits to 1.
>
> Oh, wow, yes, I cannot read apparently.
>
> Maybe there are some ROUND_DOWN and ROUND_UP macros you could use?
>

Yes but my intention was to modify the existing code as less as possible.
What do you think about version v2 of the patch ?

Christophe

2020-02-01 16:19:38

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page

On Sat, Feb 01, 2020 at 03:53:12PM +0100, Christophe Leroy wrote:
> >>No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
> >>sets all low bits to 1.
> >
> >Oh, wow, yes, I cannot read apparently.
> >
> >Maybe there are some ROUND_DOWN and ROUND_UP macros you could use?
>
> Yes but my intention was to modify the existing code as less as possible.
> What do you think about version v2 of the patch ?

It looked fine to me.

Add my

Reviewed-by: Segher Boessenkool <[email protected]>

if you want.


Segher