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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C13DCA0ED3 for ; Mon, 2 Sep 2024 10:44:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C272F8D00B9; Mon, 2 Sep 2024 06:44:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BAE988D0098; Mon, 2 Sep 2024 06:44:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A284D8D00B9; Mon, 2 Sep 2024 06:44:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 80A5A8D0098 for ; Mon, 2 Sep 2024 06:44:18 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 32767161C81 for ; Mon, 2 Sep 2024 10:44:18 +0000 (UTC) X-FDA: 82519463796.30.F9E6AD0 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by imf08.hostedemail.com (Postfix) with ESMTP id E851A16000E for ; Mon, 2 Sep 2024 10:44:14 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725273782; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GwIHh9tnM1nYJ4PUFVmASzxJazOGUrjXd4LChf14K+g=; b=8G7M88A+fk91hGunlqwXZcGf0a4Jlw4DMhDBg1TPuY6hM263hsOj9kP9kEU4vuMh8K5ve0 zfA4GQy7Iyh7LKCsXrG3juqhecCFyMfy9mZkRR4nAW1PjpyFs8dANPnytPJf9lSwwZIkrO 9+lbpaP87Wpkx9sKjVOE3F0M/ZiD7LY= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725273782; a=rsa-sha256; cv=none; b=Y8Sc3fKrZ7HqJvOkSTeSNr71TbasJyJYRiWFeE4aVLUQ4q0XvDnfzx8zFS13qoZ4GMilby vsfuJ1KXA0ZafSikvooxQF8TzEUqczbMJsUiirhJypvnJu0L9L68FyFWxy/IFUz1nNIR5d G7lC0yKRsU387T0r5B5va87dSEQr0S4= Received: from mail.maildlp.com (unknown [172.19.162.254]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Wy5191QJxzpVFG; Mon, 2 Sep 2024 18:42:21 +0800 (CST) Received: from dggpemf100008.china.huawei.com (unknown [7.185.36.138]) by mail.maildlp.com (Postfix) with ESMTPS id CA3BE180106; Mon, 2 Sep 2024 18:44:10 +0800 (CST) Received: from [10.174.177.243] (10.174.177.243) by dggpemf100008.china.huawei.com (7.185.36.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 2 Sep 2024 18:44:10 +0800 Message-ID: Date: Mon, 2 Sep 2024 18:44:09 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/5] mm: compaction: get reference before non LRU movable folio isolation Content-Language: en-US To: Matthew Wilcox CC: Andrew Morton , David Hildenbrand , Baolin Wang , Zi Yan , Vishal Moola , References: <20240829145456.2591719-1-wangkefeng.wang@huawei.com> <20240829145456.2591719-2-wangkefeng.wang@huawei.com> From: Kefeng Wang In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpemf100008.china.huawei.com (7.185.36.138) X-Stat-Signature: wa8w16c7k4699jh3k9i9hpj71g83bw19 X-Rspam-User: X-Rspamd-Queue-Id: E851A16000E X-Rspamd-Server: rspam02 X-HE-Tag: 1725273854-488503 X-HE-Meta: U2FsdGVkX194S49i4fkL2odEdIMdjHPVXZcxgXnzz+EwUWddfmBU9oN9+mO00ns3mcZI+oqOeI2u47vjrqiq4WSgvQJOL4k7Zw+mSvMGeijeEb6g7ZCHUli+jCLKNPxCxDCblha8bOlYLSLCXzlIiuravNsVQh/dpq6PPfGgvHL/J5UuStio6ZLYrrZk0e3Wr8etqoGdriy44sDJsQ1xOEYKbvHWtbrX5Gx4dZ5KvVm0YGaJbLOs5DW5nwksxszSQOFsEcFpoV946ro6fg57QGTpwR/Xasuz0bbdE7x6VG3HXJdPg05FAarFwZlffoIIIZqUgSTXTHbWTWF5F5QVkjPuS3SCy7AovflZF0geYldmE4Zli3rC2/5U64KhYj7g1BfsfBIzSmb0ZOs2hpYdO7Jz6LlQMT2IsY2idmoKS2/4r8oZRCNObj/SpUf9KDmfpZSSDJJJhYn91gEDlu/1hkFzRtYCpTF03tT2nYVB7GgHA7W+xKa/7kRf5hBZbKJVytBv3xhCyeBts0MhG2d2sXPI7wefZYAEMLIDuVAg+GVoSnUBbazq+ty5TwzWuOhxRoYzj9e07nxIZ9CKKco5cGTAwplvUTcnmvLDDBXNYflcXXmvNNNFy4VgdKKzGhJp1sgVtKFG8x4fpOKQdUVhFv2yQE7ZL10+HyNeaJkVzFd+RkWhBL1jEWemzXPSJQpGsx0l3PP87We1Wkla156QFh5nSGZn1+ppQ9PX/HZGieug4tCFyCcn/kjmxpdGaEs7k0ChgRg/+dy5L8L6XKGda6saTkTgbu3FD4xNjrfkYwzI2Qz901nbd3MfdTPpWEhwTZ68hO4l7XGePukHqvoMQbYqvgKWZSWwhemwXW84WgTmXAbqFQ+zeNpGh5L1jGw0Nx5Z5RmZfyHTBh5/bW3Od6zEgjmJN32qfOyCcepQOgKLrNHd/iLAzFMWyGutwV1Dm026Dr8p+qf/BMpzZpe VFyg4GCx JETc57cCqrPZTrSsI85akNWomVWOfRg5pykC/cI9NgK0tiMhIgeRiCIyklRh8I7FEn92CQUY07AgKNz5H6swmEAAzF7MEBorZiKsOAFRXktn/wNJf2SJz1dEKqNm5oLqM5ox4m1ATzliCtxSJT0t1z5Wx3yibmVRuqkS/ShDLsZkZSKpzMuvQsMQ7ifiEfDBf//+6b2YRz/aGbVlbM9Rh1SGrN6FBxPjwAOWmBguSAVK06aQ1R0R4gZnGJPft2iS0ZEa88a4oDayEuDA= 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: List-Subscribe: List-Unsubscribe: On 2024/8/31 22:04, Matthew Wilcox wrote: > On Thu, Aug 29, 2024 at 10:54:52PM +0800, Kefeng Wang wrote: >> Non-LRU movable folio isolation will fail if it can't grab a reference >> in isolate_movable_page(), so folio_get_nontail_page() could be called >> ahead to unify the handling of non-LRU movable/LRU folio isolation a bit, >> this is also prepare to convert isolate_movable_page() to take a folio. >> Since the reference count of the non-LRU movable folio is increased, >> a folio_put() is needed whether the folio is isolated or not. > > There's a reason I stopped where I did when converting this function > to use folios. Usually I would explain, but I think it would do you > good to think about why for a bit. Hm, I don't find the reason, The major change is that we move folio_get_nontail_page ahead, so we may try add a reference for each page, it always fails to isolate with/without this changes, so I suppose that there is no issue here, for other cases, PageBuddy/PageHuge is also handled before !PageLRU, although the check is lockless, we will re-check under lock. so is some page's reference can't try to grab or there are some special handling for movable page. The minimized changes could be something like below, just add a new folio_get_nontail_page() before isolate_movable_page(), other patches don't need to be changed. diff --git a/mm/compaction.c b/mm/compaction.c index a2b16b08cbbf..68331ba331e5 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -1091,9 +1091,13 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, locked = NULL; } - if (isolate_movable_page(page, mode)) { - folio = page_folio(page); - goto isolate_success; + folio = folio_get_nontail_page(page); + if (folio) { + if (isolate_movable_page(&folio->page, mode)) { + folio_put(folio); + goto isolate_success; + } + goto isolate_fail_put; } } but I am not clear about the reason why this has issue, could you share more tips, thank. > > Andrew, please drop these patches. > >> Signed-off-by: Kefeng Wang >> --- >> mm/compaction.c | 38 +++++++++++++++++++------------------- >> 1 file changed, 19 insertions(+), 19 deletions(-) >> >> diff --git a/mm/compaction.c b/mm/compaction.c >> index a2b16b08cbbf..8998574da065 100644 >> --- a/mm/compaction.c >> +++ b/mm/compaction.c >> @@ -1074,41 +1074,41 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, >> } >> } >> >> + /* >> + * Be careful not to clear lru flag until after we're >> + * sure the folio is not being freed elsewhere -- the >> + * folio release code relies on it. >> + */ >> + folio = folio_get_nontail_page(page); >> + if (unlikely(!folio)) >> + goto isolate_fail; >> + >> /* >> * Check may be lockless but that's ok as we recheck later. >> - * It's possible to migrate LRU and non-lru movable pages. >> - * Skip any other type of page >> + * It's possible to migrate LRU and non-lru movable folioss. >> + * Skip any other type of folios. >> */ >> - if (!PageLRU(page)) { >> + if (!folio_test_lru(folio)) { >> /* >> - * __PageMovable can return false positive so we need >> - * to verify it under page_lock. >> + * __folio_test_movable can return false positive so >> + * we need to verify it under page_lock. >> */ >> - if (unlikely(__PageMovable(page)) && >> - !PageIsolated(page)) { >> + if (unlikely(__folio_test_movable(folio)) && >> + !folio_test_isolated(folio)) { >> if (locked) { >> unlock_page_lruvec_irqrestore(locked, flags); >> locked = NULL; >> } >> >> - if (isolate_movable_page(page, mode)) { >> - folio = page_folio(page); >> + if (isolate_movable_page(&folio->page, mode)) { >> + folio_put(folio); >> goto isolate_success; >> } >> } >> >> - goto isolate_fail; >> + goto isolate_fail_put; >> } >> >> - /* >> - * Be careful not to clear PageLRU until after we're >> - * sure the page is not being freed elsewhere -- the >> - * page release code relies on it. >> - */ >> - folio = folio_get_nontail_page(page); >> - if (unlikely(!folio)) >> - goto isolate_fail; >> - >> /* >> * Migration will fail if an anonymous page is pinned in memory, >> * so avoid taking lru_lock and isolating it unnecessarily in an >> -- >> 2.27.0 >> >