From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932543AbdEKUBt (ORCPT <rfc822;w@1wt.eu>);
        Thu, 11 May 2017 16:01:49 -0400
Received: from mail-pg0-f66.google.com ([74.125.83.66]:36529 "EHLO
        mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1755259AbdEKUBr (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 11 May 2017 16:01:47 -0400
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [RFC 04/10] x86/mm: Pass flush_tlb_info to flush_tlb_others() etc
From: Nadav Amit <nadav.amit@gmail.com>
In-Reply-To: <51ec06f28360a1cc505649acaf0c9db905824115.1494160201.git.luto@kernel.org>
Date: Thu, 11 May 2017 13:01:44 -0700
Cc: X86 ML <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Borislav Petkov <bpetkov@suse.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mel Gorman <mgorman@suse.de>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Rik van Riel <riel@redhat.com>, Dave Hansen <dave.hansen@intel.com>,
        Nadav Amit <namit@vmware.com>, Michal Hocko <mhocko@suse.com>,
        Sasha Levin <sasha.levin@oracle.com>
Message-Id: <EFEA207A-72E6-4A41-A70A-D91CC9EB8A47@gmail.com>
References: <cover.1494160201.git.luto@kernel.org>
 <cover.1494160201.git.luto@kernel.org>
 <51ec06f28360a1cc505649acaf0c9db905824115.1494160201.git.luto@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
X-Mailer: Apple Mail (2.3273)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v4BK2Kxt013051


> 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