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 4536AC369BA for ; Tue, 15 Apr 2025 15:47:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E66B528005F; Tue, 15 Apr 2025 11:47:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DEDF9280024; Tue, 15 Apr 2025 11:47:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C401E28005F; Tue, 15 Apr 2025 11:47:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 9F1DE280024 for ; Tue, 15 Apr 2025 11:47:55 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 234621406E8 for ; Tue, 15 Apr 2025 15:47:56 +0000 (UTC) X-FDA: 83336708952.01.B07ED88 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf10.hostedemail.com (Postfix) with ESMTP id 8B389C0006 for ; Tue, 15 Apr 2025 15:47:54 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Dr932TZV; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf10.hostedemail.com: domain of mcgrof@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=mcgrof@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744732074; a=rsa-sha256; cv=none; b=LYJY7QCgoMkvNWCajfqG57tsm2r2jFA8GYcQi9cWwolpemwuYkKCX2moQ4kNRh3DEFEIZw DT4C/Tf98uSiucZTrCT34DTVR1CrTH9+Xzg/QDd0lENTgeqOdUbkAjOh7RjAMajjysVqEH N4enp6/aqjeF7hI4g/N4jBgHN76AIYI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744732074; 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=4w0SJjq1aWEm49mU3APH9zxHo35t6Bp1pA+U25vWzyc=; b=6RQlfj5dTVmuumwFD3YkM9E/tK9VwX9OIradRjbOAO+J7ipHMGlAfBYzUdz3jXh0KN3QbW mV0BiksFLly4hHTvTZ8FvJb58WZnlhzwDD5LWp0Px1Q2BtxmYPNJYlNthyGKvFv0ORILc4 krU7UY9b5bBSYNVQ0fjNVyo9ZbrUujk= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Dr932TZV; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf10.hostedemail.com: domain of mcgrof@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=mcgrof@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 50A00A4A433; Tue, 15 Apr 2025 15:42:25 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C0C55C4CEEB; Tue, 15 Apr 2025 15:47:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744732073; bh=kxmFcQDJ+JlN9IfPV3DM8E3EagG+moUhDz8DUHXPMkk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Dr932TZVkc9yCxpr4kJ6POc0QEMnT2JGyjYw3eFy0cjgaTuIqDQPyqjKLPss6VFI4 Qa7qjCYibaB6mt+sdNawo5jePudUTkq7svKOPdJyX32IzxSBhoRDGwr/Wi1u2oLDNy bq1WXDPexYb+N3+LjqtasY5ueOOF09THvoNggnff0CbdQO2FPapLtUMpLbucBLc7E9 KNEDfXq38XRaHDhaQkQyB0fBxSWAqlbZCtQGQop8ZH3x3CWLYO0TNPqv4O1Bj0JmbW edj3LSONuxjLsv3yTJmjLvV7ih63uD443nMoICh9l4GWybMRggMgRjkEUmVgMuScJc 9er4nsuafOrLA== Date: Tue, 15 Apr 2025 08:47:51 -0700 From: Luis Chamberlain To: Christian Brauner Cc: Jan Kara , tytso@mit.edu, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org, riel@surriel.com, dave@stgolabs.net, willy@infradead.org, hannes@cmpxchg.org, oliver.sang@intel.com, david@redhat.com, axboe@kernel.dk, hare@suse.de, david@fromorbit.com, djwong@kernel.org, ritesh.list@gmail.com, linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org, linux-mm@kvack.org, gost.dev@samsung.com, p.raghav@samsung.com, da.gomez@samsung.com, syzbot+f3c6fda1297c748a7076@syzkaller.appspotmail.com Subject: Re: [PATCH v2 1/8] migrate: fix skipping metadata buffer heads on migration Message-ID: References: <20250410014945.2140781-1-mcgrof@kernel.org> <20250410014945.2140781-2-mcgrof@kernel.org> <20250415-freihalten-tausend-a9791b9c3a03@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250415-freihalten-tausend-a9791b9c3a03@brauner> X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 8B389C0006 X-Stat-Signature: 8s1phfjni8aid8w5ys7cnhbfwjeieqfg X-Rspam-User: X-HE-Tag: 1744732074-186740 X-HE-Meta: U2FsdGVkX19Rbf6vch1gmtMpU6lqMka9qqf5FgoK2kYnz6RTBNgnKeVGqNXQsaNw1IKUBR295WcuY6G2nwTbLlMiNiNbsoddW4JuRZYh1trqit0UEvvPUjucV1dbb7Ya92Bpix1nUf1NVUctt0Ia/B8kqziETgq5LIhy7ctKdfwQpOzY6xdAYSItKpNBpu31FUo8rLm/aF7Ji5lX/NvC8+JRy8huO3L3o5JkTS+JWtWUjoKHR5JJIR3Njfx87utW9T1DBJ7RUW3sC8MIGb9rpuh/XJW689Ek182UA4Eo/2qtu3py4VfwumOBt2IfdQmTXpeNCgeg1uzYvejRiE3p5foUBGSWF4kPArhM2lhtyfIy4QotQC+Bb51C5oO8hPrwP9E45ZgZzl2lH+sgfWcKVsQ8r98aqhvsA1qKs+a+5c0h6+qc+HLYNh+vxr5hp75nG0BUx9mbCsuMX+0rEv/QYud1QYnwqi1n30FJZlhBSgq/YR0LB5BmEPR5XQfRfjlpHiZMWTFWHtPQztaqKi7SdA/AljDb8rbC5xtQzxnCP6GNEhHRj8nBjc7sR8zXSTdx78dmkS84O6whE3JLj/fFb7khrtrjgC9DFrI7f6OyReEc+/saZpunJNifYLKqkPGuFyEuA1ArYL+XC9ZvFxmrxkjzfyRy8TXCHU1SPV8zs9uDDv81Q8/k45SAgqp2HmJmg75Q4+IxfyoZErwhTlpnxoswrXAQRAHcaoCQk45gc5mTI/wtkJFMgYS9J2a/pMpjJK1LrPXKPUcrBdAfCKWOfV9vZWo0Oaj4+V7gvgN332qNZHXE6TiTEr7V1VCSXITm/xxCD/tUE5yaoCCjQVO3VcgYzyTj4/vd54zRWt8rOLAyXvXhkSEV2m/+T4jR6Ro3+kQoOiGc6b0rB1rYS3LkTb/9QywDUOYoVbyH+AoxjQpY0drVzNkyBu9+amD0BLAW6O7N9FmG2504CpqrrmZ pNhnBZ24 BDsK8m6yTajv+2M0/17hzHjEJwqrAIC1Qh+vlE1uZOhCOamjjVnjcHLAZFNQ7HJ14wKe4a51ik4/nfV5gPDYuknizE2V6oz0aEpoJkoUa9hh+avvwTyjbbNkLUz2PBmio/zUPvAa9XlMjyCWUxx3r4ikPoZWLW48sZwr/qE9pwe3hsuIlu9CpPAhqm00dYoz2mDiCOfA81pt0j9FN6reb5rnQ7Ozqq4w2PhJCi9ZBY7LFTUmK8omdfULQIwYjOYvWIeDaIw99EdmbiDsh/nk7lF6mH+M8GR4VTIFhHGBW+98lYwotsdxt8k7Liw== 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, Apr 15, 2025 at 11:05:38AM +0200, Christian Brauner wrote: > On Mon, Apr 14, 2025 at 03:19:33PM -0700, Luis Chamberlain wrote: > > On Mon, Apr 14, 2025 at 02:09:46PM -0700, Luis Chamberlain wrote: > > > On Thu, Apr 10, 2025 at 02:05:38PM +0200, Jan Kara wrote: > > > > > @@ -859,12 +862,12 @@ static int __buffer_migrate_folio(struct address_space *mapping, > > > > > } > > > > > bh = bh->b_this_page; > > > > > } while (bh != head); > > > > > + spin_unlock(&mapping->i_private_lock); > > > > > > > > No, you've just broken all simple filesystems (like ext2) with this patch. > > > > You can reduce the spinlock critical section only after providing > > > > alternative way to protect them from migration. So this should probably > > > > happen at the end of the series. > > > > > > So you're OK with this spin lock move with the other series in place? > > > > > > And so we punt the hard-to-reproduce corruption issue as future work > > > to do? Becuase the other alternative for now is to just disable > > > migration for jbd2: > > > > > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > > > index 1dc09ed5d403..ef1c3ef68877 100644 > > > --- a/fs/ext4/inode.c > > > +++ b/fs/ext4/inode.c > > > @@ -3631,7 +3631,6 @@ static const struct address_space_operations ext4_journalled_aops = { > > > .bmap = ext4_bmap, > > > .invalidate_folio = ext4_journalled_invalidate_folio, > > > .release_folio = ext4_release_folio, > > > - .migrate_folio = buffer_migrate_folio_norefs, > > > .is_partially_uptodate = block_is_partially_uptodate, > > > .error_remove_folio = generic_error_remove_folio, > > > .swap_activate = ext4_iomap_swap_activate, > > > > BTW I ask because.. are your expectations that the next v3 series also > > be a target for Linus tree as part of a fix for this spinlock > > replacement? > > Since this is fixing potential filesystem corruption I will upstream > whatever we need to do to fix this. Ideally we have a minimal fix to > upstream now and a comprehensive fix and cleanup for v6.16. Despite our efforts we don't yet have an agreement on how to fix the ext4 corruption, becuase Jan noted the buffer_meta() check in this patch is too broad and would affect other filesystems (I have yet to understand how, but will review). And so while we have agreement we can remove the spin lock to fix the sleeping while atomic incurred by large folios for buffer heads by this patch series, the removal of the spin lock would happen at the end of this series. And so the ext4 corruption is an existing issue as-is today, its separate from the spin lock removal goal to fix the sleeping while atomic.. However this series might be quite big for an rc2 or rc3 fix for that spin lock removal issue. It should bring in substantial performance benefits though, so it might be worthy to consider. We can re-run tests with the adjustment to remove the spin lock until the last patch in this series. The alternative is to revert the spin lock addition commit for Linus' tree, ie commit ebdf4de5642fb6 ("mm: migrate: fix reference check race between __find_get_block() and migration") and note that it in fact does not fix the ext4 corruption as we've noted, and in fact causes an issue with sleeping while atomic with support for large folios for buffer heads. If we do that then we punt this series for the next development window, and it would just not have the spin lock removal on the last patch. Jan Kara, Christian, thoughts? Luis