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=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,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 ADB90C433E6 for ; Thu, 18 Feb 2021 12:52:48 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1CAA161494 for ; Thu, 18 Feb 2021 12:52:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CAA161494 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 6F7076B0006; Thu, 18 Feb 2021 07:52:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A73F6B006C; Thu, 18 Feb 2021 07:52:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5BD836B006E; Thu, 18 Feb 2021 07:52:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0201.hostedemail.com [216.40.44.201]) by kanga.kvack.org (Postfix) with ESMTP id 463FE6B0006 for ; Thu, 18 Feb 2021 07:52:47 -0500 (EST) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 0C781365C for ; Thu, 18 Feb 2021 12:52:47 +0000 (UTC) X-FDA: 77831377974.02.30C53BE Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf08.hostedemail.com (Postfix) with ESMTP id 362E180192D4 for ; Thu, 18 Feb 2021 12:52:39 +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=1613652765; 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=my7Llvrq4fVu5U4ytT0HWufvyvA3sg6mSRU0ZcpXfMk=; b=lq1e6GlxqqQOwkGrgILpNZyyk4ZcCqyTqKsUuqrd4A7/PG+rF9zLpt2c8n2jE7yWA1TytV AnygHSvglydRiAV+fxNpBPVo5NcY8gqlbtUzLLu3CbgwTuLyn1mZLne0+1JkfWH4lLCpS4 Qemt6aycGuU9agUx4tklwXWiUrqpU5I= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 26511AD78; Thu, 18 Feb 2021 12:52:45 +0000 (UTC) Date: Thu, 18 Feb 2021 13:52:38 +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 1/2] mm: Make alloc_contig_range handle free hugetlb pages Message-ID: References: <20210217100816.28860-1-osalvador@suse.de> <20210217100816.28860-2-osalvador@suse.de> <20210218100917.GA4842@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210218100917.GA4842@localhost.localdomain> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 362E180192D4 X-Stat-Signature: 3zayi3wz3p3ksds9ugyzwhfzr8kau8c9 Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf08; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1613652759-238554 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 Thu 18-02-21 11:09:17, Oscar Salvador wrote: > On Wed, Feb 17, 2021 at 04:00:11PM +0100, Michal Hocko wrote: > > Is this really necessary? dissolve_free_huge_page will take care of this > > and the race windown you are covering is really tiny. > > Probably not, I was trying to shrink to race window as much as possible > but the call to dissolve_free_huge_page might be enough. > > > > + nid = page_to_nid(page); > > > + spin_unlock(&hugetlb_lock); > > > + > > > + /* > > > + * Before dissolving the page, we need to allocate a new one, > > > + * so the pool remains stable. > > > + */ > > > + new_page = alloc_fresh_huge_page(h, gfp_mask, nid, nmask, NULL); > > > > wrt. fallback to other zones, I haven't realized that the primary > > usecase is a form of memory offlining (from virt-mem). I am not yet sure > > what the proper behavior is in that case but if breaking hugetlb pools, > > similar to the normal hotplug operation, is viable then this needs a > > special mode. We do not want a random alloc_contig_range user to do the > > same. So for starter I would go with __GFP_THISNODE here. > > Ok, makes sense. > __GFP_THISNODE will not allow fallback to other node's zones. > Since we only allow the nid the page belongs to, nodemask should be > NULL, right? I would have to double check because hugetlb has a slightly different expectations from nodemask than the page allocator. The later translates that to all possible nodes but hugetlb API tries to dereference nodes. Maybe THIS node special cases it somewhere. > > > + if (!h) > > > + /* > > > + * The page might have been dissolved from under our feet. > > > + * If that is the case, return success as if we dissolved it > > > + * ourselves. > > > + */ > > > + return true; > > > > nit I would put the comment above the conditin for both cases. It reads > > more easily that way. At least without { }. > > Yes, makes sense. > > > Other than that I haven't noticed any surprises. > > I did. The 'put_page' call should be placed above, right after getting > the page. Otherwise, refcount == 1 and we will fail to dissolve the > new page if we need to (in case old page fails to be dissolved). > I already fixed that locally. I am not sure I follow. newly allocated pages is unreferenced unconditionally and the old page is not referenced by this path. -- Michal Hocko SUSE Labs