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 E3AC3C4345F for ; Thu, 11 Apr 2024 13:48:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 79D386B0082; Thu, 11 Apr 2024 09:48:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 772116B0099; Thu, 11 Apr 2024 09:48:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 63A936B009A; Thu, 11 Apr 2024 09:48:59 -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 3DC436B0099 for ; Thu, 11 Apr 2024 09:48:59 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 0A5E3A0B9E for ; Thu, 11 Apr 2024 13:48:59 +0000 (UTC) X-FDA: 81997381998.07.B5514BF Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf13.hostedemail.com (Postfix) with ESMTP id 268BF2001A for ; Thu, 11 Apr 2024 13:48:56 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf13.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=1712843337; 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=8EmSHNru08Qgpd+bZkkxBxH75B40CEqU79I1xOw9buk=; b=1MHW0WlmKcQQiRYiecS+B2Q2OFKkazZnZixoCO6ljO8GDluSl5KtBHhVTCzxk0v2uH/jrc tiIA8l2p/1v+c8S8Znm1mbB/Njo0Y9PI1s1tfTgLhCqR21aLB4W64edbkVXguenwHKd6QI aV7Ai2VmcgbCfkgkbxZa0ZyxUVcOEJo= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf13.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=1712843337; a=rsa-sha256; cv=none; b=0qkWJgobpSkdLdowRF9X+YaOcX8ktPqkQZFFVdO72Uruu01lQiVSy0vX3O5HPg9jQP3S7C fZu733bb1adSn0ksadacb6YuKF5lJHnCCP4VhZ8eOAH/dx2XwXb1ny1dEL2qHlNVXdikBs t+iQbudoBbstoymafoTGyd3Pihw31X4= 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 BA5C9339; Thu, 11 Apr 2024 06:49:25 -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 D76F83F64C; Thu, 11 Apr 2024 06:48:53 -0700 (PDT) Message-ID: <3cda8e87-7095-4aad-beb1-6a420912df34@arm.com> Date: Thu, 11 Apr 2024 14:48:52 +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> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 268BF2001A X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: ck3nk514sp6jictgpk8389k319mthrdb X-HE-Tag: 1712843336-875478 X-HE-Meta: U2FsdGVkX1/fqPB7SF1d6NRS0kGwaJVHMO44mcssKxdTJ+JJqiVZQkLZ3Ncohal4tn1v8UwGxzJ6kCA3QXJdo95aeqa3AghS2XPG81Gcx5hcLexCxSQiNMt7rzajgfcLVrCIaBe+sIZljc86LhSQQfXUx5Jvzg/xdatxaqEvZ+uuVhQZm/1W7M+mlqF+6C8AAndLYJ9abew6M4OgjQ0CluUWWaRYUFTUqha0VFk+kOAxjt20hKfbUj4cKdQ/aSZxYhCnyUK/bl8xJiEVfpC+LXGXumPkDjeSB4Kf0iP4Jx/kNKuCbG176JmvJtJuqPf/2vgDDJyW2Wm3Xt4QyrUrZsDb/gi2ESuMlNe64fI51Ttu1tfBWKj5cqxXqLgCUKJVGbLoXI6B78r1EyDicnFJuMR86NYE/nutOc8YDEsB3ctLDBn6n21Iqklu9VypRaFQlVumAxmnMpQtyCPvj1vjmmtWBnhP2k8LtZuhRtGodozmTxoTs05IdTQgdi6BIL+6lteojjY57SEaNwfcodAiQceuhtEJ4ruIo4RVyEolZ1/VrIEsmaM8PNZpt4NLRwaJ3JaxH4OILE12FsluT0of9Y9SbgpziuCJRYlqf7QmSIOcZoE8U/zfS6dmAc0xoW3dquuY7pnOKWBvIrjicBkVntai0IFhi48+Ly+YuJcz549htNFEWZmzmEjwdUXVF9qk/ugLffBZ5O1OEgrnk/QGohKYaCdJs+DINI9pqJ5DgrAsXVHIfRlNGB6Cl1kyVgDCudicSlBATSJQqT3q1W+PZVsBI2UukGMIRzv8mLYO7bKtZhj4Z4H0T+DGh0qYMZeBoGrs7v890lISnKgUTvrsSnPWksieu0jO9MBr2PKoDXGLDkkfkFjEnRVunCzbRSojtuyvnNdT7KBVUDAOPfNh/DYIva4CKQPtVdBvXrbyKaX2a3wu3vBcuVTb4HFs14UtLI4GZ7duFrqDwXgWtBz MTPKYjZs oktZ3Id7XiDTsXa6IHK5QClNlnh9Hg+m1gfyH4ozT3vTV/3HTfW8rezP9CHR4/i6wuOaLPh+ztnfm6VPe8LVO0JF4MTy8h45dBShm2yaXkZSrEnuiTlTXc0RrPqCS74nu9TIh4157kqwwqeUHicepZHJaoJB9rhcai/PWxW3n54zDmNlPRmtbNjVgA0mDJazkfkKhw8wHW966ODR9PudLX+0oBhfvCwDmZE5DVeTaGd5d2w+9SXtCzbf9givltugXbpDyx4ZK66kpBH5KRChr4RTgQg== 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: [...] >>> + >>> + 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; } What am I missing?