From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752029AbdDOBRK (ORCPT ); Fri, 14 Apr 2017 21:17:10 -0400 Received: from mga04.intel.com ([192.55.52.120]:61915 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751258AbdDOBRI (ORCPT ); Fri, 14 Apr 2017 21:17:08 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,201,1488873600"; d="scan'208";a="74311128" From: "Huang\, Ying" To: Johannes Weiner Cc: "Huang\, Ying" , Andrew Morton , , , Andrea Arcangeli , "Ebru Akagunduz" , Michal Hocko , "Tejun Heo" , Hugh Dickins , Shaohua Li , Minchan Kim , Rik van Riel , Subject: Re: [PATCH -mm -v8 1/3] mm, THP, swap: Delay splitting THP during swap out References: <20170406053515.4842-1-ying.huang@intel.com> <20170406053515.4842-2-ying.huang@intel.com> <20170414145856.GA9812@cmpxchg.org> Date: Sat, 15 Apr 2017 09:17:04 +0800 In-Reply-To: <20170414145856.GA9812@cmpxchg.org> (Johannes Weiner's message of "Fri, 14 Apr 2017 10:58:56 -0400") Message-ID: <87k26mzcz3.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Johannes, Johannes Weiner writes: > Hi Huang, > > I reviewed this patch based on the feedback I already provided, but > eventually gave up and rewrote it. Please take review feedback more > seriously in the future. Thanks a lot for your help! I do respect all your review and effort. The -v8 patch doesn't take all your comments, just because I thought we have not reach consensus for some points and I want to use -v8 patch to discuss them. One concern I have before is whether to split THP firstly when swap space or memcg swap is used up. Now I think your solution is acceptable. And if we receive any regression report for that in the future, it's not very hard to deal with. > Attached below is the reworked patch. Most changes are to the layering > (page functions, cluster functions, range functions) so that we don't > make the lowest swap range code require a notion of huge pages, or > make the memcg page functions take size information that can be > gathered from the page itself. I turned the config symbol into a > generic THP_SWAP that can later be extended when we add 2MB IO. The > rest is function naming, #ifdef removal etc. For some #ifdef in swapfile.c, it is to avoid unnecessary code size increase for !CONFIG_TRANSPARENT_HUGEPAGE or platform with THP swap optimization disabled. Is it an issue? > Please review whether this is an acceptable version for you. Yes. It is good for me. I will give it more test on next Monday. [...] > diff --git a/mm/Kconfig b/mm/Kconfig > index c89f472b658c..660fb765bf7d 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -447,6 +447,18 @@ choice > benefit. > endchoice > > +config ARCH_WANTS_THP_SWAP > + def_bool n > + > +config THP_SWAP > + def_bool y > + depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP > + help > + Swap transparent huge pages in one piece, without splitting. > + XXX: For now this only does clustered swap space allocation. Is 'XXX' here intended. > + > + For selection by architectures with reasonable THP sizes. > + > config TRANSPARENT_HUGE_PAGECACHE > def_bool y > depends on TRANSPARENT_HUGEPAGE > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index d14dd961f626..4a5c1ca21894 100644 [...] Best Regards, Huang, Ying