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=-15.9 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1, USER_IN_DEF_DKIM_WL autolearn=ham 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 B9AF3C433DF for ; Mon, 15 Jun 2020 21:04:08 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 73CB32071A for ; Mon, 15 Jun 2020 21:04:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="KtYbf5T/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 73CB32071A 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 055F46B0002; Mon, 15 Jun 2020 17:04:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 007CE6B0003; Mon, 15 Jun 2020 17:04:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E60146B0005; Mon, 15 Jun 2020 17:04:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0144.hostedemail.com [216.40.44.144]) by kanga.kvack.org (Postfix) with ESMTP id CE6776B0002 for ; Mon, 15 Jun 2020 17:04:07 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 8A868181AC9CC for ; Mon, 15 Jun 2020 21:04:07 +0000 (UTC) X-FDA: 76932673734.27.glass80_0b1484e26df9 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id 4131C3D66B for ; Mon, 15 Jun 2020 21:04:07 +0000 (UTC) X-HE-Tag: glass80_0b1484e26df9 X-Filterd-Recvd-Size: 6758 Received: from mail-qk1-f195.google.com (mail-qk1-f195.google.com [209.85.222.195]) by imf24.hostedemail.com (Postfix) with ESMTP for ; Mon, 15 Jun 2020 21:04:06 +0000 (UTC) Received: by mail-qk1-f195.google.com with SMTP id c185so17207801qke.7 for ; Mon, 15 Jun 2020 14:04:06 -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=c3CcV+Eqa52WujImAF4meTvGkOC9cvtsKvotjvbz2f0=; b=KtYbf5T/cb5Ryx7y9SBsiANN5FWhanrodOgrnGLo1Z/fjEfDD28weByG3vEOKELAEZ vsYpGPRd7MjqR5s+TDWjJBVcIFlIfcSm/UAemCnjm+gD58gbO0qPa1RPC/FlqQyUt7Uf Ns0p9mcPSmoNCKEp7r4ecW8KIGGAFYdWbRJXrHSpfPYLOKM3UfgJpiPyoADkAek6qDxL lLffTKJLOnRQyrzIPfSCsJpScjeMXMjH/7864MSluBjaN7r8ltZnJZ7gm0IYdJSHMW4Q lk//rbUFxqF2sMYR7tuqMjYLdSTJB7c/Hd5AtEPgQhLXNaIzmRclmbep3IhMVyMG5UMG SFcA== 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=c3CcV+Eqa52WujImAF4meTvGkOC9cvtsKvotjvbz2f0=; b=MGWGRKyV+j+++bfU/HHevv+ASEjDm/8BmEHYBuSweXBGGDmp7W/O7yqaYgBtgpWlb4 Q0BwFmGNepGVx4YszKtHCcK0+nhOjCL1nt/9nqajNpGKrMwiBtfW+nd6bN9fUks030/Y qhK7tTLTzgd4MxATieRHR0Qh2WmlmSybznKdc8T323A8veDUcr63XSON2/hoUhiShYGs 0A7T5mBwLmOxubw/UcdtX4dR9KgkPJj4d12dVKy8xCrixlv+9/msqr/Ltgt32F0/93IT wSCCIOL3FOKsr7kpy3ZrpxxMa5+wSk7UJg7HQVZSnmQnWzT2F91c6bjrTYGOOQ476bQN z6mA== X-Gm-Message-State: AOAM531jsWeEoHPmx9m70M83uht1Z9S8X/DldahWBE6PPu5EJJCXj6Tz hZ8JJlaXCUYcbdqLxI6LUBWlqg== X-Google-Smtp-Source: ABdhPJxm4LxnHr3trK46lRfSXaMcE1m0MTQu7lTK/5zm53taHdW4x9QvF20Al3clChYRF3o29eNxow== X-Received: by 2002:a37:38c:: with SMTP id 134mr17616150qkd.434.1592255045366; Mon, 15 Jun 2020 14:04:05 -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 n13sm14405725qtb.20.2020.06.15.14.04.03 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Mon, 15 Jun 2020 14:04:04 -0700 (PDT) Date: Mon, 15 Jun 2020 14:03:50 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Vlastimil Babka cc: Hugh Dickins , Mel Gorman , Andrew Morton , Li Wang , Alex Shi , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] mm, page_alloc: capture page in task context only In-Reply-To: <01287865-153d-42e7-afd8-1178ec6bc5b9@suse.cz> Message-ID: References: <01287865-153d-42e7-afd8-1178ec6bc5b9@suse.cz> 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: 4131C3D66B X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 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 Fri, 12 Jun 2020, Vlastimil Babka wrote: > On 6/10/20 10:48 PM, Hugh Dickins wrote: > > While stressing compaction, one run oopsed on NULL capc->cc in > > __free_one_page()'s task_capc(zone): compact_zone_order() had been > > interrupted, and a page was being freed in the return from interrupt. > > > > Though you would not expect it from the source, both gccs I was using > > (a 4.8.1 and a 7.5.0) had chosen to compile compact_zone_order() with > > the ".cc = &cc" implemented by mov %rbx,-0xb0(%rbp) immediately before > > callq compact_zone - long after the "current->capture_control = &capc". > > An interrupt in between those finds capc->cc NULL (zeroed by an earlier > > rep stos). > > Ugh, nasty. Same here with gcc 10. Thanks for checking, nice to know that I'm in good company :) > > > This could presumably be fixed by a barrier() before setting > > current->capture_control in compact_zone_order(); but would also need > > more care on return from compact_zone(), in order not to risk leaking > > a page captured by interrupt just before capture_control is reset. > > I was hoping a WRITE_ONCE(current->capture_control) would be enough, > but apparently it's not (I tried). Right, I don't think volatiles themselves actually constitute barriers; but I'd better keep quiet, I notice the READ_ONCE/WRITE_ONCE/data_race industry has been busy recently, and I'm likely out-of-date and mistaken. > > > Maybe that is the preferable fix, but I felt safer for task_capc() to > > exclude the rather surprising possibility of capture at interrupt time. > > > Fixes: 5e1f0f098b46 ("mm, compaction: capture a page under direct compaction") > > Cc: stable@vger.kernel.org # 5.1+ > > Signed-off-by: Hugh Dickins > > Acked-by: Vlastimil Babka Thanks, and to Mel for his. > > But perhaps I would also make sure that we don't expose the half initialized > capture_control and run into this problem again later. It's not like this is a > fast path where barriers hurt. Something like this then? (with added comments) Would it be very rude if I leave that to you and to Mel? to add, or to replace mine if you wish - go ahead. I can easily see that more sophistication at the compact_zone_order() end may be preferable to another test and branch inside __free_one_page() (and would task_capc() be better with an "unlikely" in it?). But it seems unnecessary to have a fix at both ends, and I'm rather too wound up in other things at the moment, to want to read up on the current state of such barriers, and sign off on the Vlastipatch below myself (but I do notice that READ_ONCE seems to have more in it today than I remember, which probably accounts for why you did not put the barrier() I expected to see on the way out). Hugh > > diff --git a/mm/compaction.c b/mm/compaction.c > index fd988b7e5f2b..c89e26817278 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -2316,15 +2316,17 @@ static enum compact_result compact_zone_order(struct zone *zone, int order, > .page = NULL, > }; > > - current->capture_control = &capc; > + barrier(); > + > + WRITE_ONCE(current->capture_control, &capc); > > ret = compact_zone(&cc, &capc); > > VM_BUG_ON(!list_empty(&cc.freepages)); > VM_BUG_ON(!list_empty(&cc.migratepages)); > > - *capture = capc.page; > - current->capture_control = NULL; > + WRITE_ONCE(current->capture_control, NULL); > + *capture = READ_ONCE(capc.page); > > return ret; > }