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 0C225C54E67 for ; Fri, 15 Mar 2024 11:38:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7528580123; Fri, 15 Mar 2024 07:38:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6DAD1800B4; Fri, 15 Mar 2024 07:38:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5A2A180123; Fri, 15 Mar 2024 07:38:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 45C79800B4 for ; Fri, 15 Mar 2024 07:38:46 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id E19A1412CE for ; Fri, 15 Mar 2024 11:38:45 +0000 (UTC) X-FDA: 81899076210.05.A6C78DA Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf30.hostedemail.com (Postfix) with ESMTP id 2AD9680002 for ; Fri, 15 Mar 2024 11:38:43 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; spf=pass (imf30.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710502723; 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=NZ05qyoXCjalWfY4qtGxvMup9vrh+lJb9Fr3R49IqWs=; b=6onoG2BfmbL6c8ENKd0kTrAn+Ecvrna4CN9/wudwEgckZrmdPU/6NYRUXM5vYEYyEVYtKW 1ujspvvbhknmQch0ZIYKEZZPydoNwQ5hwRf6UhAky+poq6JrN+rYGIbEd3LTa4VGIzxPBw KOZOoy8meDc+MKXoIouAZA3ZJGom4SA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710502723; a=rsa-sha256; cv=none; b=C5tHCJt4F9C0v+ueANLN3s7PdJYZTBfv33EQfIF8uGX/hjCCvyNlHalzaODb9rNUnka7J8 cIsZbFK2SEHuxYqE1YXV8JBHPiaZDnJckbBQjEbr4i7uU6ix0vQUWdr8Csqm3jGnUjhGQZ lbzTgpR67e9uUyPtuedDt2kYIYWzObA= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; spf=pass (imf30.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 79BA5C15; Fri, 15 Mar 2024 04:39:17 -0700 (PDT) Received: from [10.57.69.160] (unknown [10.57.69.160]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 40C543F762; Fri, 15 Mar 2024 04:38:39 -0700 (PDT) Message-ID: <5cc147fa-492e-46c3-963d-2f6e6680dce9@arm.com> Date: Fri, 15 Mar 2024 11:38:37 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 5/6] mm: vmscan: Avoid split during shrink_folio_list() Content-Language: en-GB To: David Hildenbrand Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Yin Fengwei , Andrew Morton , Matthew Wilcox , Huang Ying , Gao Xiang , Yu Zhao , Yang Shi , Michal Hocko , Kefeng Wang , Barry Song <21cnbao@gmail.com>, Chris Li References: <20240311150058.1122862-1-ryan.roberts@arm.com> <20240311150058.1122862-6-ryan.roberts@arm.com> <1db242d3-5ff1-4ef5-b20a-578a317fa859@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 2AD9680002 X-Rspam-User: X-Stat-Signature: tgahgdy55785byr9dmdt4rkeezduim33 X-Rspamd-Server: rspam03 X-HE-Tag: 1710502723-959273 X-HE-Meta: U2FsdGVkX189j2KEhnENJxBlQEEsPU+W67bn/pArL/udRzhK2EyXVu0JUAol4vZ1hqZM+z5dw1fVvkLagZls06RNYt6v7/G6nYEnQDqHppeMR8+A6RvNUKR9P54Zh50rB4YflDmggT/ZfPCjUYRzkg4Ao3iRS/Zct+E3PrvyZXUNS8GRZjLGC2Yd0yNrfCW2OJbEH8lViKSdG4TBERYmH0EZYZ/Y7A5HfEBDP1j3M/u/y+eXjn7xaVyHlbO9G6Y1/NNZKcRh+P78wV0AMP5ePJwQmjmcCbWqGfRH2hzl+f/fVLDN82nKgZHfu6BVlNKKDdiJVHIbuOXd6I3cIm+n8pw8+Ya+tJHIbrGXzgg1WIy/kZ82G/qPv2oYd/IWh02WCpYp1H3ZQXUaRWDMLcaDXbVW1gWzOGHATeq/iXAvaL/A2M9ZL5PB+4dbqitoWURbHfHUOweefUpbL0mkKOsyvajAbZ5foVnVHpl154QT3l+ViZ1lv5HrOR08G8OrZIzrGDBM0GHha3XYixMMERRCNqO9UhiNFVAyz5erRbHr/440SCuF4UR7dlJ9MycOFqGoqz62CRQ0Czku8SgEKZB5/mX/h6zGQ4MpcStx4apFYnS5kqzcj+RsLpjdhEmH0ZL625G/o8vuEGVo3rDudM++86vi9r3P11rxVIz0Zbn2j85alaRi1xxO+WnHnK9HJxBq9Z56Yl9lU078bc4hOzIYpjY9LRmwCsx2UH0lIrMRTZdCdDvJHJhPn1Gj4w3Yzc7iTd0jaQyKHNOYFe/37jWMb0x5ua+YHuIttwI94WtatCd4tROFx0g2bSvSJdP5YZPg+up9TchiUmKxypXmFneWkdiwmKTJsy64AXJ3RzyvgwB3h37fiU4MzdGwKpRiDbSdEVHdkwsOphVb6W0wdnMnW4uuCAyunZbR9oK8cVGEqxCDBie4q+cDmruKgwELp4E6FvZbZFSY6It/Xd2IOGX lzzIy6gi m5Grl1hOLRod//Wq85uE9PqvFOiWZOSz765mFN6L722IGY5gugUzfxMNRkYdjqNnC1YsjK4yykjeNl0bArqjjXnZF9tBYNX9GyOZ5jSTRPqESa4XAkzbNt1TR0kVH4Pz9TWaGxJwB7qW0wyCxmEHLkAJ7g83tmTBD8WPtqRUtoTXs5dXIDRqBnyM6WEjvytZZ5VRSizDyNTxwJffJNJBSu1y/qY6aaARt/rb+gVugyAktOPYdxrlt+2uOjfktzo0zybFW0R40iuCnaS3iqL8ynWuFaGM55Ym1ImOyJo4a8DKK41MHnz0NL2XkmJqasx1TP5/ltKn7Aw+/WHDiAL23NqruYA== 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: Hi Yin Fengwei, On 15/03/2024 11:12, David Hildenbrand wrote: > On 15.03.24 11:49, Ryan Roberts wrote: >> On 15/03/2024 10:43, David Hildenbrand wrote: >>> On 11.03.24 16:00, Ryan Roberts wrote: >>>> Now that swap supports storing all mTHP sizes, avoid splitting large >>>> folios before swap-out. This benefits performance of the swap-out path >>>> by eliding split_folio_to_list(), which is expensive, and also sets us >>>> up for swapping in large folios in a future series. >>>> >>>> If the folio is partially mapped, we continue to split it since we want >>>> to avoid the extra IO overhead and storage of writing out pages >>>> uneccessarily. >>>> >>>> Signed-off-by: Ryan Roberts >>>> --- >>>>    mm/vmscan.c | 9 +++++---- >>>>    1 file changed, 5 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>> index cf7d4cf47f1a..0ebec99e04c6 100644 >>>> --- a/mm/vmscan.c >>>> +++ b/mm/vmscan.c >>>> @@ -1222,11 +1222,12 @@ static unsigned int shrink_folio_list(struct list_head >>>> *folio_list, >>>>                        if (!can_split_folio(folio, NULL)) >>>>                            goto activate_locked; >>>>                        /* >>>> -                     * Split folios without a PMD map right >>>> -                     * away. Chances are some or all of the >>>> -                     * tail pages can be freed without IO. >>>> +                     * Split partially mapped folios map >>>> +                     * right away. Chances are some or all >>>> +                     * of the tail pages can be freed >>>> +                     * without IO. >>>>                         */ >>>> -                    if (!folio_entire_mapcount(folio) && >>>> +                    if (!list_empty(&folio->_deferred_list) && >>>>                            split_folio_to_list(folio, >>>>                                    folio_list)) >>>>                            goto activate_locked; >>> >>> Not sure if we might have to annotate that with data_race(). >> >> I asked that exact question to Matthew in another context bt didn't get a >> response. There are examples of checking if the deferred list is empty with and >> without data_race() in the code base. But list_empty() is implemented like this: >> >> static inline int list_empty(const struct list_head *head) >> { >>     return READ_ONCE(head->next) == head; >> } >> >> So I assumed the READ_ONCE() makes everything safe without a lock? Perhaps not >> sufficient for KCSAN? > > Yeah, there is only one use of data_race with that list. > > It was added in f3ebdf042df4 ("THP: avoid lock when check whether THP is in > deferred list"). > > Looks like that was added right in v1 of that change [1], so my best guess is > that it is not actually required. > > If not required, likely we should just cleanup the single user. > > [1] > https://lore.kernel.org/linux-mm/20230417075643.3287513-2-fengwei.yin@intel.com/ Do you have any recollection of why you added the data_race() markup? Thanks, Ryan >