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 14C1EC433DB for ; Thu, 18 Feb 2021 13:59:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7628E64E33 for ; Thu, 18 Feb 2021 13:59:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7628E64E33 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 CFE4A6B0006; Thu, 18 Feb 2021 08:59:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CD55F6B006C; Thu, 18 Feb 2021 08:59:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BEC1F6B006E; Thu, 18 Feb 2021 08:59:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0034.hostedemail.com [216.40.44.34]) by kanga.kvack.org (Postfix) with ESMTP id A8D866B0006 for ; Thu, 18 Feb 2021 08:59:43 -0500 (EST) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 73F00180AD83A for ; Thu, 18 Feb 2021 13:59:43 +0000 (UTC) X-FDA: 77831546646.06.game44_211740027655 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id 3FFF710062CBC for ; Thu, 18 Feb 2021 13:59:43 +0000 (UTC) X-HE-Tag: game44_211740027655 X-Filterd-Recvd-Size: 4820 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf31.hostedemail.com (Postfix) with ESMTP for ; Thu, 18 Feb 2021 13:59:42 +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=1613656781; 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=lgTJXDtHZ7f4SQvSOSpC+i5e37PXzyHNAje7FYKp6bw=; b=JiMLA3jObClwxXa9KllaXZCzOxuB7TceARQau9wmKvnRrtqxycVGASVLWo3jg1tw324L/h uI7mfT14P8CM/KIV2KgJEbTo/jahFQ31xphpsKgRzq0IU50bHlVQppfD0vV7rg10WmvAK1 vcxjoWKweF803EXgNPEs8mXfSS2WdhY= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3C929AF4C; Thu, 18 Feb 2021 13:59:41 +0000 (UTC) Date: Thu, 18 Feb 2021 14:59:40 +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> <20210218133250.GA7983@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210218133250.GA7983@localhost.localdomain> 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 14:32:50, Oscar Salvador wrote: > On Thu, Feb 18, 2021 at 01:52:38PM +0100, Michal Hocko wrote: > > > 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. > > Uhm, I do not quite follow here. > AFAICS, alloc_fresh_huge_page->alloc_buddy_huge_page does nothing > with the nodemask, bur rather with nodes_retry mask. That is done > to not retry on a node we failed to allocate a page. > > Now, alloc_buddy_huge_page calls __alloc_pages_nodemask directly. > If my understanding is correct, it is ok to have a null nodemask > as __next_zones_zonelist() will go through our own zonelist, > since __GFP_THISNODE made us take ZONELIST_NOFALLBACK. As I've said. Page allocator can cope with NULL nodemask just fine. I have checked the code and now remember the tricky part. It is alloc_gigantic_page which cannot work with NULL nodemask because it relies on for_each_node_mask and that, unlike zonelist iterator, cannot cope with NULL node mask. This is the case only for !GFP_THISNODE. > Actually, I do not see how passing a non-null nodemask migth have > helped there, unless we allow to specify more nodes. No, nodemask is doesn't make any difference. > > > 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. > > Current code is: > > allocate_a_new_page (new_page's refcount = 1) > dissolve_old_page > : if fail > dissolve_new_page (we cannot dissolve it refcount != 0) > put_page(new_page); OK, new_page would go to the pool rather than get freed as this is neither a surplus nnor a temporary page. > It should be: > > allocate_a_new_page (new_page's refcount = 1) > put_page(new_page); (new_page's refcount = 0) > dissolve_old_page > : if fail > dissolve_new_page (we can dissolve it as refcount == 0) > > I hope this clarifies it . OK, I see the problem now. And your above solution is not really optimal either. Your put_page would add the page to the pool and so it could be used by somebody. One way around it would be either directly manipulating reference count which is fugly or you can make it a temporal page (alloc_migrate_huge_page) or maybe even better not special case this here but rather allow migrating free hugetlb pages in the migrate_page path. -- Michal Hocko SUSE Labs