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=-3.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 8F0D2C43461 for ; Wed, 9 Sep 2020 16:11:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 24FD92166E for ; Wed, 9 Sep 2020 16:11:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="rlfBLzoK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 24FD92166E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 62D9E900003; Wed, 9 Sep 2020 12:11:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 604B06B0082; Wed, 9 Sep 2020 12:11:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4F2E2900003; Wed, 9 Sep 2020 12:11:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0053.hostedemail.com [216.40.44.53]) by kanga.kvack.org (Postfix) with ESMTP id 39BD96B007E for ; Wed, 9 Sep 2020 12:11:44 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id EAC25181AEF07 for ; Wed, 9 Sep 2020 16:11:43 +0000 (UTC) X-FDA: 77244013686.21.bath87_5916b76270de Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id 6FB3A180442C2 for ; Wed, 9 Sep 2020 16:11:41 +0000 (UTC) X-HE-Tag: bath87_5916b76270de X-Filterd-Recvd-Size: 6691 Received: from mail-il1-f196.google.com (mail-il1-f196.google.com [209.85.166.196]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Wed, 9 Sep 2020 16:11:40 +0000 (UTC) Received: by mail-il1-f196.google.com with SMTP id t13so2846352ile.9 for ; Wed, 09 Sep 2020 09:11:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fi6EPOk4V99JFbqYmEhNPN5TNg8BdROS9o2xQ3RBg5E=; b=rlfBLzoK50AdGuXYFUe5sFCgGZuO8AeB+DJLXTpOd98w2tONPNvWw7PRN8UwfX1kcB qpaUWzVXxXFCR4QS6X1eKPXKlVCOw84PnHMfRJcwucgm7MCtrONclkv3c2KGz6NKRWF6 bIpuGEl92zf3lDES3mrCY0o9yN5bbmlesde/nSjHgaz11aqQZJE94l16lobir7khJp1G cAcR6z9rG4oAj+dPwZ9HPfHvbwf3BM1FGFuYK/NIxWfxS1rSM4FBiuz2BiSu5KXeaOPw QTce2zDZfvrieGVGSjHvcdTXGqtBQ81gE7ZOjeMSmma7RQZoOn1sEkpUSmqsZ5lAvgfp /a4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fi6EPOk4V99JFbqYmEhNPN5TNg8BdROS9o2xQ3RBg5E=; b=Gw6IloOoZ+wTfKct+tkusVbZBzoL8p8f3HDghoTzy7I6m+eAs/HP+V1HZT5izGvrzd aKPJwAOdz3JR4hu8dCDi7VFHQqlWv5ktke7RqKNW2M6JXWfomGiZwUmutT3cbqDGgEKg nlRXRLnTudod9GD0kcXtDiDfDxbiKTyMlrV1ewvZTMZe6zSTl7O9Q5IhVOA7m6zyThe2 25bU28L72JmUJUhHrNBmPaiNufEwKxzLO+PKJ+Ft31voV88GhgxzQHFapvGj6C+286XH Gw/u3jfFDwd2mfny2uq0PeKTLvAxzfQBOSKPp9ianFHHGwey07Kk0CZjoXc8IFfQSDWv tKyQ== X-Gm-Message-State: AOAM530TQ0XgRKASkSZLNwBEWrcSxJPO2nD4DeYhedVkFs2ZbVZbnbme 9VbNLM45GT1wHKym4hM2LD+/Wrw7ljCPENyA/qI= X-Google-Smtp-Source: ABdhPJx7KEby+OfH82kXGH3NIHe6v3PfIbd7mVCLU00Uzi2e/UVTJzdsgLsJe7WH+xQ5FqL4JHH6CPwAlr9OBlnQ7WQ= X-Received: by 2002:a92:ae06:: with SMTP id s6mr4063803ilh.64.1599667899575; Wed, 09 Sep 2020 09:11:39 -0700 (PDT) MIME-Version: 1.0 References: <1598273705-69124-1-git-send-email-alex.shi@linux.alibaba.com> <20200824114204.cc796ca182db95809dd70a47@linux-foundation.org> In-Reply-To: From: Alexander Duyck Date: Wed, 9 Sep 2020 09:11:28 -0700 Message-ID: Subject: Re: [PATCH v18 00/32] per memcg lru_lock: reviews To: Hugh Dickins Cc: Alex Shi , Andrew Morton , Mel Gorman , Tejun Heo , Konstantin Khlebnikov , Daniel Jordan , Matthew Wilcox , 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, Vlastimil Babka , Minchan Kim , Qian Cai Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 6FB3A180442C2 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 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 Tue, Sep 8, 2020 at 4:41 PM Hugh Dickins wrote: > > [PATCH v18 28/32] mm/compaction: Drop locked from isolate_migratepages_block > Most of this consists of replacing "locked" by "lruvec", which is good: > but please fold those changes back into 20/32 (or would it be 17/32? > I've not yet looked into the relationship between those two), so we > can then see more clearly what change this 28/32 (will need renaming!) > actually makes, to use lruvec_holds_page_lru_lock(). That may be a > good change, but it's mixed up with the "locked"->"lruvec" at present, > and I think you could have just used lruvec for locked all along > (but of course there's a place where you'll need new_lruvec too). I am good with my patch being folded in. No need to keep it separate. > [PATCH v18 29/32] mm: Identify compound pages sooner in isolate_migratepages_block > NAK. I agree that isolate_migratepages_block() looks nicer this way, but > take a look at prep_new_page() in mm/page_alloc.c: post_alloc_hook() is > where set_page_refcounted() changes page->_refcount from 0 to 1, allowing > a racing get_page_unless_zero() to succeed; then later prep_compound_page() > is where PageHead and PageTails get set. So there's a small race window in > which this patch could deliver a compound page when it should not. So the main motivation for the patch was to avoid the case where we are having to reset the LRU flag. One question I would have is what if we swapped the code block with the __isolate_lru_page_prepare section? WIth that we would be taking a reference on the page, then verifying the LRU flag is set, and then testing for compound page flag bit. Would doing that close the race window since the LRU flag being set should indicate that the allocation has already been completed has it not? > [PATCH v18 30/32] mm: Drop use of test_and_set_skip in favor of just setting skip > I haven't looked at this yet (but recall that per-memcg lru_lock can > change the point at which compaction should skip a contended lock: IIRC > the current kernel needs nothing extra, whereas some earlier kernels did > need extra; but when I look at 30/32, may find these remarks irrelevant). > > [PATCH v18 31/32] mm: Add explicit page decrement in exception path for isolate_lru_pages > The title of this patch is definitely wrong: there was an explicit page > decrement there before (put_page), now it's wrapping it up inside a > WARN_ON(). We usually prefer to avoid doing functional operations > inside WARN/BUGs, but I think I'll overlook that - anyone else worried? > The comment is certainly better than what was there before: yes, this > warning reflects the difficulty we have in thinking about the > TestClearPageLRU protocol: which I'm still not sold on, but > agree we should proceed with. With a change in title, perhaps > "mm: add warning where TestClearPageLRU failed on freeable page"? > Acked-by: Hugh Dickins I can update that and resubmit it if needed. I know there were also some suggestions from Matthew.