Messages in this thread | | | From | Atish Patra <> | Subject | Re: [PATCH] RISC-V: Issue a local tlb flush if possible. | Date | Thu, 15 Aug 2019 20:37:04 +0000 |
| |
On Tue, 2019-08-13 at 07:30 -0700, hch@infradead.org wrote: > On Tue, Aug 13, 2019 at 12:15:15AM +0000, Atish Patra wrote: > > I thought if it recieves an empty cpumask, then it should at least > > do a > > local flush is the expected behavior. Are you saying that we should > > just skip all and return ? > > How could we ever receive an empty cpu mask? I think it could only > be empty without the current cpu. At least that is my reading of > the callers and a few other implementations. >
We get ton of them. Here is the stack dump.
[ 16.735814] [<ffffffe000035498>] walk_stackframe+0x0/0xa0^M 298436 [ 16.819037] [<ffffffe0000355f8>] show_stack+0x2a/0x34^M 298437 [ 16.899648] [<ffffffe00067b54c>] dump_stack+0x62/0x7c^M 298438 [ 16.977402] [<ffffffe0000ef6f6>] tlb_flush_mmu+0x14a/0x150^M 298439 [ 17.054197] [<ffffffe0000ef7a4>] tlb_finish_mmu+0x42/0x72^M 298440 [ 17.129986] [<ffffffe0000ede7c>] exit_mmap+0x8e/0xfa^M 298441 [ 17.203669] [<ffffffe000037d54>] mmput.part.3+0x1a/0xc4^M 298442 [ 17.275985] [<ffffffe000037e1e>] mmput+0x20/0x28^M 298443 [ 17.345248] [<ffffffe0001143c2>] flush_old_exec+0x418/0x5f8^M 298444 [ 17.415370] [<ffffffe000158408>] load_elf_binary+0x262/0xc84^M 298445 [ 17.483641] [<ffffffe000114614>] search_binary_handler.part.7+0x72/0x172^M 298446 [ 17.552078] [<ffffffe000114bb2>] __do_execve_file+0x40c/0x56a^M 298447 [ 17.617932] [<ffffffe00011503e>] sys_execve+0x26/0x32^M 298448 [ 17.682164] [<ffffffe00003437e>] ret_from_syscall+0x0/0xe^M
It looks like it is in the path of clearing the old traces of already run script or program. I am not sure if the cpumask supposed to be empty in this path.
Probably we should just issue tlb flush all for all CPUs instead of just the local CPU.
> > > if (!cpumask || cpumask_test_cpu(cpu, cpumask) { > > > if ((start == 0 && size == -1) || size > PAGE_SIZE) > > > local_flush_tlb_all(); > > > else if (size == PAGE_SIZE) > > > local_flush_tlb_page(start); > > > cpumask_clear_cpu(cpuid, cpumask); > > > > This will modify the original cpumask which is not correct. clear > > part > > has to be done on hmask to avoid a copy. > > Indeed. But looking at the x86 tlbflush implementation it seems like > we > could use cpumask_any_but() to avoid having to modify the passed in > cpumask.
Looking at the x86 code, it uses cpumask_any_but to just test if there any other cpu present apart from the current one.
If yes, it calls smp_call_function_many which ignores the current cpu and execute tlb flush code on all other cpus.
For RISC-V, it has to still send the cpumask containing local cpu and M-mode software may do a local tlb flush the tlbs again for no reason.
Regards, Atish
| |