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