Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 19 May 2021 21:46:51 -0400
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     "Huang, Ying" <ying.huang@intel.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, Matthew Wilcox <willy@infradead.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Peter Xu <peterx@redhat.com>, Hugh Dickins <hughd@google.com>,
        Mel Gorman <mgorman@suse.de>, Rik van Riel <riel@surriel.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Michal Hocko <mhocko@kernel.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Tim Chen <tim.c.chen@intel.com>
Subject: Re: [PATCH] mm: move idle swap cache pages to the tail of LRU after
 COW
Message-ID: <YKW/ix3Yg5HRuBaC@cmpxchg.org>
References: <20210519013313.1274454-1-ying.huang@intel.com>
 <YKUlfeAiq/vv+dHl@cmpxchg.org>
 <87r1i28ahm.fsf@yhuang6-desk1.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87r1i28ahm.fsf@yhuang6-desk1.ccr.corp.intel.com>
Precedence: bulk

On Thu, May 20, 2021 at 09:22:45AM +0800, Huang, Ying wrote:
> Johannes Weiner <hannes@cmpxchg.org> writes:
> 
> > On Wed, May 19, 2021 at 09:33:13AM +0800, Huang Ying wrote:
> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index b83f734c4e1d..2b6847f4c03e 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c
> >> @@ -3012,6 +3012,11 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
> >>  				munlock_vma_page(old_page);
> >>  			unlock_page(old_page);
> >>  		}
> >> +		if (page_copied && PageSwapCache(old_page) &&
> >> +		    !page_mapped(old_page) && trylock_page(old_page)) {
> >> +			try_to_free_idle_swapcache(old_page);
> >> +			unlock_page(old_page);
> >
> > If there are no more swap or pte references, can we just attempt to
> > free the page right away, like we do during regular unmap?
> >
> > 		if (page_copied)
> > 			free_swap_cache(old_page);
> > 		put_page(old_page);
> 
> A previous version of the patch does roughly this.
> 
> https://lore.kernel.org/lkml/20210113024241.179113-1-ying.huang@intel.com/
> 
> But Linus has concerns with the overhead introduced in the hot COW path.

Sorry, I had missed that thread.

It sounds like there were the same concerns about the LRU shuffling
overhead in the COW page. Now we have numbers for that, but not the
free_swap_cache version. Would you be able to run the numbers for that
as well? It would be interesting to see how much the additional code
complexity buys us.

> Another possibility is to move the idle swap cache page to the tail of
> the file LRU list.  But the question is how to identify the page.

The LRU type is identified by PG_swapbacked, and we do clear that for
anon pages to implement MADV_FREE. It may work here too. But I'm
honestly a bit skeptical about the ROI on this...