2017-12-05 12:41:50

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 5/9] x86/uv: Use the right tlbflush API

Since uv_flush_tlb_others() implements flush_tlb_others() which is
about flushing user mappings, we should use __flush_tlb_single(),
which too is about flushing user mappings.

Cc: Andrew Banman <[email protected]>
Cc: Mike Travis <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/platform/uv/tlb_uv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -299,7 +299,7 @@ static void bau_process_message(struct m
local_flush_tlb();
stat->d_alltlb++;
} else {
- __flush_tlb_one(msg->address);
+ __flush_tlb_single(msg->address);
stat->d_onetlb++;
}
stat->d_requestee++;



2017-12-05 21:10:07

by Andrew Banman

[permalink] [raw]
Subject: Re: [PATCH 5/9] x86/uv: Use the right tlbflush API

On 12/5/17 6:34 AM, Peter Zijlstra wrote:
> Since uv_flush_tlb_others() implements flush_tlb_others() which is
> about flushing user mappings, we should use __flush_tlb_single(),
> which too is about flushing user mappings.
>
> Cc: Andrew Banman<[email protected]>
> Cc: Mike Travis<[email protected]>
> Signed-off-by: Peter Zijlstra (Intel)<[email protected]>
> ---
> arch/x86/platform/uv/tlb_uv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/arch/x86/platform/uv/tlb_uv.c
> +++ b/arch/x86/platform/uv/tlb_uv.c
> @@ -299,7 +299,7 @@ static void bau_process_message(struct m
> local_flush_tlb();
> stat->d_alltlb++;
> } else {
> - __flush_tlb_one(msg->address);
> + __flush_tlb_single(msg->address);
> stat->d_onetlb++;
> }
> stat->d_requestee++;

This looks like the right thing to do. We'll be testing it and complain later if
we find any problems, but I'm not expecting any since this patch looks to
maintain our status quo.

Best,

Andrew

2017-12-05 21:28:11

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 5/9] x86/uv: Use the right tlbflush API

On Tue, Dec 05, 2017 at 03:09:48PM -0600, Andrew Banman wrote:
> On 12/5/17 6:34 AM, Peter Zijlstra wrote:
> >Since uv_flush_tlb_others() implements flush_tlb_others() which is
> >about flushing user mappings, we should use __flush_tlb_single(),
> >which too is about flushing user mappings.
> >
> >Cc: Andrew Banman<[email protected]>
> >Cc: Mike Travis<[email protected]>
> >Signed-off-by: Peter Zijlstra (Intel)<[email protected]>
> >---
> > arch/x86/platform/uv/tlb_uv.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >--- a/arch/x86/platform/uv/tlb_uv.c
> >+++ b/arch/x86/platform/uv/tlb_uv.c
> >@@ -299,7 +299,7 @@ static void bau_process_message(struct m
> > local_flush_tlb();
> > stat->d_alltlb++;
> > } else {
> >- __flush_tlb_one(msg->address);
> >+ __flush_tlb_single(msg->address);
> > stat->d_onetlb++;
> > }
> > stat->d_requestee++;
>
> This looks like the right thing to do. We'll be testing it and complain later if
> we find any problems, but I'm not expecting any since this patch looks to
> maintain our status quo.

Well, with KPTI (the-patch-set-formerly-known-as-kaiser), there will be
a distinct difference between the two.

With KPTI __flush_tlb_one() would end up invalidating all kernel
mappings while __flush_tlb_single() will end up only invalidating the
user mappings of the current mm.

2017-12-05 23:05:00

by Andrew Banman

[permalink] [raw]
Subject: Re: [PATCH 5/9] x86/uv: Use the right tlbflush API



On 12/5/17 3:27 PM, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 03:09:48PM -0600, Andrew Banman wrote:
>> On 12/5/17 6:34 AM, Peter Zijlstra wrote:
>>> Since uv_flush_tlb_others() implements flush_tlb_others() which is
>>> about flushing user mappings, we should use __flush_tlb_single(),
>>> which too is about flushing user mappings.
>>>
>>> Cc: Andrew Banman<[email protected]>
>>> Cc: Mike Travis<[email protected]>
>>> Signed-off-by: Peter Zijlstra (Intel)<[email protected]>
>>> ---
>>> arch/x86/platform/uv/tlb_uv.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> --- a/arch/x86/platform/uv/tlb_uv.c
>>> +++ b/arch/x86/platform/uv/tlb_uv.c
>>> @@ -299,7 +299,7 @@ static void bau_process_message(struct m
>>> local_flush_tlb();
>>> stat->d_alltlb++;
>>> } else {
>>> - __flush_tlb_one(msg->address);
>>> + __flush_tlb_single(msg->address);
>>> stat->d_onetlb++;
>>> }
>>> stat->d_requestee++;
>>
>> This looks like the right thing to do. We'll be testing it and complain later if
>> we find any problems, but I'm not expecting any since this patch looks to
>> maintain our status quo.
>
> Well, with KPTI (the-patch-set-formerly-known-as-kaiser), there will be
> a distinct difference between the two.
>
> With KPTI __flush_tlb_one() would end up invalidating all kernel
> mappings while __flush_tlb_single() will end up only invalidating the
> user mappings of the current mm.
>

Right! Now the KPTI __flush_tlb_single() equals the old
__flush_tlb_one(), less the call to count_vm_tlb_event().

ACK