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.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 79089C433E0 for ; Fri, 26 Feb 2021 12:46:25 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C0C6A64DFF for ; Fri, 26 Feb 2021 12:46:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C0C6A64DFF Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 294806B0005; Fri, 26 Feb 2021 07:46:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2447C6B0006; Fri, 26 Feb 2021 07:46:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 159CF6B006C; Fri, 26 Feb 2021 07:46:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0126.hostedemail.com [216.40.44.126]) by kanga.kvack.org (Postfix) with ESMTP id 00F986B0005 for ; Fri, 26 Feb 2021 07:46:23 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id B050E181D1305 for ; Fri, 26 Feb 2021 12:46:23 +0000 (UTC) X-FDA: 77860392246.25.B0BC750 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf21.hostedemail.com (Postfix) with ESMTP id 8AC5BE0011F7 for ; Fri, 26 Feb 2021 12:46:19 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1614343582; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=bdJY5RFMgsg1qZIGV+8We7dBBKJWSZLg3sOQFhNuLP8=; b=jABI7NJ4rsC5M3reB3V+ElmNWY5++2uJ1Czpgd4k+paqTQD7B3e2p3E6oTgyL93dWdKYjy RU8F9tDyXQN1xeZkOAYYPCGB8yPf7CCTbgx6PQxT1ZBMTjnUiHgsSbtqUohN0SKErrtupY bhto//iq/7vn8l3+rla0KfzXJhdY0ZM= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id DB503AF1E; Fri, 26 Feb 2021 12:46:21 +0000 (UTC) Date: Fri, 26 Feb 2021 13:46:21 +0100 From: Michal Hocko To: Oscar Salvador 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: References: <20210222135137.25717-1-osalvador@suse.de> <20210222135137.25717-3-osalvador@suse.de> <20210226102424.GA3557@linux> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210226102424.GA3557@linux> X-Stat-Signature: hyz4mgkdcc7ebgieabpzqxt77siwt4yk X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 8AC5BE0011F7 Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf21; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1614343579-272090 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 26-02-21 11:24:29, Oscar Salvador wrote: > 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. Well, I will leave it to others. I do not feel strongly about this but to me it makes the code harder to think about because the situation is unstable and any of those condition can change as they are evaluated. So an explicit checks makes the code harder in the end. I would simply got with if (isolate_huge_page(head, list) || !alloc_and_dissolve_huge_page()) ret = true; if either of the conditional needs a retry then it should be done internally. Like alloc_and_dissolve_huge_page already does to stabilize the PageFreed flag. An early bail out on non-free hugetlb page would also better be done inside alloc_and_dissolve_huge_page. -- Michal Hocko SUSE Labs