On Fri, 2019-09-20 at 13:11 -0700, John Hubbard wrote:
> On 9/20/19 12:50 PM, Leonardo Bras wrote:
> > Skips slow part of serialize_against_pte_lookup if there is no running
> > lockless pagetable walk.
> >
> > Signed-off-by: Leonardo Bras <[email protected]>
> > ---
> > arch/powerpc/mm/book3s64/pgtable.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> > index 13239b17a22c..41ca30269fa3 100644
> > --- a/arch/powerpc/mm/book3s64/pgtable.c
> > +++ b/arch/powerpc/mm/book3s64/pgtable.c
> > @@ -95,7 +95,8 @@ static void do_nothing(void *unused)
> > void serialize_against_pte_lookup(struct mm_struct *mm)
> > {
> > smp_mb();
> > - smp_call_function_many(mm_cpumask(mm), do_nothing, NULL, 1);
> > + if (running_lockless_pgtbl_walk(mm))
> > + smp_call_function_many(mm_cpumask(mm), do_nothing, NULL, 1);
>
> Hi,
>
> If you do this, then you are left without any synchronization. So it will
> have race conditions: a page table walk could begin right after the above
> check returns "false", and then code such as hash__pmdp_huge_get_and_clear()
> will continue on right away, under the false assumption that it has let
> all the current page table walks complete.
>
> The current code uses either interrupts or RCU to synchronize, and in
> either case, you end up scheduling something on each CPU. If you remove
> that entirely, I don't see anything left. ("Pure" atomic counting is not
> a synchronization technique all by itself.)
>
> thanks,
Hello John,
Thanks for the fast feedback.
See, before calling serialize_against_pte_lookup(), there is always an
update or clear on the pmd. So, if a page table walk begin right after
the check returns "false", there is no problem, since it will use the
updated pmd.
Think about serialize, on a process with a bunch of cpus. After you
check the last processor (wait part), there is no guarantee that the
first one is not starting a lockless pagetable walk.
The same mechanism protect both methods.
Does it make sense?
Best regards,
Leonardo Bras