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 7D47DC48BF6 for ; Mon, 26 Feb 2024 05:05:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C7A256B00B0; Mon, 26 Feb 2024 00:05:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C032E940008; Mon, 26 Feb 2024 00:05:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A56876B0160; Mon, 26 Feb 2024 00:05:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 89B786B00B0 for ; Mon, 26 Feb 2024 00:05:57 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3CAAD1605DC for ; Mon, 26 Feb 2024 05:05:57 +0000 (UTC) X-FDA: 81832767954.22.666FC9B Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) by imf01.hostedemail.com (Postfix) with ESMTP id 710FF40006 for ; Mon, 26 Feb 2024 05:05:55 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aulHpLzV; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708923955; 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:dkim-signature; bh=n0sV/ZkJed5YTOmJzKp3YfxIPpujeOY2hnQ4Z72HoPQ=; b=RyPxyhqWUUX8VAlQNc4xs5VgOD66SZkUQ74mQ5WKR+7xs81TqF0Sm0Chl9alfd/J6RBsGN uIFELQGMLMTAHKT8S7V4PriHf1o1gIa16LRj0lE3+UKKxLaCVbWZgKl/6yBMV1iAO9TPRD UrF+uKjFEefJ3/fid+KaCMiAEtlXaGs= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aulHpLzV; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708923955; a=rsa-sha256; cv=none; b=VDiHzwstpgOBPokmZ1J7OS+q1kAscG9SuAao9g/jgVz4hlXTkeR7QVeCwO2aQK7GdEuE2o PeGLVmdFv8RfrrY0SANu+D4Bd4kAMqmDh6FpkpFUo6E2AaNOsSk8cARCgH56sqk6qzkTw3 DrS43ERwzFbg+MweYKotnfjreIJLXNo= Received: by mail-ua1-f44.google.com with SMTP id a1e0cc1a2514c-7d5fce59261so1798606241.3 for ; Sun, 25 Feb 2024 21:05:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708923954; x=1709528754; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=n0sV/ZkJed5YTOmJzKp3YfxIPpujeOY2hnQ4Z72HoPQ=; b=aulHpLzVQRt8e4MHEOcdkhoOuhWUsN+VS5mV1DQxPyaAF4MZO7sM/gqwOCFOf3LT9K vh+46tXsh7zigxqZ7EI2OyO4FdzjUCizzc+4ddV8Z+L3VZ1FRnybidieD+Lm1n2HfHha hiXs6jnX1vX5cI9FFS5gDL8cZwHAVwaO1Z5A1XEGHCYlULhd0jbjLjUkI6XrbMi5EdMP ODc2qX+bi7If93crHxSGEwBRZ0cFOh55St+Vj1wMcQMtJrlk/2Dd5FshD0Xsr2V8WNo7 spiTtEVJoxCuJ1qwNI4AcoRPgkrOc+6y2ltdeNDPByIeLT+OVFdcxP+4N1Tuq8ta1gnU PPBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708923954; x=1709528754; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=n0sV/ZkJed5YTOmJzKp3YfxIPpujeOY2hnQ4Z72HoPQ=; b=ryYTigMCCJVYemrWfeWQocpKkdPLT8zQWqN/FurtMFkYUqakGLFVnkJrOTqq78x7mH itn+eBmYM/s8EGWv8YJqPkPg5kccoxNMDJVY+rpgNbjI084Nn/DAYfkqFyvxU1QdYsA1 LjuNTh3IaK2sD0HsNagFh4QC/7XpU9GcFgNv0i2h5EtYp3owKQNxAbMZqYA+xgT03mpj cqLpoiyHt1Dn6/tql515rXCoSOYk5bPHL+ffHIWYB0EXoc3gYwvpKlOr3Ah7NkwrB5eA HB4O6f2mUCJH9dQMhy3IeAm86rqQ9oWJbmUHBjyMxWOPSMzQKAhjenwaipscZ023aY5w BdvA== X-Forwarded-Encrypted: i=1; AJvYcCUib3NEudvF7PeRCvWMwVxFEa1RRqrOXKbv8PzLAWVRBTbkIM21ETTNVI9hpoGF+2Kl8amLhlmf2dipNvSXDC+R6po= X-Gm-Message-State: AOJu0YwOG2WK/Ic38QSKgvzWYcYMgPTnVGfolBb7bPouJ9uIToRfjs6c QGVAaSfr9jOYWKAozkl44OLo5p/UGx2CfCYf1DmZ58AqWdHu+MuSHTn/ST6FqQw0I93lQT4BcTu p8UVI7P+IK9UTSZhFAJTa4qaruTM= X-Google-Smtp-Source: AGHT+IHBzo46moG0layxvLBfJZRHuu4bELDVi9idfDPBnL5nE0tDjUn+TE4R77py2A2yeUoxtrBM9/4KA9fJSj31Zsc= X-Received: by 2002:a05:6102:3f53:b0:470:4aa5:2c5b with SMTP id l19-20020a0561023f5300b004704aa52c5bmr4598628vsv.0.1708923954489; Sun, 25 Feb 2024 21:05:54 -0800 (PST) MIME-Version: 1.0 References: <20231025144546.577640-1-ryan.roberts@arm.com> <20240118111036.72641-1-21cnbao@gmail.com> <20240118111036.72641-6-21cnbao@gmail.com> <58efd8d4-28aa-45d6-b384-7463568b24bb@redhat.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Mon, 26 Feb 2024 18:05:43 +1300 Message-ID: Subject: Re: [PATCH RFC 5/6] mm: rmap: weaken the WARN_ON in __folio_add_anon_rmap() To: Chris Li Cc: David Hildenbrand , ryan.roberts@arm.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, mhocko@suse.com, shy828301@gmail.com, wangkefeng.wang@huawei.com, willy@infradead.org, xiang@kernel.org, ying.huang@intel.com, yuzhao@google.com, surenb@google.com, steven.price@arm.com, Barry Song , Chuanhua Han Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 710FF40006 X-Stat-Signature: 6kujmyk1ad349198uek5qgwtctwk9ai6 X-Rspam-User: X-HE-Tag: 1708923955-47209 X-HE-Meta: U2FsdGVkX1+moZ+sZuIU2WXpqbsSXpSNCuvfhExXtUmDnVP/+iuIElqrRVsRu9KKux+HDbN3Ykkv8e23v8JZTEI8UG1f/6uxt4Y99DREaHV7+iO4Ru+8RvWQYbZBKow/01h7P7Z4KK0DX55cX2AyiIx9BtD5xu7u04pJPkS2p7mDmp+T7/5W6U9T9+g+ufrtxZ5q7uaNQ9buStNTaBq+qIMQzPMPd1mY2oNDaE16W/nhO75R2pbYJ0lG+Ws6osSu8WyWUtwC0rG5Yuu2DnDGagNPym4cBwayEk2lnc+4jf/Er3rRAAJQuCV5m1hxvIuhMyjEUiOUQc47903d1RE+4ZmkueFjmDEYkBercSlvEEMjbfH7HuF8A920J70lETq+UQUoLg2Q4U1EtEW2jttbeVrRGrhF0w+cbYmBbTfNQOgTUuxnNING0TIexJOaSOdZ6oN32/3EzWlfuLMcU6dpoiUzs6wdxF2u6YlGQs+bFiMxGxy3+mOnT0pLmH4appP4R5XK6ltJlmFBcwn3XakNEnR1eq8KlNcfXGkxogQRBw4KqSgXiadIhyOzdoWa8sNZwRHNAX0R77L4yRH9x0wiip8jxFdb31/YfkPmuPaGVEb4tkiheHIHQkX87pp03WZfOynDpOYcbp36GMyczCD9Dr/3SmR2gSvKTPy0ernGxEUNXzFhqz9aCgioyq7TJxVaJ4JYVUOxrzwdor/0/Dc0Uy4WaXu8lOXKnCGQj9X91kB8F4KQiuE6YI/mjP6/kEoc/V5TTJhFWqyUrYEKbSa7bCVq+u6FeF9yiQa07UzSxfDLDg9XoHAE1cOmVbpCgOG7HJ6kqnsY3pul/sGiCQrEQIMmIry6aM/t7RGXoiVENLzo7TuuMNnzvh9hWVqeHBcw5SPiDzRHRuekMre6u1D4J9Jv9zz4muJw26aTOdgeB9ob4DMkTw9hdyleWLQF4rDVD2VpX4457zNZXPMk8GO +NjuR/U4 1VITHOvo3AR0hO2tjdBzG9WfItO5zLvAiEzPIb6iyaT67ekblo1QUgHil5uaOz1ZqzgzvdX3h2EO+VKgJEqWV5Q8EG6nM+/3mBs2jUp7cAbqEw5SFkcTl+JSgSA/Z6l2yFnlaiO4pHlONEqKhre8cETaQ8ArjxRyDhy/vEjk6OMhUyLVvf/lrfTGDl42ID6kwJcfVwx3a3oU2kPwBFZ8/VseNw3swuAh6wfA1kDYuCZmvGn604MLQZs6Vo0I0+A+Y9TEGjLKlZkIqzJV7MlcKjrwTH2MB1kPavteEOOR8hoRz5svj4mJE58KUoj6qD5GCfupe0EneANLMtCmOizRYRZia28w5ISy25/uqgLF907lr5F81MSi+S5mBjNkyrYcuj3wSpX1CQarXj6lIPUaN7tCrxafO1nUAdJJ5xWRp6LMmd4093r5dWrEAvhP01CozdtYjq4Hh2OjgvoS5zaQj4MQzxv3iyZzGt0SkPSX6P+jp3/oELUlO4hF/bjvg9AUsav5a 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 Tue, Jan 30, 2024 at 5:32=E2=80=AFAM Chris Li wrote: > > On Mon, Jan 29, 2024 at 2:07=E2=80=AFAM David Hildenbrand wrote: > > > > On 29.01.24 04:25, Chris Li wrote: > > > Hi David and Barry, > > > > > > On Mon, Jan 22, 2024 at 10:49=E2=80=AFPM Barry Song <21cnbao@gmail.co= m> wrote: > > >> > > >>> > > >>> > > >>> I have on my todo list to move all that !anon handling out of > > >>> folio_add_anon_rmap_ptes(), and instead make swapin code call add > > >>> folio_add_new_anon_rmap(), where we'll have to pass an exclusive fl= ag > > >>> then (-> whole new folio exclusive). > > >>> > > >>> That's the cleaner approach. > > >>> > > >> > > >> one tricky thing is that sometimes it is hard to know who is the fir= st > > >> one to add rmap and thus should > > >> call folio_add_new_anon_rmap. > > >> especially when we want to support swapin_readahead(), the one who > > >> allocated large filio might not > > >> be that one who firstly does rmap. > > > > > > I think Barry has a point. Two tasks might race to swap in the folio > > > then race to perform the rmap. > > > folio_add_new_anon_rmap() should only call a folio that is absolutely > > > "new", not shared. The sharing in swap cache disqualifies that > > > condition. > > > > We have to hold the folio lock. So only one task at a time might do the > > folio_add_anon_rmap_ptes() right now, and the > > folio_add_new_shared_anon_rmap() in the future [below]. > > > > Ah, I see. The folio_lock() is the answer I am looking for. > > > Also observe how folio_add_anon_rmap_ptes() states that one must hold > > the page lock, because otherwise this would all be completely racy. > > > > From the pte swp exclusive flags, we know for sure whether we are > > dealing with exclusive vs. shared. I think patch #6 does not properly > > check that all entries are actually the same in that regard (all > > exclusive vs all shared). That likely needs fixing. > > > > [I have converting per-page PageAnonExclusive flags to a single > > per-folio flag on my todo list. I suspect that we'll keep the > > per-swp-pte exlusive bits, but the question is rather what we can > > actually make work, because swap and migration just make it much more > > complicated. Anyhow, future work] > > > > > > > >> is it an acceptable way to do the below in do_swap_page? > > >> if (!folio_test_anon(folio)) > > >> folio_add_new_anon_rmap() > > >> else > > >> folio_add_anon_rmap_ptes() > > > > > > I am curious to know the answer as well. > > > > > > Yes, the end code should likely be something like: > > > > /* ksm created a completely new copy */ > > if (unlikely(folio !=3D swapcache && swapcache)) { > > folio_add_new_anon_rmap(folio, vma, vmf->address); > > folio_add_lru_vma(folio, vma); > > } else if (folio_test_anon(folio)) { > > folio_add_anon_rmap_ptes(rmap_flags) > > } else { > > folio_add_new_anon_rmap(rmap_flags) > > } > > > > Maybe we want to avoid teaching all existing folio_add_new_anon_rmap() > > callers about a new flag, and just have a new > > folio_add_new_shared_anon_rmap() instead. TBD. if we have to add a wrapper like folio_add_new_shared_anon_rmap() to avoid "if (folio_test_anon(folio))" and "else" everywhere, why not we just do it in folio_add_anon_rmap_ptes() ? folio_add_anon_rmap_ptes() { if (!folio_test_anon(folio)) return folio_add_new_anon_rmap(); } Anyway, I am going to change the patch 4/6 to if(folio_test_anon)/else firs= t and drop this 5/6. we may figure out if we need a wrapper later. > > There is more than one caller needed to perform that dance around > folio_test_anon() then decide which function to call. It would be nice > to have a wrapper function folio_add_new_shared_anon_rmap() to > abstract this behavior. > > > > > > > > > > BTW, that test might have a race as well. By the time the task got > > > !anon result, this result might get changed by another task. We need > > > to make sure in the caller context this race can't happen. Otherwise > > > we can't do the above safely. > > Again, folio lock. Observe the folio_lock_or_retry() call that covers > > our existing folio_add_new_anon_rmap/folio_add_anon_rmap_pte calls. > > Ack. Thanks for the explanation. > > Chris Thanks Barry