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=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 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 D5E3BC433E0 for ; Fri, 26 Feb 2021 10:24:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1CCB164EEE for ; Fri, 26 Feb 2021 10:24:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CCB164EEE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 542016B0005; Fri, 26 Feb 2021 05:24:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F4B26B0006; Fri, 26 Feb 2021 05:24:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 406BD6B006C; Fri, 26 Feb 2021 05:24:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0206.hostedemail.com [216.40.44.206]) by kanga.kvack.org (Postfix) with ESMTP id 28CE36B0005 for ; Fri, 26 Feb 2021 05:24:34 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CE57FDDE8 for ; Fri, 26 Feb 2021 10:24:33 +0000 (UTC) X-FDA: 77860034826.25.2235E23 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf01.hostedemail.com (Postfix) with ESMTP id A8D072000385 for ; Fri, 26 Feb 2021 10:24:32 +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 E8982AF33; Fri, 26 Feb 2021 10:24:31 +0000 (UTC) Date: Fri, 26 Feb 2021 11:24:29 +0100 From: Oscar Salvador To: Michal Hocko Cc: Andrew Morton , Mike Kravetz , David Hildenbrand , Muchun Song , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 2/2] mm: Make alloc_contig_range handle in-use hugetlb pages Message-ID: <20210226102424.GA3557@linux> References: <20210222135137.25717-1-osalvador@suse.de> <20210222135137.25717-3-osalvador@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Stat-Signature: p58hhcapnxfk3y6wzxrrew9bqn7zrwtn X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: A8D072000385 Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf01; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1614335072-496441 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, Feb 26, 2021 at 09:46:57AM +0100, Michal Hocko wrote: > On Mon 22-02-21 14:51:37, Oscar Salvador wrote: > [...] > > @@ -2394,9 +2397,19 @@ bool isolate_or_dissolve_huge_page(struct page *page) > > */ > > if (hstate_is_gigantic(h)) > > return ret; > > - > > - if (!page_count(head) && alloc_and_dissolve_huge_page(h, head)) > > +retry: > > + if (page_count(head) && isolate_huge_page(head, list)) { > > ret = true; > > + } else if (!page_count(head)) { > > This is rather head spinning. Do we need to test page_count in the else > branch? Do you want to optimize for a case where the page cannot be > isolated because of page_huge_active? Well, I wanted to explictly call out both cases. We either 1) have an in-use page and we try to issolate it or 2) we have a free page (count == 0). If the page could not be dissolved due to page_huge_active, this would either mean that page is about to be freed, or that someone has already issolated the page. Being the former case, one could say that falling-through alloc_and_dissolve is ok. But no, I did not really want to optimize anything here, just wanted to be explicit about what we are checking and why. > > + > > + if (!err) { > > + ret = true; > > + } else if (err == -EBUSY && try_again) { > > + try_again = false; > > + goto retry; > > + } > > Is this retry once logic really needed? Does it really give us any real > benefit? alloc_and_dissolve_huge_page already retries when the page is > being freed. alloc_and_dissolve_huge_page retries when the page is about to be freed. Here we retry in case alloc_and_dissolve_huge_page() noticed that someone grabbed the page (refcount 0 -> 1), which would possibly mean userspace started using this page. If that is the case, we give isolate_huge_page another chance to see if we can succeed and we can migrate the page. Chances this to happen? Honestly, as any race, quite low. So, the answer to your question would be, no, it is not really needed, I just felt we could play "clever" here. -- Oscar Salvador SUSE L3