From: Luis Chamberlain <mcgrof@kernel.org>
To: Davidlohr Bueso <dave@stgolabs.net>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: jack@suse.cz, tytso@mit.edu, adilger.kernel@dilger.ca,
brauner@kernel.org, willy@infradead.org, hare@suse.de,
djwong@kernel.org, linux-ext4@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH -next 0/7] fs/buffer: split pagecache lookups into atomic or blocking
Date: Wed, 16 Apr 2025 12:27:57 -0700 [thread overview]
Message-ID: <aAAEvcrmREWa1SKF@bombadil.infradead.org> (raw)
In-Reply-To: <20250415231635.83960-1-dave@stgolabs.net>
On Tue, Apr 15, 2025 at 04:16:28PM -0700, Davidlohr Bueso wrote:
> Hello,
>
> This is a respin of the series[0] to address the sleep in atomic scenarios for
> noref migration with large folios, introduced in:
>
> 3c20917120ce61 ("block/bdev: enable large folio support for large logical block sizes")
>
> The main difference is that it removes the first patch and moves the fix (reducing
> the i_private_lock critical region in the migration path) to the final patch, which
> also introduces the new BH_Migrate flag. It also simplifies the locking scheme in
> patch 1 to avoid folio trylocking in the atomic lookup cases. So essentially blocking
> users will take the folio lock and hence wait for migration, and otherwise nonblocking
> callers will bail the lookup if a noref migration is on-going. Blocking callers
> will also benefit from potential performance gains by reducing contention on the
> spinlock for bdev mappings.
>
> It is noteworthy that this series is probably too big for Linus' tree, so there are
> two options:
>
> 1. Revert 3c20917120ce61, add this series + 3c20917120ce61 for next. Or,
Reverting due to a fix series is odd, I'd advocate this series as a set
of fixes to Linus' tree because clearly folio migration was not complete
for buffer_migrate_folio_norefs() and this is part of the loose bits to help
it for large folios. This issue was just hard to reproduce. The enabler
of large folios on the block device cache is actually commit
47dd67532303 ("block/bdev: lift block size restrictions to 64k") which
goes later after 3c20917120ce61.
Jan Kara, since you've already added your Reviewed-by for all patches
do you have any preference how this trickles to Linus?
> 2. Cherry pick patch 7 as a fix for Linus' tree, and leave the rest for next.
> But that could break lookup callers that have been deemed unfit to bail.
>
> Patch 1: carves a path for callers that can block to take the folio lock.
> Patch 2: adds sleeping flavors to pagecache lookups, no users.
> Patches 3-6: converts to the new call, where possible.
> Patch 7: does the actual sleep in atomic fix.
>
> Thanks!
kdevops has tested this patch series and compared it to the baseline [0]
and has found no regressions on ext4.
Tested-by: kdevops@lists.linux.dev
Detailed test results below:
Comparing commits:
Baseline: a74831cc4300 | linux-ext4-kpd: Linux 6.15-rc2
Test: 6b337686249b | 6.15-rc2 + these patches
Baseline Kernel:6.15.0-rc2-g8ffd015db85f
Test Kernel: 6.15.0-rc2-00006-g89e084d709fc
Verbose Test Results Comparison:
================================================================================
Profile: ext4_1k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_2k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_advanced_features
| BASELINE | TEST
-------------------|--------------|--------------
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/270 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/477 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_bigalloc1024k_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/033 | [fail] | [fail]
ext4/034 | [fail] | [fail]
ext4/045 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/075 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/091 | [fail] | [fail]
generic/112 | [fail] | [fail]
generic/127 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/234 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/251 | [fail] | [fail]
generic/263 | [fail] | [fail]
generic/280 | [fail] | [fail]
generic/365 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/435 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/614 | [fail] | [fail]
generic/629 | [fail] | [fail]
generic/634 | [fail] | [fail]
generic/635 | [fail] | [fail]
generic/643 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/698 | [fail] | [fail]
generic/732 | [fail] | [fail]
generic/738 | [fail] | [fail]
generic/741 | [fail] | [fail]
generic/754 | [fail] | [fail]
Profile: ext4_bigalloc16k_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/033 | [fail] | [fail]
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/075 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/091 | [fail] | [fail]
generic/112 | [fail] | [fail]
generic/127 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/234 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/263 | [fail] | [fail]
generic/280 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_bigalloc2048k_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/033 | [fail] | [fail]
ext4/034 | [fail] | [fail]
ext4/045 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/075 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/091 | [fail] | [fail]
generic/112 | [fail] | [fail]
generic/127 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/234 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/251 | [fail] | [fail]
generic/263 | [fail] | [fail]
generic/280 | [fail] | [fail]
generic/365 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/435 | [fail] | [fail]
generic/471 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/614 | [fail] | [fail]
generic/629 | [fail] | [fail]
generic/634 | [fail] | [fail]
generic/635 | [fail] | [fail]
generic/643 | [fail] | [fail]
generic/645 | [fail] | [fail]
generic/676 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/698 | [fail] | [fail]
generic/732 | [fail] | [fail]
generic/736 | [fail] | [fail]
generic/738 | [fail] | [fail]
generic/741 | [fail] | [fail]
generic/754 | [fail] | [fail]
Profile: ext4_bigalloc32k_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/033 | [fail] | [fail]
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/075 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/091 | [fail] | [fail]
generic/112 | [fail] | [fail]
generic/127 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/234 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/263 | [fail] | [fail]
generic/280 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_bigalloc64k_4k
| BASELINE | TEST
-------------------|--------------|--------------
ext4/033 | [fail] | [fail]
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/075 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/091 | [fail] | [fail]
generic/112 | [fail] | [fail]
generic/127 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/234 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/263 | [fail] | [fail]
generic/280 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Profile: ext4_defaults
| BASELINE | TEST
-------------------|--------------|--------------
ext4/034 | [fail] | [fail]
ext4/055 | [fail] | [fail]
generic/082 | [fail] | [fail]
generic/219 | [fail] | [fail]
generic/223 | [fail] | [fail]
generic/230 | [fail] | [fail]
generic/231 | [fail] | [fail]
generic/232 | [fail] | [fail]
generic/233 | [fail] | [fail]
generic/235 | [fail] | [fail]
generic/270 | [fail] | [fail]
generic/381 | [fail] | [fail]
generic/382 | [fail] | [fail]
generic/566 | [fail] | [fail]
generic/587 | [fail] | [fail]
generic/600 | [fail] | [fail]
generic/601 | [fail] | [fail]
generic/681 | [fail] | [fail]
generic/682 | [fail] | [fail]
generic/741 | [fail] | [fail]
Summary:
- Total regressions: 0
- Total fixes: 0
- Unchanged failures: 261
[0] https://lore.kernel.org/all/Z__vQcCF9xovbwtT@bombadil.infradead.org/
Luis
next prev parent reply other threads:[~2025-04-16 19:28 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-15 23:16 Davidlohr Bueso
2025-04-15 23:16 ` [PATCH 1/7] fs/buffer: split locking for pagecache lookups Davidlohr Bueso
2025-04-16 9:32 ` Jan Kara
2025-04-15 23:16 ` [PATCH 2/7] fs/buffer: introduce sleeping flavors " Davidlohr Bueso
2025-04-16 9:33 ` Jan Kara
2025-04-15 23:16 ` [PATCH 3/7] fs/buffer: use sleeping version of __find_get_block() Davidlohr Bueso
2025-04-16 9:33 ` Jan Kara
2025-04-15 23:16 ` [PATCH 4/7] fs/ocfs2: " Davidlohr Bueso
2025-04-16 9:35 ` Jan Kara
2025-04-15 23:16 ` [PATCH 5/7] fs/jbd2: " Davidlohr Bueso
2025-04-16 9:38 ` Jan Kara
2025-04-15 23:16 ` [PATCH 6/7] fs/ext4: use sleeping version of sb_find_get_block() Davidlohr Bueso
2025-04-16 9:39 ` Jan Kara
2025-04-15 23:16 ` [PATCH 7/7] mm/migrate: fix sleep in atomic for large folios and buffer heads Davidlohr Bueso
2025-04-16 9:43 ` Jan Kara
2025-04-16 19:27 ` Luis Chamberlain [this message]
2025-04-17 9:57 ` [PATCH -next 0/7] fs/buffer: split pagecache lookups into atomic or blocking Christian Brauner
2025-04-22 11:25 ` Jan Kara
2025-04-17 9:58 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aAAEvcrmREWa1SKF@bombadil.infradead.org \
--to=mcgrof@kernel.org \
--cc=adilger.kernel@dilger.ca \
--cc=brauner@kernel.org \
--cc=dave@stgolabs.net \
--cc=djwong@kernel.org \
--cc=hare@suse.de \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=torvalds@linux-foundation.org \
--cc=tytso@mit.edu \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox