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 57E58C433E0 for ; Thu, 18 Mar 2021 09:59:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B8F0164EF9 for ; Thu, 18 Mar 2021 09:59:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B8F0164EF9 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 4841E6B0070; Thu, 18 Mar 2021 05:59:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 40A966B0071; Thu, 18 Mar 2021 05:59:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2AE546B0072; Thu, 18 Mar 2021 05:59:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0152.hostedemail.com [216.40.44.152]) by kanga.kvack.org (Postfix) with ESMTP id 0F7E16B0070 for ; Thu, 18 Mar 2021 05:59:16 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id C40903649 for ; Thu, 18 Mar 2021 09:59:15 +0000 (UTC) X-FDA: 77932547070.26.E4E0C59 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf29.hostedemail.com (Postfix) with ESMTP id 1C671E6 for ; Thu, 18 Mar 2021 09:59:15 +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 E76FEAC1E; Thu, 18 Mar 2021 09:59:13 +0000 (UTC) Date: Thu, 18 Mar 2021 10:59:10 +0100 From: Oscar Salvador To: Michal Hocko Cc: Andrew Morton , Vlastimil Babka , David Hildenbrand , Muchun Song , Mike Kravetz , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 4/5] mm: Make alloc_contig_range handle in-use hugetlb pages Message-ID: References: <20210317111251.17808-1-osalvador@suse.de> <20210317111251.17808-5-osalvador@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: gy63bwxtcy8r45drsy64br9o47oyahnd X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 1C671E6 Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf29; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1616061555-411424 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, Mar 18, 2021 at 10:29:57AM +0100, Michal Hocko wrote: > On Thu 18-03-21 09:54:01, Oscar Salvador wrote: > [...] > > @@ -2287,10 +2288,12 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page) > > goto unlock; > > } else if (page_count(old_page)) { > > /* > > - * Someone has grabbed the page, fail for now. > > + * Someone has grabbed the page, try to isolate it here. > > + * Fail with -EBUSY if not possible. > > */ > > - ret = -EBUSY; > > update_and_free_page(h, new_page); > > + if (!isolate_huge_page(old_page, list) > > + ret = -EBUSY; > > goto unlock; > > } else if (!HPageFreed(old_page)) { > > I do not think you want to call isolate_huge_page with hugetlb_lock > held. You would need to drop the lock before calling isolate_huge_page. Yeah, that was an oversight. As I said I did not compile it(let alone test it), otherwise the system would have screamed I guess. I was more interested in knowing whether how did it look wrt. retry concerns: diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index bcff86ca616f..a37b4ce86e58 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -583,7 +583,7 @@ struct huge_bootmem_page { struct hstate *hstate; }; -int isolate_or_dissolve_huge_page(struct page *page); +int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list); struct page *alloc_huge_page(struct vm_area_struct *vma, unsigned long addr, int avoid_reserve); struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid, @@ -866,7 +866,8 @@ static inline void huge_ptep_modify_prot_commit(struct vm_area_struct *vma, #else /* CONFIG_HUGETLB_PAGE */ struct hstate {}; -static inline int isolate_or_dissolve_huge_page(struct page *page) +static inline int isolate_or_dissolve_huge_page(struct page *page, + struct list_head *list) { return -ENOMEM; } diff --git a/mm/compaction.c b/mm/compaction.c index 9f253fc3b4f9..6e47855fd154 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -910,7 +910,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, } if (PageHuge(page) && cc->alloc_contig) { - ret = isolate_or_dissolve_huge_page(page); + ret = isolate_or_dissolve_huge_page(page, &cc->migratepages); /* * Fail isolation in case isolate_or_dissolve_huge_page @@ -927,6 +927,15 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, goto isolate_fail; } + if (PageHuge(page)) { + /* + * Hugepage was successfully isolated and placed + * on the cc->migratepages list. + */ + low_pfn += compound_nr(page) - 1; + goto isolate_success_no_list; + } + /* * Ok, the hugepage was dissolved. Now these pages are * Buddy and cannot be re-allocated because they are @@ -1068,6 +1077,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, isolate_success: list_add(&page->lru, &cc->migratepages); +isolate_success_no_list: cc->nr_migratepages += compound_nr(page); nr_isolated += compound_nr(page); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 3194c1bd9e32..f55fa6acc6f9 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2257,7 +2257,8 @@ static void restore_reserve_on_error(struct hstate *h, * Returns 0 on success, otherwise negated error. */ -static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page) +static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page, + struct list_head *list) { gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE; int nid = page_to_nid(old_page); @@ -2287,10 +2288,14 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page) goto unlock; } else if (page_count(old_page)) { /* - * Someone has grabbed the page, fail for now. + * Someone has grabbed the page, try to isolate it here. + * Fail with -EBUSY if not possible. */ - ret = -EBUSY; update_and_free_page(h, new_page); + spin_unlock(&hugetlb_lock); + if (!isolate_huge_page(old_page, list) + ret = -EBUSY; + spin_lock(&hugetlb_lock); goto unlock; } else if (!HPageFreed(old_page)) { /* @@ -2319,10 +2324,11 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page) return ret; } -int isolate_or_dissolve_huge_page(struct page *page) +int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list) { struct hstate *h; struct page *head; + int ret = -EBUSY; /* * The page might have been dissolved from under our feet, so make sure @@ -2347,7 +2353,12 @@ int isolate_or_dissolve_huge_page(struct page *page) if (hstate_is_gigantic(h)) return -ENOMEM; - return alloc_and_dissolve_huge_page(h, head); + if (page_count(head) && isolate_huge_page(head, list)) + ret = 0; + else if (!page_count(head)) + ret = alloc_and_dissolve_huge_page(h, head, list); + + return ret; } struct page *alloc_huge_page(struct vm_area_struct *vma, diff --git a/mm/vmscan.c b/mm/vmscan.c index 562e87cbd7a1..42aaef30633e 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1507,8 +1507,9 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, LIST_HEAD(clean_pages); list_for_each_entry_safe(page, next, page_list, lru) { - if (page_is_file_lru(page) && !PageDirty(page) && - !__PageMovable(page) && !PageUnevictable(page)) { + if (!PageHuge(page) && page_is_file_lru(page) && + !PageDirty(page) && !__PageMovable(page) && + !PageUnevictable(page)) { ClearPageActive(page); list_move(&page->lru, &clean_pages); } The spin_lock after the isolate_huge_page() in alloc_and_dissolve_huge_page() could probably be spared by placing a goto out directly before the return. But just a POC. -- Oscar Salvador SUSE L3