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 6DB9BC4345F for ; Thu, 11 Apr 2024 14:39:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E0B386B0085; Thu, 11 Apr 2024 10:39:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DBBAC6B0087; Thu, 11 Apr 2024 10:39:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CD14A6B008A; Thu, 11 Apr 2024 10:39:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id AC4276B0085 for ; Thu, 11 Apr 2024 10:39:35 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0B830C0BC1 for ; Thu, 11 Apr 2024 14:39:35 +0000 (UTC) X-FDA: 81997509510.27.8F6B56B Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf02.hostedemail.com (Postfix) with ESMTP id 4CE1080011 for ; Thu, 11 Apr 2024 14:39:32 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf02.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712846372; 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=/krf3bvpmTqcv6EaoOxn55uLx6wU5zx/Bf1iFv5YyQI=; b=YGqzWRRW+doxf6SN7Sp2DlrncrWSIvKWPsmiXQ4qQA30zT3z/EEdGdXk8q3QGKbBdAMjAS JT2lrjlyTPF0O02P4kzxjGTNltFONPBg7ukLgP28iYkM2IjwWEC0MEHJthnT3U5ZImUFnO WIO5WeQT7GsAVVJNTnrxuLB3nXR1oGA= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf02.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712846372; a=rsa-sha256; cv=none; b=BFcfAvUS0AUJAvRgrG3Ju7QEl9mZSYIpPzdoDu/QvyQauIEM1PGSdLgFcQWzDNDNOh+nFT 1aIbYnUGXOTRkGKw29i4LyJ74EjgHjKPGDOzKyLE7ElvCTx5Puoa7OiRwjWCIhuKWVWx1p G6nTbbQddGn9Os6CV/UWWz/VDGXDGUk= 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 1C159339; Thu, 11 Apr 2024 07:40:01 -0700 (PDT) Received: from [10.1.38.151] (XHFQ2J9959.cambridge.arm.com [10.1.38.151]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C2E6C3F64C; Thu, 11 Apr 2024 07:39:29 -0700 (PDT) Message-ID: <8d674b15-ef74-4d96-bc27-8794f744517c@arm.com> Date: Thu, 11 Apr 2024 15:39:28 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/2] mm/madvise: optimize lazyfreeing with mTHP in madvise_free Content-Language: en-GB To: Lance Yang Cc: akpm@linux-foundation.org, david@redhat.com, 21cnbao@gmail.com, mhocko@suse.com, fengwei.yin@intel.com, zokeefe@google.com, shy828301@gmail.com, xiehuan09@gmail.com, wangkefeng.wang@huawei.com, songmuchun@bytedance.com, peterx@redhat.com, minchan@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240408042437.10951-1-ioworker0@gmail.com> <20240408042437.10951-2-ioworker0@gmail.com> <38c4add8-53a2-49ca-9f1b-f62c2ee3e764@arm.com> <3cda8e87-7095-4aad-beb1-6a420912df34@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: qypgzbujwddfenmi9e4zpooys9mpniyb X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4CE1080011 X-HE-Tag: 1712846372-875961 X-HE-Meta: U2FsdGVkX1+sL6ru/uKVXcmqcn3sAy7jpmxAPkwoZ2Em6/4M0xfE7apj+cm20c2Ltwb56Wgmgm8I2DzcP7PulvjrHElMaQZ7x4jDhaoWymLUmEEnwDVqFG8p0I/m4E6/B5uXhltx3yv6G6N7bx9Wy3sX0k7ksvv2CU6RS04ZtqDH/4vY9K5BURAlubYQZEPAOwL4rJBg0a/8Wy9plGNf2unOCHkaT7uolT3kqgNQ3+9e+N0hd5bAzsqW+FnQuDao/W1EN45YyCy8jph8NPEOzlMKmvUVbhYNHzpF448B/Icj3j1q1NxSfwM/BaICxMOq017C9KkuWZcqdYqvzEWXxhO+v+q9ERPakAFgNCIjqKOeuy787GkrlfP/3NfFVumE+A584xBKpUv87aX2X7xBoMMjX3y/Tdv0ONB/o8X24yT6pnlirMPODRdjrQdFBfmDeROUpOVZd5Zp16uPHi/OEFuozc60ryfAJwFZJ4l0rfTv80BRzRMGsTYI/dDVkRSVd0PrdHmftaI4NS6sx8qT6TqM+gUdR0+MjeGPdi4wJp9Pf2lAis8woxYwQB4r/FGcjg+4HQ4MtyXTdjgBVjzadNb7eldwT7u5eK/JP8z3vYkTyvjX5U2G0YcDEO6snl5GJjT51VM4rk5xhkm6XI1eYjcZcaYqQHKTe+gxQ4sD4elqyiEiLkdukOGGCZAf8ReBXMrfJ7tOTSfGJoY0W+cSJDtP/rG2jH4UrLj56O3vQxrQ80Rj6ICq/Pupr119xMeqpMHyw0dqBGbLv/II/CjhnCHk76k3a+I6315L07Q4d58Ysjt4PfU0foyRahhU4orjTYYSvglIHkRgCD1PBf+726KYCvm1AjMoajmAvrKxZVac8hY4ltdzq34Mo4rmTZ/GDdoiuJJ4lNULl5pW1Ywh5dkAl48TGmpRu7I60iWGB8P/ntJvcZABhLyoAYLAccCgnRrvruUDLLfS9ry1/IR RSG6N90b kQBHv9CY4Q6i9D/2BNtTnQpIk1qh5Gc2J/B7qNz3z1Yg2r2a8rlE0bWgXozvuhZielkE9Kk5gygencf7DKhiiPBNMW1UevBJOahcN8BWjCHi174hB/yvQxgQv5U29mh/nQ2OSbkWjwmwxJQMI4pCTTVjJSVDW3CuyiNfyc3SYXylOJGZwwIkGfYJRI0HW9T6vgXa9cZj6Brb97okncX6SWVAcu8/truwepGPw5gTmm8zCM6dZmr3RzwcRwFgxMMlVJfYj4eEunZ1FGYCrF11RsRAxJAvB3ACuEBsfa+xMzij0g6+ULvLt8nk7if5FFe5swpuFUCVs2hHEJdYkNPzjxgBVYvqVP3X5j6YAM7d7c21LEHQ= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, 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 11/04/2024 15:07, Lance Yang wrote: > On Thu, Apr 11, 2024 at 9:48 PM Ryan Roberts wrote: >> >> [...] >> >>>>> + >>>>> + if (!folio_trylock(folio)) >>>>> + continue; >>>> >>>> This is still wrong. This should all be protected by the "if >>>> (folio_test_swapcache(folio) || folio_test_dirty(folio))" as it was previously >>>> so that you only call folio_trylock() if that condition is true. You are >>>> unconditionally locking here, then unlocking, then relocking below if the >>>> condition is met. Just put everything inside the condition and lock once. >>> >>> I'm not sure if it's safe to call folio_mapcount() without holding the >>> folio lock. >>> >>> As mentioned earlier by David in the v2[1] >>>> What could work for large folios is making sure that #ptes that map the >>>> folio here correspond to the folio_mapcount(). And folio_mapcount() >>>> should be called under folio lock, to avoid racing with swapout/migration. >>> >>> [1] https://lore.kernel.org/all/5cc05529-eb80-410e-bc26-233b0ba0b21f@redhat.com/ >> >> But I'm not suggesting that you should call folio_mapcount() without the lock. >> I'm proposing this: >> >> if (folio_test_swapcache(folio) || folio_test_dirty(folio)) { >> if (!folio_trylock(folio)) >> continue; >> /* >> - * If folio is shared with others, we mustn't clear >> - * the folio's dirty flag. >> + * If we have a large folio at this point, we know it is >> + * fully mapped so if its mapcount is the same as its >> + * number of pages, it must be exclusive. >> */ >> - if (folio_mapcount(folio) != 1) { >> + if (folio_mapcount(folio) != folio_nr_pages(folio)) { >> folio_unlock(folio); >> continue; >> } > > IIUC, if the folio is clean and not in the swapcache, we still need to > compare the number of batched PTEs against folio_mapcount(). Why? That's not how the old code worked. In fact the comment says that the reason for the exclusive check is to avoid marking a dirty *folio* as clean if shared; that would be bad because we could throw away data that others relied upon. It's perfectly safe to clear the dirty flag from the *pte* even if it is shared; the ptes are private to the process so that won't affect sharers. You should just follow the pattern already estabilished by the original code. The only difference is that because the folio is now (potentially) large, you have to change the way to detect exclusivity. > > Thanks, > Lance > >> >> What am I missing? >>