linux-kernel - Re: [RFC 04/10] x86/mm: Pass flush_tlb_info to flush_tlb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALCETrUAGHHCS1_NX+ckjYqnqg7t2M5bzu7uVafN7J3U2x-thA@mail.gmail.com>
Date:   Thu, 11 May 2017 20:41:18 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bpetkov@...e.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mel Gorman <mgorman@...e.de>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Rik van Riel <riel@...hat.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Nadav Amit <namit@...are.com>, Michal Hocko <mhocko@...e.com>,
        Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [RFC 04/10] x86/mm: Pass flush_tlb_info to flush_tlb_others() etc

On Thu, May 11, 2017 at 1:01 PM, Nadav Amit <nadav.amit@...il.com> wrote:
>
>> On May 7, 2017, at 5:38 AM, Andy Lutomirski <luto@...nel.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’)?

I added this to the queue.

>
> 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.

Me neither.  The value 0 can happen too, e.g. for init_mm.  Maybe I'll
clean that up when I'm all done with this stuff.

>
> Thanks,
> Nadav