2024-06-06 09:51:13

by Chao Yu

[permalink] [raw]
Subject: [PATCH] f2fs: get rid of buffer_head use

For later folio conversion.

Cc: Matthew Wilcox <[email protected]>
Signed-off-by: Chao Yu <[email protected]>
---
v1:
- I checked recovery flow of superblock w/ below testcase, it passes.
1. mkfs.f2fs /dev/vdd
2. dd if=/dev/zero of=/dev/vdd bs=4k count=1
3. mount /dev/vdd /mnt/f2fs
4. umount /mnt/f2fs
5. dd if=/dev/zero of=/dev/vdd bs=4k count=1 seek=1
6. mount /dev/vdd /mnt/f2fs
7. umount /mnt/f2fs

fs/f2fs/data.c | 1 -
fs/f2fs/f2fs.h | 1 -
fs/f2fs/file.c | 1 -
fs/f2fs/inode.c | 1 -
fs/f2fs/super.c | 95 ++++++++++++++++++++++++++++++-------------------
5 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b9b0debc6b3d..ad495ea87b32 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -7,7 +7,6 @@
*/
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
-#include <linux/buffer_head.h>
#include <linux/sched/mm.h>
#include <linux/mpage.h>
#include <linux/writeback.h>
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9688df332147..5ed6b689f6b3 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -11,7 +11,6 @@
#include <linux/uio.h>
#include <linux/types.h>
#include <linux/page-flags.h>
-#include <linux/buffer_head.h>
#include <linux/slab.h>
#include <linux/crc32.h>
#include <linux/magic.h>
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c50213da474d..efc676bc7800 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -8,7 +8,6 @@
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
#include <linux/stat.h>
-#include <linux/buffer_head.h>
#include <linux/writeback.h>
#include <linux/blkdev.h>
#include <linux/falloc.h>
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index dbfebbddf675..87982e06bbe7 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -7,7 +7,6 @@
*/
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
-#include <linux/buffer_head.h>
#include <linux/writeback.h>
#include <linux/sched/mm.h>
#include <linux/lz4.h>
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4a1bc8f40f9a..e67f0ac78c00 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -11,7 +11,6 @@
#include <linux/fs_context.h>
#include <linux/sched/mm.h>
#include <linux/statfs.h>
-#include <linux/buffer_head.h>
#include <linux/kthread.h>
#include <linux/parser.h>
#include <linux/mount.h>
@@ -3333,24 +3332,45 @@ loff_t max_file_blocks(struct inode *inode)
return result;
}

-static int __f2fs_commit_super(struct buffer_head *bh,
- struct f2fs_super_block *super)
+static int __f2fs_commit_super(struct f2fs_sb_info *sbi, struct page *page,
+ bool update)
{
- lock_buffer(bh);
- if (super)
- memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
- set_buffer_dirty(bh);
- unlock_buffer(bh);
-
+ struct f2fs_super_block *super;
+ struct bio *bio;
/* it's rare case, we can do fua all the time */
- return __sync_dirty_buffer(bh, REQ_SYNC | REQ_PREFLUSH | REQ_FUA);
+ blk_opf_t opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA;
+ int ret;
+
+ lock_page(page);
+ if (update) {
+ super = F2FS_RAW_SUPER(sbi);
+ memcpy(page_address(page) + F2FS_SUPER_OFFSET,
+ super, sizeof(*super));
+ }
+ set_page_dirty(page);
+ clear_page_dirty_for_io(page);
+ set_page_writeback(page);
+ unlock_page(page);
+
+ bio = bio_alloc(sbi->sb->s_bdev, 1, opf, GFP_NOFS);
+
+ /* it doesn't need to set crypto context for superblock update */
+ bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(page->index);
+
+ if (bio_add_page(bio, page, PAGE_SIZE, 0) != PAGE_SIZE)
+ f2fs_bug_on(sbi, 1);
+
+ ret = submit_bio_wait(bio);
+ end_page_writeback(page);
+
+ return ret;
}

static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
- struct buffer_head *bh)
+ struct page *page)
{
struct f2fs_super_block *raw_super = (struct f2fs_super_block *)
- (bh->b_data + F2FS_SUPER_OFFSET);
+ (page_address(page) + F2FS_SUPER_OFFSET);
struct super_block *sb = sbi->sb;
u32 segment0_blkaddr = le32_to_cpu(raw_super->segment0_blkaddr);
u32 cp_blkaddr = le32_to_cpu(raw_super->cp_blkaddr);
@@ -3425,7 +3445,7 @@ static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
set_sbi_flag(sbi, SBI_NEED_SB_WRITE);
res = "internally";
} else {
- err = __f2fs_commit_super(bh, NULL);
+ err = __f2fs_commit_super(sbi, page, false);
res = err ? "failed" : "done";
}
f2fs_info(sbi, "Fix alignment : %s, start(%u) end(%llu) block(%u)",
@@ -3437,13 +3457,12 @@ static inline bool sanity_check_area_boundary(struct f2fs_sb_info *sbi,
return false;
}

-static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
- struct buffer_head *bh)
+static int sanity_check_raw_super(struct f2fs_sb_info *sbi, struct page *page)
{
block_t segment_count, segs_per_sec, secs_per_zone, segment_count_main;
block_t total_sections, blocks_per_seg;
struct f2fs_super_block *raw_super = (struct f2fs_super_block *)
- (bh->b_data + F2FS_SUPER_OFFSET);
+ (page_address(page) + F2FS_SUPER_OFFSET);
size_t crc_offset = 0;
__u32 crc = 0;

@@ -3601,7 +3620,7 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
}

/* check CP/SIT/NAT/SSA/MAIN_AREA area boundary */
- if (sanity_check_area_boundary(sbi, bh))
+ if (sanity_check_area_boundary(sbi, page))
return -EFSCORRUPTED;

return 0;
@@ -3948,7 +3967,7 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
{
struct super_block *sb = sbi->sb;
int block;
- struct buffer_head *bh;
+ struct page *page;
struct f2fs_super_block *super;
int err = 0;

@@ -3957,32 +3976,32 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
return -ENOMEM;

for (block = 0; block < 2; block++) {
- bh = sb_bread(sb, block);
- if (!bh) {
+ page = read_mapping_page(sb->s_bdev->bd_inode->i_mapping,
+ block, NULL);
+ if (IS_ERR(page)) {
f2fs_err(sbi, "Unable to read %dth superblock",
block + 1);
- err = -EIO;
+ err = PTR_ERR(page);
*recovery = 1;
- continue;
}

/* sanity checking of raw super */
- err = sanity_check_raw_super(sbi, bh);
+ err = sanity_check_raw_super(sbi, page);
if (err) {
f2fs_err(sbi, "Can't find valid F2FS filesystem in %dth superblock",
block + 1);
- brelse(bh);
+ put_page(page);
*recovery = 1;
continue;
}

if (!*raw_super) {
- memcpy(super, bh->b_data + F2FS_SUPER_OFFSET,
+ memcpy(super, page_address(page) + F2FS_SUPER_OFFSET,
sizeof(*super));
*valid_super_block = block;
*raw_super = super;
}
- brelse(bh);
+ put_page(page);
}

/* No valid superblock */
@@ -3996,7 +4015,7 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,

int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
{
- struct buffer_head *bh;
+ struct page *page;
__u32 crc = 0;
int err;

@@ -4014,22 +4033,24 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
}

/* write back-up superblock first */
- bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
- if (!bh)
- return -EIO;
- err = __f2fs_commit_super(bh, F2FS_RAW_SUPER(sbi));
- brelse(bh);
+ page = read_mapping_page(sbi->sb->s_bdev->bd_inode->i_mapping,
+ sbi->valid_super_block ? 0 : 1, NULL);
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+ err = __f2fs_commit_super(sbi, page, true);
+ put_page(page);

/* if we are in recovery path, skip writing valid superblock */
if (recover || err)
return err;

/* write current valid superblock */
- bh = sb_bread(sbi->sb, sbi->valid_super_block);
- if (!bh)
- return -EIO;
- err = __f2fs_commit_super(bh, F2FS_RAW_SUPER(sbi));
- brelse(bh);
+ page = read_mapping_page(sbi->sb->s_bdev->bd_inode->i_mapping,
+ sbi->valid_super_block, NULL);
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+ err = __f2fs_commit_super(sbi, page, true);
+ put_page(page);
return err;
}

--
2.40.1



2024-06-06 12:38:27

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] f2fs: get rid of buffer_head use

On Thu, Jun 06, 2024 at 05:50:37PM +0800, Chao Yu wrote:
> For later folio conversion.

What tree is this against?

> @@ -3957,32 +3976,32 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
> return -ENOMEM;
>
> for (block = 0; block < 2; block++) {
> - bh = sb_bread(sb, block);
> - if (!bh) {
> + page = read_mapping_page(sb->s_bdev->bd_inode->i_mapping,
> + block, NULL);

You need to use bd_mapping, not bd_inode->i_mapping (since May 21 in
Linus' tree).

And I don't think there's much point in switching to pages as an
intermediate step. You may as well go straight to folios.

folio = read_mapping_folio(sb->s_bdev->bd_mapping, block, NULL);


2024-06-06 13:22:23

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH] f2fs: get rid of buffer_head use

On 2024/6/6 20:38, Matthew Wilcox wrote:
> On Thu, Jun 06, 2024 at 05:50:37PM +0800, Chao Yu wrote:
>> For later folio conversion.
>
> What tree is this against?

Last dev-test branch of f2fs git tree, I guess it's a little behind
to linus' tree.

>
>> @@ -3957,32 +3976,32 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
>> return -ENOMEM;
>>
>> for (block = 0; block < 2; block++) {
>> - bh = sb_bread(sb, block);
>> - if (!bh) {
>> + page = read_mapping_page(sb->s_bdev->bd_inode->i_mapping,
>> + block, NULL);
>
> You need to use bd_mapping, not bd_inode->i_mapping (since May 21 in
> Linus' tree).

Will update it once f2fs codebase was rebased to linus' tree.

>
> And I don't think there's much point in switching to pages as an
> intermediate step. You may as well go straight to folios.
>
> folio = read_mapping_folio(sb->s_bdev->bd_mapping, block, NULL);

Fine, let me work on this in v2.

Thanks,

>