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=-12.8 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1, USER_IN_DEF_DKIM_WL 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 4742EC433E2 for ; Wed, 9 Sep 2020 18:24:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B640B21D40 for ; Wed, 9 Sep 2020 18:24:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="XDE4XX5W" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B640B21D40 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C4BE56B0073; Wed, 9 Sep 2020 14:24:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BFCE26B0074; Wed, 9 Sep 2020 14:24:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC5206B007B; Wed, 9 Sep 2020 14:24:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0232.hostedemail.com [216.40.44.232]) by kanga.kvack.org (Postfix) with ESMTP id 9728D6B0073 for ; Wed, 9 Sep 2020 14:24:31 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 5804C180AD817 for ; Wed, 9 Sep 2020 18:24:31 +0000 (UTC) X-FDA: 77244348342.21.bed90_4d057b1270df Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id 29595180442C0 for ; Wed, 9 Sep 2020 18:24:31 +0000 (UTC) X-HE-Tag: bed90_4d057b1270df X-Filterd-Recvd-Size: 6532 Received: from mail-qv1-f65.google.com (mail-qv1-f65.google.com [209.85.219.65]) by imf06.hostedemail.com (Postfix) with ESMTP for ; Wed, 9 Sep 2020 18:24:30 +0000 (UTC) Received: by mail-qv1-f65.google.com with SMTP id ef16so2016686qvb.8 for ; Wed, 09 Sep 2020 11:24:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=Ruhhup1GiVMzdtnhUGJBHZmdP0+i41QleFToFouJcG0=; b=XDE4XX5WI/xPzfW/NMmUCQ3gVCnjbXrHt8OZ8zTa8NBevb7IHbrl/CNJzNEBbJAWSt mb2pkZ6iQFCSt/q5DVeqGb+Nus0hYvK68luUixqriqlyeKJdl+oqygph3LDTrnWc6T9h fWv/uvdnEzXrsILGx8SzqA65K21CKUQDVaAl0JFq2hcncH1UebzvxYpWJbFh7zVFBpBu gingaYs6WXb1VQj6dXz40KyyJXkaaFLrzQ4rRC1CX61vyJvkxHuszDndK7jT5Lrbkojv DJeqyHfkPPnmTJGrQIIwUhTtQ+MRbeYsaCG00Wkar1qmHCnOEQgsdMgsei9v2ccL99kT XkOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=Ruhhup1GiVMzdtnhUGJBHZmdP0+i41QleFToFouJcG0=; b=mCm18BFsdqn6ofqoG7l9hHdCcTVnJlL3Z/iA/L2FjXZOjO0L08xqMwiTDzij8vsOd/ yVX0+xXtPw9EnVlt2nSxmRIaBwAqpiPdkD5j/O2shv7FKwwiD629vwpyXcW1QCGS5ker sFSFp/epOZHmGzwnoukrc7+MJkQTVLBE2Pn+kgieYaXhp/6aKRRmm+BqxO8fvxtIjbyN QfsxvULx2MlxN+nS3ZdUnPlbz5aTBpu+wAxXrowmeEOq6oFAXyCxZsMfaq85pkJH0JFT qImVeW9ONphC0izgeQXnHtxPpmplYmjzDHbCqkzRAEuivpnq7B4nGXdibrY+X7UevCVw Ll3g== X-Gm-Message-State: AOAM530nJ7hLUlWoDKa8EiOpFpFSp3iwt5wryhdyY4TKHHZt0oMdZGg6 dhvOwyFX+6QlGxMlQTwKcCFJ4w== X-Google-Smtp-Source: ABdhPJwLHtK06MB86ozbrs1dqxtlh9p5l/69lx5+FCQ89PVrR+dpBTlHWMb5oT5whELFYBtY0OnqTQ== X-Received: by 2002:a0c:ff4b:: with SMTP id y11mr5502578qvt.3.1599675869492; Wed, 09 Sep 2020 11:24:29 -0700 (PDT) Received: from eggly.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id c13sm3841553qtq.5.2020.09.09.11.24.26 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Wed, 09 Sep 2020 11:24:28 -0700 (PDT) Date: Wed, 9 Sep 2020 11:24:14 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Alexander Duyck cc: Matthew Wilcox , Alex Shi , Andrew Morton , Mel Gorman , Tejun Heo , Hugh Dickins , Konstantin Khlebnikov , Daniel Jordan , Johannes Weiner , kbuild test robot , linux-mm , LKML , cgroups@vger.kernel.org, Shakeel Butt , Joonsoo Kim , Wei Yang , "Kirill A. Shutemov" , Rong Chen , Michal Hocko , Vladimir Davydov , shy828301@gmail.com, Alexander Duyck Subject: Re: [PATCH v18 31/32] mm: Add explicit page decrement in exception path for isolate_lru_pages In-Reply-To: Message-ID: References: <1598273705-69124-1-git-send-email-alex.shi@linux.alibaba.com> <1598273705-69124-32-git-send-email-alex.shi@linux.alibaba.com> <20200909010118.GB6583@casper.infradead.org> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Rspamd-Queue-Id: 29595180442C0 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 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 Wed, 9 Sep 2020, Alexander Duyck wrote: > On Tue, Sep 8, 2020 at 6:01 PM Matthew Wilcox wrote: > > On Mon, Aug 24, 2020 at 08:55:04PM +0800, Alex Shi wrote: > > > +++ b/mm/vmscan.c > > > @@ -1688,10 +1688,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > > > > > > if (!TestClearPageLRU(page)) { > > > /* > > > - * This page may in other isolation path, > > > - * but we still hold lru_lock. > > > + * This page is being isolated in another > > > + * thread, but we still hold lru_lock. The > > > + * other thread must be holding a reference > > > + * to the page so this should never hit a > > > + * reference count of 0. > > > */ > > > - put_page(page); > > > + WARN_ON(put_page_testzero(page)); > > > goto busy; > > > > I read Hugh's review and that led me to take a look at this. We don't > > do it like this. Use the same pattern as elsewhere in mm: > > > > page_ref_sub(page, nr); > > VM_BUG_ON_PAGE(page_count(page) <= 0, page); > > > > > > Actually for this case page_ref_dec(page) would make more sense > wouldn't it? Otherwise I agree that would be a better change if that > is the way it has been handled before. I just wasn't familiar with > those other spots. After overnight reflection, my own preference would be simply to drop this patch. I think we are making altogether too much of a fuss here over what was simply correct as plain put_page() (and further from correct if we change it to leak the page in an unforeseen circumstance). And if Alex's comment was not quite grammatically correct, never mind, it said as much as was worth saying. I got more worried by his placement of the "busy:" label, but that does appear to work correctly. There's probably a thousand places where put_page() is used, where it would be troublesome if it were the final put_page(): this one bothered you because you'd been looking at isolate_migratepages_block(), and its necessary avoidance of lru_lock recursion on put_page(); but let's just just leave this put_page() as is. Hugh