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=-8.8 required=3.0 tests=BAYES_00, 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 62E86C433E0 for ; Thu, 4 Mar 2021 10:19:42 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F0F3B64F27 for ; Thu, 4 Mar 2021 10:19:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F0F3B64F27 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 8BECC6B0008; Thu, 4 Mar 2021 05:19:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 86E386B000C; Thu, 4 Mar 2021 05:19:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 75DC66B000D; Thu, 4 Mar 2021 05:19:41 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0168.hostedemail.com [216.40.44.168]) by kanga.kvack.org (Postfix) with ESMTP id 58B376B0008 for ; Thu, 4 Mar 2021 05:19:41 -0500 (EST) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 164A38249980 for ; Thu, 4 Mar 2021 10:19:41 +0000 (UTC) X-FDA: 77881795362.07.19F7B67 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf24.hostedemail.com (Postfix) with ESMTP id D7750A0009C5 for ; Thu, 4 Mar 2021 10:19:38 +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 24362AAC5; Thu, 4 Mar 2021 10:19:39 +0000 (UTC) Date: Thu, 4 Mar 2021 11:19:36 +0100 From: Oscar Salvador To: David Hildenbrand Cc: Andrew Morton , Mike Kravetz , Muchun Song , Michal Hocko , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/2] mm: Make alloc_contig_range handle free hugetlb pages Message-ID: References: <20210222135137.25717-1-osalvador@suse.de> <20210222135137.25717-2-osalvador@suse.de> <3f071dd4-3181-f4e0-fd56-1a70f6ac72fe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3f071dd4-3181-f4e0-fd56-1a70f6ac72fe@redhat.com> X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: D7750A0009C5 X-Stat-Signature: q8ra17fywqqubftogwbsg8c96qabyznw Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf24; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1614853178-508953 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 Mon, Mar 01, 2021 at 03:09:06PM +0100, David Hildenbrand wrote: > On 22.02.21 14:51, Oscar Salvador wrote: > > @@ -905,6 +905,18 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > > valid_page = page; > > } > > + if (PageHuge(page) && cc->alloc_contig) { > > + if (!isolate_or_dissolve_huge_page(page)) > > + goto isolate_fail; Bleh, sorry for the lateness David, I was farly busy. > So, the callchain is: > > alloc_contig_range()->__alloc_contig_migrate_range()->isolate_migratepages_range()->isolate_migratepages_block() > > The case I am thinking about is if we run out of memory and would return > -ENOMEM from alloc_and_dissolve_huge_page(). We silently drop the real error > (e.g., -ENOMEM vs. -EBUSY vs. e.g., -EAGAIN) we had in > isolate_or_dissolve_huge_page(). Yes, that is true. > I think we should not swallo such return values in > isolate_or_dissolve_huge_page() and instead properly report esp. -ENOMEM > properly up this callchain now. Otherwise we'll end up retrying / reporting > -EBUSY, which is misleading. I am not sure I follow you here. So, atm, alloc_and_dissolve_huge_page can either generate -ENOMEM or -EBUSY wrt. error codes. -ENOMEM when we cannot allocate a page, and -EBUSY when we raced with someone. You mean to only report ENOMEM down the chain? > From isolate_migratepages_range()/isolate_migratepages_block() we'll keep > reporting "pfn > 0". > > a) In isolate_migratepages_range() we'll keep iterating over pageblocks > although we should just fail with -ENOMEM right away. > > b) In __alloc_contig_migrate_range() we'll keep retrying up to 5 times > although we should just fail with -ENOMEM. We end up returning "-EBUSY" > after retrying. > > c) In alloc_contig_range() we'll continue trying to isolate although we > should just return -ENOMEM. Yes, "fatal" errors get masked, and hence we treat everything as "things are busy, let us try again", and this is rather unforunate. > I think we have should start returning proper errors from > isolate_migratepages_range()/isolate_migratepages_block() on critical issues > (-EINTR, -ENOMEM) instead of going via "!pfn vs. pfn" and retrying on "pfn". > > So we should then fail with -ENOMEM during isolate_migratepages_range() > cleanly, just as we would do when we get -ENOMEM during migrate_pages(). I guess we could rework the interface and make isolate_migratepages_range and isolate_migratepages_block to report the right thing. I yet have to check that this does not mess up a lot with the compaction interface. But overall I agree with your point here, and I am willing to to tackle this. The question is whether we want to do this as part of this series, or after this series gets picked up. IMHO, we could do it after, as a follow-up, unless you feel strong about it. What do you think? -- Oscar Salvador SUSE L3