From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5425FC2D0A3 for ; Thu, 12 Nov 2020 11:24:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CB91A2222A for ; Thu, 12 Nov 2020 11:24:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CB91A2222A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F1EB36B0071; Thu, 12 Nov 2020 06:24:20 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id ED0D66B0072; Thu, 12 Nov 2020 06:24:20 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DC1B46B0073; Thu, 12 Nov 2020 06:24:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id AEB406B0071 for ; Thu, 12 Nov 2020 06:24:20 -0500 (EST) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 4F1403626 for ; Thu, 12 Nov 2020 11:24:20 +0000 (UTC) X-FDA: 77475532680.02.copy01_5000d1b27306 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id 3463210097AA0 for ; Thu, 12 Nov 2020 11:24:20 +0000 (UTC) X-HE-Tag: copy01_5000d1b27306 X-Filterd-Recvd-Size: 4583 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Thu, 12 Nov 2020 11:24:19 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 5D0F0AF0D; Thu, 12 Nov 2020 11:24:18 +0000 (UTC) Subject: Re: [PATCH v21 14/19] mm/lru: introduce TestClearPageLRU To: Hugh Dickins Cc: Alex Shi , akpm@linux-foundation.org, mgorman@techsingularity.net, tj@kernel.org, khlebnikov@yandex-team.ru, daniel.m.jordan@oracle.com, willy@infradead.org, hannes@cmpxchg.org, lkp@intel.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, shakeelb@google.com, iamjoonsoo.kim@lge.com, richard.weiyang@gmail.com, kirill@shutemov.name, alexander.duyck@gmail.com, rong.a.chen@intel.com, mhocko@suse.com, vdavydov.dev@gmail.com, shy828301@gmail.com, Michal Hocko References: <1604566549-62481-1-git-send-email-alex.shi@linux.alibaba.com> <1604566549-62481-15-git-send-email-alex.shi@linux.alibaba.com> From: Vlastimil Babka Message-ID: <057bb8c8-3993-5638-42e9-f58d72f21b9e@suse.cz> Date: Thu, 12 Nov 2020 12:24:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 11/12/20 3:03 AM, Hugh Dickins wrote: > On Wed, 11 Nov 2020, Vlastimil Babka wrote: >> On 11/5/20 9:55 AM, Alex Shi wrote: >> >> > --- a/mm/vmscan.c >> > +++ b/mm/vmscan.c >> > @@ -1542,7 +1542,7 @@ unsigned int reclaim_clean_pages_from_list(struct >> > zone *zone, >> > */ >> > int __isolate_lru_page(struct page *page, isolate_mode_t mode) >> > { >> > - int ret = -EINVAL; >> > + int ret = -EBUSY; >> > /* Only take pages on the LRU. */ >> > if (!PageLRU(page)) >> > @@ -1552,8 +1552,6 @@ int __isolate_lru_page(struct page *page, >> > isolate_mode_t mode) >> > if (PageUnevictable(page) && !(mode & ISOLATE_UNEVICTABLE)) >> > return ret; >> > - ret = -EBUSY; >> >> I'm not sure why this change is here, looks unrelated to the patch? >> >> Oh I see, you want to prevent the BUG() in isolate_lru_pages(). > > Yes, I suggested this part of the patch to Alex, when I hit that BUG(). > >> >> But due to that, the PageUnevictable check was also affected unintentionally. >> But I don't think it's that important to BUG() when we run into >> PageUnevictable unexpectedly, so that's probably ok. > > Not unintentional. __isolate_lru_page(), or __isolate_lru_page_prepare(), > is a silly function, used by two callers whose requirements are almost > entirely disjoint. The ISOLATE_UNEVICTABLE case is only for compaction.c, > which takes no interest in -EINVAL versus -EBUSY, and has no such BUG(). > > I think it dates back to lumpy reclaim days, and it probably made more > sense back then. Ah, thanks for explaining. >> >> But with that, we can just make __isolate_lru_page() a bool function and >> remove the ugly switch in isolate_lru_pages()? > > I agree that the switch statement in isolate_lru_pages() seems pointless > now, and can be turned into an if{}else{}. But that cleanup is a > diversion from this particular TestClearPageLRU patch, and I think from > the whole series (checking final state of the patchset, yes, the switch > is still there - though I think there have been variant series which > removed it). > > Can we please leave that cleanup until after the series has gone in? Sure thing! The patch seems functionally fine, so Acked-by: Vlastimil Babka > I think several of us have cleanups or optimization that we want to > follow (I had one that inlines what isolate_migratepages_block() wanted > of __isolate_lru_page() into that function, so simplifying what vmscan.c > needs; perhaps that can now eliminate it completely, I've not tried > recently). But there was a point at which the series was growing > ten patches per release as we all added our bits and pieces on top, > it got harder and harder to review the whole, and further from > getting the basics in: I do push back against that tendency. > > Hugh >