Re: [RFC 04/10] x86/mm: Pass flush_tlb_info to flush_tlb_others() etc

Nadav Amit
2017-05-11T20:01:49Z

> On May 7, 2017, at 5:38 AM, Andy Lutomirski <luto@kernel.org> wrote:
>
> @@ -243,15 +237,15 @@ static void flush_tlb_func(void *info)
> return;
> }
>
> - if (f->flush_end == TLB_FLUSH_ALL) {
> + if (f->end == TLB_FLUSH_ALL) {
> local_flush_tlb();
> trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
> } else {
> unsigned long addr;
> unsigned long nr_pages =
> - (f->flush_end - f->flush_start) / PAGE_SIZE;
> - addr = f->flush_start;
> - while (addr < f->flush_end) {
> + (f->end - f->start) / PAGE_SIZE;
> + addr = f->start;
> + while (addr < f->end) {
> __flush_tlb_single(addr);
> addr += PAGE_SIZE;
> }
> @@ -260,33 +254,27 @@ static void flush_tlb_func(void *info)
> }
>
> void native_flush_tlb_others(const struct cpumask *cpumask,
> - struct mm_struct *mm, unsigned long start,
> - unsigned long end)
> + const struct flush_tlb_info *info)
> {
> - struct flush_tlb_info info;
> -
> - info.flush_mm = mm;
> - info.flush_start = start;
> - info.flush_end = end;
> -
> count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
> - if (end == TLB_FLUSH_ALL)
> + if (info->end == TLB_FLUSH_ALL)
> trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);
> else
> trace_tlb_flush(TLB_REMOTE_SEND_IPI,
> - (end - start) >> PAGE_SHIFT);
> + (info->end - info->start) >> PAGE_SHIFT);

I know it is stupid, but since you already change the code, can you make
flush_tlb_func() and native_flush_tlb_others() consistent in the way
they compute the number of pages? (either ‘>> PAGE_SHIFT’ or ‘/ PAGE_SIZE’)?

On a different topic: I do not like or actually understand why TLBSTATE_OK
is defined as 1 and not 0. The very least it would generate a better code.

Thanks,
Nadav