On 7/11/23 07:01, Christoph Hellwig wrote:
I think that's because it doesn't look at sbi->s_ndevs in
destroy_device_list. Let's try the variant below, which also fixes
the buildbot warning for non-zoned configfs:
---
From 645d8dceaa97b6ee73be067495b111b15b187498 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@xxxxxx>
Date: Fri, 7 Jul 2023 10:31:49 +0200
Subject: f2fs: don't reopen the main block device in f2fs_scan_devices
f2fs_scan_devices reopens the main device since the very beginning, which
has always been useless, and also means that we don't pass the right
holder for the reopen, which now leads to a warning as the core super.c
holder ops aren't passed in for the reopen.
Fixes: 3c62be17d4f5 ("f2fs: support multiple devices")
Fixes: 0718afd47f70 ("block: introduce holder ops")
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
block/blk-flush.c | 2 +-
fs/f2fs/super.c | 20 ++++++++------------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ca31163da00a55..30883beb750a59 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1561,7 +1561,8 @@ static void destroy_device_list(struct f2fs_sb_info *sbi)
int i;
for (i = 0; i < sbi->s_ndevs; i++) {
- blkdev_put(FDEV(i).bdev, sbi->sb->s_type);
+ if (i > 0)
+ blkdev_put(FDEV(i).bdev, sbi->sb->s_type);
You could have started the loop at '1', and avoid the curious 'if'
clause ...
#ifdef CONFIG_BLK_DEV_ZONED
kvfree(FDEV(i).blkz_seq);
#endif
@@ -4190,16 +4191,12 @@ static int f2fs_scan_devices(struct f2fs_sb_info *sbi)
sbi->aligned_blksize = true;
for (i = 0; i < max_devices; i++) {
-
- if (i > 0 && !RDEV(i).path[0])
+ if (i == 0)
+ FDEV(0).bdev = sbi->sb->s_bdev;
+ else if (!RDEV(i).path[0])
break;
- if (max_devices == 1) {
- /* Single zoned block device mount */
- FDEV(0).bdev =
- blkdev_get_by_dev(sbi->sb->s_bdev->bd_dev, mode,
- sbi->sb->s_type, NULL);
- } else {
+ if (max_devices > 1) {
/* Multi-device mount */
memcpy(FDEV(i).path, RDEV(i).path, MAX_PATH_LEN);
FDEV(i).total_segments =
Similar here; wouldn't it be better to unroll the loop, and have the
check for 'max_devices' outside of the loop?
Then the loop can be coded for the 'max_device > 1' case only, and avoid
all the special casing in the loop ...
@@ -4215,10 +4212,9 @@ static int f2fs_scan_devices(struct f2fs_sb_info *sbi)
FDEV(i).end_blk = FDEV(i).start_blk +
(FDEV(i).total_segments <<
sbi->log_blocks_per_seg) - 1;
+ FDEV(i).bdev = blkdev_get_by_path(FDEV(i).path,
+ mode, sbi->sb->s_type, NULL);
}
- FDEV(i).bdev = blkdev_get_by_path(FDEV(i).path, mode,
- sbi->sb->s_type,
- NULL);
}
if (IS_ERR(FDEV(i).bdev))
return PTR_ERR(FDEV(i).bdev);
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
