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 B3ADDC2BBCA for ; Tue, 25 Jun 2024 19:51:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2DA5D6B00A3; Tue, 25 Jun 2024 15:51:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 289AB6B00A6; Tue, 25 Jun 2024 15:51:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 12A9F6B00A7; Tue, 25 Jun 2024 15:51:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id E41256B00A6 for ; Tue, 25 Jun 2024 15:51:25 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 96D611A05ED for ; Tue, 25 Jun 2024 19:51:25 +0000 (UTC) X-FDA: 82270455330.17.B714EE1 Received: from mail-ot1-f43.google.com (mail-ot1-f43.google.com [209.85.210.43]) by imf25.hostedemail.com (Postfix) with ESMTP id BDE47A000E for ; Tue, 25 Jun 2024 19:51:23 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=AE9fkFXs; spf=pass (imf25.hostedemail.com: domain of hughd@google.com designates 209.85.210.43 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719345072; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=azMBw7gmWM9HSC5tNmEt9pv4LiSBiDJ9augQc8wkk9o=; b=491kfmPkSa1MzaYA8Isy86UpLlMvqniHi4PcbxDQiBkYurmntmXbPfaDHbrwRJjf54V4F6 j94EG8pCDimdf0Ljoj+c0ui7dLCm+jNcWHt8+rfxRJ/GmdSaYC0vkY3MtA2ha8ev/0w8+I c3qe01VE2O46FNFUwbZV0bMFU0TdbI4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=AE9fkFXs; spf=pass (imf25.hostedemail.com: domain of hughd@google.com designates 209.85.210.43 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719345072; a=rsa-sha256; cv=none; b=3UffhVv1FM8dY8Qn/xspEv7FQSyJahx4wCdVAu9CrwYKQrtDCsRRAWdfF7jvbZYuk/gI2H 6vlDC5V0fQZlASNt0xWKz77xH2aYsEiEsTUYw3DUuI2zRFQEDnNgWFZ3wmfkhHmjwNcTaF MMtvh8xWNxugwXq0bYtDHfyWk9F5MTg= Received: by mail-ot1-f43.google.com with SMTP id 46e09a7af769-700a6853664so2222388a34.1 for ; Tue, 25 Jun 2024 12:51:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1719345083; x=1719949883; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=azMBw7gmWM9HSC5tNmEt9pv4LiSBiDJ9augQc8wkk9o=; b=AE9fkFXsY/R5EK72sfbOnTY1Az2NpBZwnhRvbD9s6CUidq8on4Dek1XC6nvR5Fk5ky U+XzVxJxX7C8R9/MpHMvLWMwKZkKc3n+KvVavMUf0aYA51N8lTujNawySVDUcsvrk7rf E8MMmp1edjXdzO50+9oAXCwUBhnbpBFN2veMDDBRCHaqXg60/lZfXbEiAh/B4Om4+Pcl RX1qulGmenZvHP0hfveZmMW8b7+zYt/4TIdzFHSYKZpgiC92dHbCFkDEtntblPhcN96x MURAbQf0ihFYirKyUCUfP995VH3/QrOHkZbioqmYeD/LX0J/EVOdJk4mvLNm8blj412Z Ne/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719345083; x=1719949883; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=azMBw7gmWM9HSC5tNmEt9pv4LiSBiDJ9augQc8wkk9o=; b=d15yWfEjt0K5HwwF/smoH+3vpSagqq6AyQpFf91stSVdc18gH7l8LMwRuGnChDIokr Ya/aIk7BsIaG6EODvlaDaURvw6bIXd5dHyV+I2Ce7UNtOKJ/oILWQAngNhrSQHqFkFu1 Gt3CFOjCoRR3ClzUH+PhhP8Ag/p+pdC1ND9408p3gVbhKln2jMwwkc3VD3NTimYKa0k2 IJkk41RABqANIEGXWBzsAXkYID+sJIB+z8I2Bp2PD3GXc+fZJrmi2+2DNp5x+QC8hFuy HPRDP813M7SRBmyQvlADdQUMaaPNH3PrBurUEdR/MQ4LIQ74arl3g8lpvTN8xe2vPVk3 Jh4A== X-Forwarded-Encrypted: i=1; AJvYcCWlIMFiAeIfZ+wYDyY/Mml0Rs8HwPppVgr8CgU9Q9P8CTvJj6RY9yHz6DnJSbbKPOVcbwFbkSZhTzX1irxlT/8z5QU= X-Gm-Message-State: AOJu0Yxl/DJb4fZMHPNoPltZPOAuhBMckhx1mLZZYuGkX5loJYGlVwBi xAjWk6x7gLpDzWy4pijlCpSWUfnKtc0arY1konOKhQsoHHc41MfT8dUP/X3W3A== X-Google-Smtp-Source: AGHT+IFTMEIU9Nv/3bwizaKORLdLJCO505fbgRnjhZkkGTeli+KMSTnq3ULx1AVuyKH+Dfs0b8LSYQ== X-Received: by 2002:a05:6830:1002:b0:6f9:b69f:b64f with SMTP id 46e09a7af769-700afa4553dmr8574270a34.35.1719345082479; Tue, 25 Jun 2024 12:51:22 -0700 (PDT) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-700cda3e985sm106465a34.75.2024.06.25.12.51.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Jun 2024 12:51:21 -0700 (PDT) Date: Tue, 25 Jun 2024 12:51:09 -0700 (PDT) From: Hugh Dickins To: Kefeng Wang cc: Hugh Dickins , Andrew Morton , linux-mm@kvack.org, Tony Luck , Miaohe Lin , nao.horiguchi@gmail.com, Matthew Wilcox , David Hildenbrand , Muchun Song , Benjamin LaHaise , jglisse@redhat.com, Zi Yan , Jiaqi Yan , Vishal Moola , Alistair Popple , Jane Chu , Oscar Salvador Subject: Re: [PATCH mm-unstable] mm: migrate: folio_ref_freeze() under xas_lock_irq() In-Reply-To: Message-ID: <3346dbd7-b6f6-c191-b9b2-070f2ae60567@google.com> References: <07edaae7-ea5d-b6ae-3a10-f611946f9688@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Stat-Signature: 6keu6a4fy7mfzw8i36cib186qjn9opwc X-Rspam-User: X-Rspamd-Queue-Id: BDE47A000E X-Rspamd-Server: rspam02 X-HE-Tag: 1719345083-353865 X-HE-Meta: U2FsdGVkX18kf8m7LEbMr8jX7CjoHfJuOjgQGsVgHa/Us55TCcPfsgjZy+iidFBRIeC1Q9oaxc2o0LSjfFL/q9A29OaSOD3SVnuMkeur6ziow0q5HvG5fSTitAjH+r0qXavzLGbSo2Ei+Lq+LfYfDblHvZA1w+2NCCAHpyh0x3IOHMRmkt06RkTAQdIH5BSUz7eviBA0f8DwMp0OLiAagdL3pK3F8b9Z0WnBMkSHjAGnnALXI2XhvA4CpsvlzjkDZIXZmHhsdfekhkBc9Kn7ecugAO4b2Yeduor+J2/hzd0S6rmuZn6iit7iiWdKShybOKbRov0dJG2ftUixuQTTZs5DGvB6LktsKzFYP9DpHA6K9tHMTQYKzgdtuGtytQSpBMoOyIq/wQLOk6Jnh1S7XvLLr7FH+/z9CqHGGCOZPV4CeplbpxzvZJcAdkiu+oHfEwKv+hG/FBQJCTqeYkuhaGxpE0W+Yg+ROzwJlXt8/SmElfHG5OzoU+p0XvBBymDsp+ytahFRKDcl0p3SayesPBeWaZvbKhIBCkqtKXaFZx6Jf2nGyGk4m1hTDN8gPK70/T9gBbeW24pD7WKHDwJ91B4MUNFtnYF+1WXB81nz4Nn0/e9qwF7s0REyVxOJwK4TdjQ7mlwhvoo+iauqxufdCXoqfkaWydgRV1HqTFUpPOFiroiu7ytyhwz4Db47yo/yEAAAFSBTzNoIC0yBxY2Dnj2+a8s3c+e/LWnQW1882Y5LHyeDvz+Yn5ZIUNVp2a/Qwh6uqJyxFedtfVnLXF6q2YvYKjKqzaon73kPIfA2UdlBUUFvF7PHGeVkI/o6+lnlaz/fdAUeVRYsq6IR/fxwfrjrSBdXFUMr45bB8eHHyn/1EeKYs3k3HQXPTKP8K6zvl4qYN1nkJHdI1n+Apx9ewIaTQLx8sZkTpsiIuFZ8A8S3c5JI2VV+eQ9J/0dm8/uKLhvzA85drCi6ne8LwAv qq+V5vVz hltYpY8YOYJ/rY6h7ziP/AJyq/6adrME0tjHvB9JaqkzANU8oce7uL7IHa4xwzsFksQlDAlLIduJARlPx3XlvZCB60TytiN3FW0a9f/ZCOVWzrcOLZMHmyQWlQH9JThytggZgoedwD4O0tdV9Loe6GeynyiaTbT8AMr6Sd8J0k+y3tSbKJM3pxvuwoEDpW9ErJkGPF9DhewV/9LQwmPgO0Yfw/rQq8ygfGUjc2m7msxHSWtQh0KsNu/xoH3Haf67nKQHsD56JfNVebC4T5xP5dS1mysDLfwnkiRBWMkOlHuRw2mfqEMTwla1S4yR5Jt5FoUvkO338pF0vlPCIfTcjFnG5mJg1Q0dXE81W2niTXy6W9OoxWV/HCWAZ3+RF7lpw0SuLKS286JiypSdyvFKghPdWqrXyjKMDnuxdrSXbikq2Nn8+GsqeBK18Y31XlHMYHPe2sEcvvCzMPu88fPZDtKS+6b0SThyo9gkfAE21jlc7av0k/zaoK+t35eSPVKWHZ+fkmSesciAckbk4BUHANV/yrg== 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, 25 Jun 2024, Kefeng Wang wrote: > On 2024/6/25 13:04, Hugh Dickins wrote: > > Commit "mm: migrate: split folio_migrate_mapping()" drew attention to > > "Note, the folio_ref_freeze() is moved out of xas_lock_irq(), Since the > > folio is already isolated and locked during migration, so suppose that > > there is no functional change." > > > > That was a mistake. Freezing a folio's refcount to 0 is much like taking > > a spinlock: see how filemap_get_entry() takes rcu_read_lock() then spins > > around until the folio is unfrozen. If the task freezing is preempted (or > > calls cond_resched(), as folio_mc_copy() may do), then it risks deadlock: > > in my case, one CPU in zap_pte_range() with free_swap_and_cache_nr() > > trying to reclaim swap while PTL is held, all the other CPUs in reclaim > > spinning for that PTL. > > Oh, thanks for pointing this out, I didn't take that into account. > > > > I'm uncertain whether it's necessary for interrupts to be disabled as > > well as preemption, but since they have to be disabled for the page > > cache migration, it's much the best to do it all together as before. > > So revert to folio_ref_freeze() under xas_lock_irq(): but keep the > > preliminary folio_ref_count() check, which does make sense before > > trying to copy the folio's data. > > This is what my RFC version[1] does, which adds same reference check to > avoid the unnecessary folio_mc_copy(). > > Should I resend all patches, or Andrew directly pick this one? Andrew asks for a resend: I was only aiming to fix the bug, and have no perspective on how much of the series remains worthwhile. > > > Thanks. > > [1] > https://lore.kernel.org/linux-mm/20240129070934.3717659-7-wangkefeng.wang@huawei.com/ > > > > > > Use "expected_count" for the expected count throughout. > > > > Signed-off-by: Hugh Dickins > > --- > > mm/migrate.c | 59 +++++++++++++++++++++++++--------------------------- > > 1 file changed, 28 insertions(+), 31 deletions(-) > > > > diff --git a/mm/migrate.c b/mm/migrate.c > > index 27f070f64f27..8beedbb42a93 100644 > > --- a/mm/migrate.c > > +++ b/mm/migrate.c > > @@ -400,8 +400,8 @@ static int folio_expected_refs(struct address_space > > *mapping, > > * 2 for folios with a mapping > > * 3 for folios with a mapping and PagePrivate/PagePrivate2 set. > > */ > > -static void __folio_migrate_mapping(struct address_space *mapping, > > - struct folio *newfolio, struct folio *folio, int expected_cnt) > > +static int __folio_migrate_mapping(struct address_space *mapping, > > + struct folio *newfolio, struct folio *folio, int > > expected_count) > > We can rename it back to folio_migrate_mapping(). Almost. I did want to remove the __ layer, but internally the last parameter is expected_count, whereas externally it is extra_count: each has merit in its place, so I left them as is. Hugh