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 S1752836AbbDPTVm (ORCPT <rfc822;w@1wt.eu>);
	Thu, 16 Apr 2015 15:21:42 -0400
Received: from mail-pd0-f181.google.com ([209.85.192.181]:35968 "EHLO
	mail-pd0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752621AbbDPTVe (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 16 Apr 2015 15:21:34 -0400
Date: Thu, 16 Apr 2015 12:21:31 -0700 (PDT)
From: Hugh Dickins <hughd@google.com>
X-X-Sender: hugh@eggly.anvils
To: Mel Gorman <mgorman@suse.de>
cc: Linux-MM <linux-mm@kvack.org>, Rik van Riel <riel@redhat.com>,
        Hugh Dickins <hughd@google.com>, Minchan Kim <minchan@kernel.org>,
        Dave Hansen <dave.hansen@intel.com>, Andi Kleen <andi@firstfloor.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] mm: Send a single IPI to TLB flush multiple pages
 when unmapping
In-Reply-To: <1429179766-26711-3-git-send-email-mgorman@suse.de>
Message-ID: <alpine.LSU.2.11.1504161157390.17733@eggly.anvils>
References: <1429179766-26711-1-git-send-email-mgorman@suse.de> <1429179766-26711-3-git-send-email-mgorman@suse.de>
User-Agent: Alpine 2.11 (LSU 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, 16 Apr 2015, Mel Gorman wrote:
>  
>  	/* Move the dirty bit to the physical page now the pte is gone. */
> -	if (pte_dirty(pteval))
> +	if (pte_dirty(pteval)) {
> +		/*
> +		 * If the PTE was dirty then the TLB must be flushed before
> +		 * the page is unlocked as IO can start in parallel. Without
> +		 * the flush, writes could still happen and data would be
> +		 * potentially lost.
> +		 */
> +		if (deferred)
> +			flush_tlb_page(vma, address);

Okay, yes, that should deal with it; and you're probably right that the
safe pte_dirty !pte_write case is too uncommon to be worth another test.

But it would be better to batch even in the pte_dirty case: noting that
it has occurred in the tlb_ubc, then if so, doing try_to_unmap_flush()
before leaving try_to_unmap().

Particularly as you have already set_tlb_ubc_flush_pending() above,
so shrink_lruvec() may then follow with an unnecessary flush; though
I guess a little rearrangement here could stop that.

> +
>  		set_page_dirty(page);
> +	}
>  
>  	/* Update high watermark before we lower rss */
>  	update_hiwater_rss(mm);