Re: [f2fs-dev] [bug report] f2fs: issue small discard by LBA order
Hi Dan,
On 2018/8/7 19:19, Dan Carpenter wrote:
> On Tue, Aug 07, 2018 at 06:52:45PM +0800, Chao Yu wrote:
>> Hi Dan,
>>
>> Thanks for your report, let me try fixing this issue.
>>
>> Out of curiosity, the bug was introduced in __issue_discard_cmd_range() early
>> instead of recent commit 20ee4382322c ("f2fs: issue small discard by LBA
>> order"), did Smatch missed to check original commit?
>>
>
> It does warn about __issue_discard_cmd_range() but I didn't look at
> which patch introduced that warning...
Actually, it was introduced at 2017, I guess Smatch has missed to check below
patch due to some reason, if we have already setup to check each commit of
released linux.
8412663d177d9 ("f2fs: support issuing/waiting discard in range")
Author's commit time: 2017-10-04 09:08:32 +0800
>
> This code is several months old, but I don't know why it's only warning
> about it now.
Anyway, I sent a patch to fix this issue, I have simply tested with fault
injection in discard flow, it looks there is no further error reported.
[PATCH] f2fs: fix use-after-free of dicard command entry
Thanks,
>
> regards,
> dan carpenter
>
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: fix use-after-free of dicard command entry
As Dan Carpenter reported:
The patch 20ee4382322c: "f2fs: issue small discard by LBA order" from
Jul 8, 2018, leads to the following Smatch warning:
fs/f2fs/segment.c:1277 __issue_discard_cmd_orderly()
warn: 'dc' was already freed.
See also:
fs/f2fs/segment.c:2550 __issue_discard_cmd_range() warn: 'dc' was already freed.
In order to fix this issue, let's get error from __submit_discard_cmd(),
and release current discard command after we referenced next one.
Reported-by: Dan Carpenter
Signed-off-by: Chao Yu
---
fs/f2fs/segment.c | 79 +++
1 file changed, 45 insertions(+), 34 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index be1bf38400ca..8826ea683804 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -998,7 +998,7 @@ static void __update_discard_tree_range(struct f2fs_sb_info
*sbi,
struct block_device *bdev, block_t lstart,
block_t start, block_t len);
/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
-static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
+static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
struct discard_policy *dpolicy,
struct discard_cmd *dc,
unsigned int *issued)
@@ -1015,10 +1015,10 @@ static void __submit_discard_cmd(struct f2fs_sb_info
*sbi,
int err = 0;
if (dc->state != D_PREP)
- return;
+ return 0;
if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
- return;
+ return 0;
trace_f2fs_issue_discard(bdev, dc->start, dc->len);
@@ -1057,43 +1057,44 @@ static void __submit_discard_cmd(struct f2fs_sb_info
*sbi,
SECTOR_FROM_BLOCK(len),
GFP_NOFS, 0, );
submit:
- if (!err && bio) {
- /*
-* should keep before submission to avoid D_DONE
-* right away
-*/
+ if (err) {
spin_lock_irqsave(>lock, flags);
- if (last)
+ if (dc->state == D_PARTIAL)
dc->state = D_SUBMIT;
- else
- dc->state = D_PARTIAL;
- dc->bio_ref++;
spin_unlock_irqrestore(>lock, flags);
- atomic_inc(>issing_discard);
- dc->issuing++;
- list_move_tail(>list, wait_list);
+ break;
+ }
- /* sanity check on discard range */
- __check_sit_bitmap(sbi, start, start + len);
+ f2fs_bug_on(sbi, !bio);
- bio->bi_private = dc;
- bio->bi_end_io = f2fs_submit_discard_endio;
- bio->bi_opf |= flag;
- submit_bio(bio);
+ /*
+* should keep before submission to avoid D_DONE
+* right away
+*/
+ spin_lock_irqsave(>lock, flags);
+ if (last)
+ dc->state = D_SUBMIT;
+ else
+ dc->state = D_PARTIAL;
+ dc->bio_ref++;
+ spin_unlock_irqrestore(>lock, flags);
- atomic_inc(>issued_discard);
+ atomic_inc(>issing_discard);
+ dc->issuing++;
+ list_move_tail(>list, wait_list);
- f2fs_update_iostat(sbi, FS_DISCARD, 1);
- } else {
- spin_lock_irqsave(>lock, flags);
- if (dc->state == D_PARTIAL)
- dc->state = D_SUBMIT;
- spin_unlock_irqrestore(>lock, flags);
+ /* sanity check on discard range */
+ __check_sit_bitmap(sbi, start, start + len);
- __remove_discard_cmd(sbi, dc);
- err = -EIO;
- }
+ bio->bi_private = dc;
+ bio->bi_end_io = f2fs_submit_discard_endio;
+ bio->bi_opf |= flag;
+ submit_bio(bio);
+
+ atomic_inc(>issued_discard);
+
+ f2fs_update_iostat(sbi, FS_DISCARD, 1);
lstart += len;
start += len;
@@ -1101,8 +1102,9 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
len = total_len;
}
- if (len)
+ if (!err && len)
__update_discard_tree_range(sbi, bdev, lstart, start, len);
+ return err;
}
static struct discard_cmd *__insert_discard_tree(struct
[f2fs-dev] [PATCH v3] f2fs: checkpoint disabling
This adds a lightweight non-persistent snapshotting scheme to f2fs.
To use, mount with the option checkpoint=disable, and to return to
normal operation, remount with checkpoint=enable. If the filesystem
is shut down before remounting with checkpoint=enable, it will revert
back to its apparent state when it was first mounted with
checkpoint=disable. This is useful for situations where you wish to be
able to roll back the state of the disk in case of some critical
failure.
Signed-off-by: Daniel Rosenberg
---
v3: Rebased, and fixed issue in inc_valid_block_count from kbuild-all.
It turns out there was a separate issue in that code anyways, with
log_blocks_per_seg vs blocks_per_seg.
v2: Included changes suggested by Jaegeuk and Chao.
it now holds the gc lock around setting up free space tracking in
f2fs_disable_checkpoint. I wasn't quite sure what was mean by
the comment on should_update_outplace, but I've changed it to return
true for NEW_ADDR as well. It may be better to instead skip over the
reset of that block if old_blkaddr is NEW_ADDR, because I think that
means the data has not yet been committed, and thus wouldn't be
overwriting data from the previous checkpoint.
We're currently using this during updates to extend the period of time
that we can safely roll back a faulty update. After performing an update
and reboot, we'd mount in this mode. If there ends up being some sort of
fatal error, data changes are automatically reverted, allowing us to
revert to the state before the update without worrying about any incompatible
changes that may've been made to data while running under the faulty system.
It increases our ability to revert a faulty update from up to mounting user
data partitions to a bit farther along.
Documentation/filesystems/f2fs.txt | 5 ++
fs/f2fs/data.c | 23 ++
fs/f2fs/f2fs.h | 56 ++
fs/f2fs/file.c | 18 +
fs/f2fs/gc.c | 4 +
fs/f2fs/segment.c | 58 ---
fs/f2fs/segment.h | 26 +++
fs/f2fs/super.c| 116 +++--
8 files changed, 290 insertions(+), 16 deletions(-)
diff --git a/Documentation/filesystems/f2fs.txt
b/Documentation/filesystems/f2fs.txt
index 69f8de9957397..a026b353a99d4 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -193,6 +193,11 @@ fsync_mode=%s Control the policy of fsync.
Currently supports "posix",
non-atomic files likewise "nobarrier" mount option.
test_dummy_encryption Enable dummy encryption, which provides a fake fscrypt
context. The fake fscrypt context is used by xfstests.
+checkpoint=%s Set to "disable" to turn off checkpointing. Set to
"enable"
+ to reenable checkpointing. Is enabled by default. While
+ disabled, any unmounting or unexpected shutdowns will
cause
+ the filesystem contents to appear as they did when the
+ filesystem was mounted with that option.
DEBUGFS ENTRIES
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 45f043ee48bdb..c43c198b373cc 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1692,6 +1692,19 @@ bool f2fs_should_update_outplace(struct inode *inode,
struct f2fs_io_info *fio)
if (test_opt(sbi, LFS))
return true;
+ if (test_opt(sbi, DISABLE_CHECKPOINT)) {
+ struct seg_entry *se;
+ unsigned int segno, offset;
+
+ if (!fio || fio->old_blkaddr == NULL_ADDR ||
+ fio->old_blkaddr == NEW_ADDR)
+ return true;
+ segno = GET_SEGNO(sbi, fio->old_blkaddr);
+ se = get_seg_entry(sbi, segno);
+ offset = GET_BLKOFF_FROM_SEG0(sbi, fio->old_blkaddr);
+ if (f2fs_test_bit(offset, se->ckpt_valid_map))
+ return true;
+ }
if (S_ISDIR(inode->i_mode))
return true;
if (f2fs_is_atomic_file(inode))
@@ -1719,10 +1732,13 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
{
struct page *page = fio->page;
struct inode *inode = page->mapping->host;
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct dnode_of_data dn;
struct extent_info ei = {0,0,0};
struct node_info ni;
bool ipu_force = false;
+ bool need_tmp_grab = test_opt(sbi, DISABLE_CHECKPOINT);
+ blkcnt_t tmp_block = 1;
int err = 0;
set_new_dnode(, inode, NULL, NULL, 0);
@@ -1800,6 +1816,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
if (err)
goto out_writepage;
+ if (need_tmp_grab) {
+ err = inc_valid_block_count(sbi, dn.inode,
Re: [f2fs-dev] [bug report] f2fs: issue small discard by LBA order
On Tue, Aug 07, 2018 at 06:52:45PM +0800, Chao Yu wrote:
> Hi Dan,
>
> Thanks for your report, let me try fixing this issue.
>
> Out of curiosity, the bug was introduced in __issue_discard_cmd_range() early
> instead of recent commit 20ee4382322c ("f2fs: issue small discard by LBA
> order"), did Smatch missed to check original commit?
>
It does warn about __issue_discard_cmd_range() but I didn't look at
which patch introduced that warning...
This code is several months old, but I don't know why it's only warning
about it now.
regards,
dan carpenter
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [bug report] f2fs: issue small discard by LBA order
Hi Dan,
Thanks for your report, let me try fixing this issue.
Out of curiosity, the bug was introduced in __issue_discard_cmd_range() early
instead of recent commit 20ee4382322c ("f2fs: issue small discard by LBA
order"), did Smatch missed to check original commit?
Thanks,
On 2018/8/7 16:17, Dan Carpenter wrote:
> Hello Chao Yu,
>
> The patch 20ee4382322c: "f2fs: issue small discard by LBA order" from
> Jul 8, 2018, leads to the following Smatch warning:
>
> fs/f2fs/segment.c:1277 __issue_discard_cmd_orderly()
> warn: 'dc' was already freed.
>
> fs/f2fs/segment.c
> 1260 while (dc) {
> 1261 struct rb_node *node;
> 1262
> 1263 if (dc->state != D_PREP)
> 1264 goto next;
> 1265
> 1266 if (dpolicy->io_aware && !is_idle(sbi)) {
> 1267 io_interrupted = true;
> 1268 break;
> 1269 }
> 1270
> 1271 dcc->next_pos = dc->lstart + dc->len;
> 1272 __submit_discard_cmd(sbi, dpolicy, dc, );
>^^
> Smatch is complaining that __submit_discard_cmd() frees dc on the
> error path.
>
> 1273
> 1274 if (issued >= dpolicy->max_requests)
> 1275 break;
> 1276 next:
> 1277 node = rb_next(>rb_node);
> ^^^
> Dereference.
>
> 1278 dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> 1279 }
> 1280
> 1281 blk_finish_plug();
> 1282
>
> See also:
> fs/f2fs/segment.c:2550 __issue_discard_cmd_range() warn: 'dc' was already
> freed.
>
> regards,
> dan carpenter
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [RFC PATCH 3/3] f2fs-tools: introduce sb checksum
On 2018/8/7 17:17, Junling Zheng wrote:
> This patch introduced superblock checksum.
>
> Signed-off-by: Junling Zheng
> ---
> fsck/mount.c | 52 ++
> fsck/resize.c | 11 ++
> include/f2fs_fs.h | 6 +-
> mkfs/f2fs_format.c | 9
> 4 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 957f531..b29af3c 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
> DISP_u32(sb, node_ino);
> DISP_u32(sb, meta_ino);
> DISP_u32(sb, cp_payload);
> + DISP_u32(sb, crc);
> DISP("%-.256s", sb, version);
> printf("\n");
> }
> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
> if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
> MSG(0, "%s", " lost_found");
> }
> + if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> + MSG(0, "%s", " sb_checksum");
> + }
> MSG(0, "\n");
> MSG(0, "Info: superblock encrypt level = %d, salt = ",
> sb->encryption_level);
> @@ -602,6 +606,20 @@ int sanity_check_raw_super(struct f2fs_super_block *sb,
> u64 offset)
>
> if (sanity_check_area_boundary(sb, offset))
> return -1;
> +
> + if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> + if (CHKSUM_OFFSET_SB != get_sb(checksum_offset)) {
> + MSG(0, "\tInvalid SB CRC offset: %u\n",
> + get_sb(checksum_offset));
> + return -1;
> + }
> + if (f2fs_crc_valid(get_sb(crc), sb,
> + get_sb(checksum_offset))) {
> + MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> + return -1;
> + }
> + }
> +
> return 0;
> }
>
> @@ -609,6 +627,7 @@ int validate_super_block(struct f2fs_sb_info *sbi, int
> block)
> {
> u64 offset;
> char buf[F2FS_BLKSIZE];
> + u32 old_crc, new_crc;
>
> sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
>
> @@ -646,6 +665,18 @@ int validate_super_block(struct f2fs_sb_info *sbi, int
> block)
>
> memcpy(sbi->raw_super->version,
> c.version, VERSION_LEN);
> + if (le32_to_cpu(sbi->raw_super->feature) &
> + F2FS_FEATURE_SB_CHKSUM) {
> + /* Recalculate CRC */
> + old_crc = le32_to_cpu(sbi->raw_super->crc);
> + new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC,
> + sbi->raw_super,
> +
> CHKSUM_OFFSET_SB);
> + sbi->raw_super->crc = cpu_to_le32(new_crc);
> + MSG(0, "Info: update SB CRC successfully"
> + "(0x%x --> 0x%x)\n", old_crc, new_crc);
> + }
As most of these codes are copied one, how about introducing functions to avoid
redundant codes and improve readability:
verify_sb_checksum()
update_sb_checksum(, bool initialize/update)
> +
> ret = dev_write(sbi->raw_super, offset,
> sizeof(struct f2fs_super_block));
> ASSERT(ret >= 0);
> @@ -2453,6 +2484,7 @@ static int check_sector_size(struct f2fs_super_block
> *sb)
> int index;
> u_int32_t log_sectorsize, log_sectors_per_block;
> u_int8_t *zero_buff;
> + u32 old_crc, new_crc;
>
> log_sectorsize = log_base_2(c.sector_size);
> log_sectors_per_block = log_base_2(c.sectors_per_blk);
> @@ -2467,6 +2499,16 @@ static int check_sector_size(struct f2fs_super_block
> *sb)
> set_sb(log_sectorsize, log_sectorsize);
> set_sb(log_sectors_per_block, log_sectors_per_block);
>
> + if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> + /* Recalculate CRC */
> + old_crc = get_sb(crc);
> + new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> + CHKSUM_OFFSET_SB);
> + set_sb(crc, new_crc);
> + MSG(0, "Info: update SB CRC successfully"
> + "(0x%x --> 0x%x)\n", old_crc, new_crc);
> + }
> +
> memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
> for (index = 0; index < 2; index++) {
> @@ -2485,6 +2527,7 @@ static int check_sector_size(struct f2fs_super_block
> *sb)
> static void tune_sb_features(struct f2fs_sb_info *sbi)
> {
> int sb_changed = 0;
> + u32 old_crc, new_crc;
> struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
>
>
Re: [f2fs-dev] [RFC PATCH 2/3] f2fs-tools: rename CHECKSUM_OFFSET to CHKSUM_OFFSET_CP
On 2018/8/7 17:17, Junling Zheng wrote: > This patch renamed CHECKSUM_OFFSET to CHKSUM_OFFSET_CP. Hmm... How about CP_CHECKSUM_OFFSET? Thanks, -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [RFC PATCH v2 1/3] f2fs: support superblock checksum
On 2018/8/7 17:39, Junling Zheng wrote:
> Now we support crc32 checksum for superblock.
>
> Signed-off-by: Junling Zheng
> ---
> v1 -> v1:
> - fix to switch endian of crc.
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/super.c | 28
> include/linux/f2fs_fs.h | 3 ++-
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4525f4f82af0..d404ede01fcd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -147,6 +147,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_INODE_CRTIME0x0100
> #define F2FS_FEATURE_LOST_FOUND 0x0200
> #define F2FS_FEATURE_VERITY 0x0400 /* reserved */
> +#define F2FS_FEATURE_SB_CHKSUM 0x0800
Need to adjust sysfs entry for this new feature.
>
> #define F2FS_HAS_FEATURE(sb, mask) \
> ((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index bd57be470e23..c782fab52a12 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2149,6 +2149,8 @@ static int sanity_check_raw_super(struct f2fs_sb_info
> *sbi,
> (bh->b_data + F2FS_SUPER_OFFSET);
> struct super_block *sb = sbi->sb;
> unsigned int blocksize;
> + size_t crc_offset = 0;
> + __u32 crc = 0;
>
> if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
> f2fs_msg(sb, KERN_INFO,
> @@ -2283,6 +2285,23 @@ static int sanity_check_raw_super(struct f2fs_sb_info
> *sbi,
> if (sanity_check_area_boundary(sbi, bh))
> return 1;
>
> + if (F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
> + crc_offset = le32_to_cpu(raw_super->checksum_offset);
> + if (crc_offset !=
> + offsetof(struct f2fs_super_block, crc)) {
> + f2fs_msg(sb, KERN_INFO,
> + "Invalid SB checksum offset: %lu",
> + crc_offset);
> + return 1;
> + }
> + crc = le32_to_cpu(raw_super->crc);
> + if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
> + f2fs_msg(sb, KERN_INFO,
> + "Invalid SB checksum value: %u", crc);
> + return 1;
> + }
> + }
How about moving above codes to the beginning of sanity_check_raw_super(), since
once crc value is wrong, all other verification is unneeded.
Thanks,
> +
> return 0;
> }
>
> @@ -2568,6 +2587,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;
> + __u32 crc = 0;
> int err;
>
> if ((recover && f2fs_readonly(sbi->sb)) ||
> @@ -2576,6 +2596,14 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool
> recover)
> return -EROFS;
> }
>
> + /* we should update superblock crc here */
> + if (!recover &&
> + F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
> + crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
> + offsetof(struct f2fs_super_block, crc));
> + F2FS_RAW_SUPER(sbi)->crc = cpu_to_le32(crc);
> + }
> +
> /* write back-up superblock first */
> bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
> if (!bh)
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index f70f8ac9c4f4..aa4b586569c1 100644
> --- a/include/linux/f2fs_fs.h
> +++ b/include/linux/f2fs_fs.h
> @@ -112,7 +112,8 @@ struct f2fs_super_block {
> struct f2fs_device devs[MAX_DEVICES]; /* device list */
> __le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
> __u8 hot_ext_count; /* # of hot file extension */
> - __u8 reserved[314]; /* valid reserved region */
> + __u8 reserved[310]; /* valid reserved region */
> + __le32 crc; /* checksum of superblock */
> } __packed;
>
> /*
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [RFC PATCH v2 1/3] f2fs: support superblock checksum
Now we support crc32 checksum for superblock.
Signed-off-by: Junling Zheng
---
v1 -> v1:
- fix to switch endian of crc.
fs/f2fs/f2fs.h | 1 +
fs/f2fs/super.c | 28
include/linux/f2fs_fs.h | 3 ++-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4525f4f82af0..d404ede01fcd 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,6 +147,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_INODE_CRTIME 0x0100
#define F2FS_FEATURE_LOST_FOUND0x0200
#define F2FS_FEATURE_VERITY0x0400 /* reserved */
+#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_HAS_FEATURE(sb, mask) \
((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bd57be470e23..c782fab52a12 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2149,6 +2149,8 @@ static int sanity_check_raw_super(struct f2fs_sb_info
*sbi,
(bh->b_data + F2FS_SUPER_OFFSET);
struct super_block *sb = sbi->sb;
unsigned int blocksize;
+ size_t crc_offset = 0;
+ __u32 crc = 0;
if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
f2fs_msg(sb, KERN_INFO,
@@ -2283,6 +2285,23 @@ static int sanity_check_raw_super(struct f2fs_sb_info
*sbi,
if (sanity_check_area_boundary(sbi, bh))
return 1;
+ if (F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
+ crc_offset = le32_to_cpu(raw_super->checksum_offset);
+ if (crc_offset !=
+ offsetof(struct f2fs_super_block, crc)) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid SB checksum offset: %lu",
+ crc_offset);
+ return 1;
+ }
+ crc = le32_to_cpu(raw_super->crc);
+ if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid SB checksum value: %u", crc);
+ return 1;
+ }
+ }
+
return 0;
}
@@ -2568,6 +2587,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;
+ __u32 crc = 0;
int err;
if ((recover && f2fs_readonly(sbi->sb)) ||
@@ -2576,6 +2596,14 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool
recover)
return -EROFS;
}
+ /* we should update superblock crc here */
+ if (!recover &&
+ F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
+ crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
+ offsetof(struct f2fs_super_block, crc));
+ F2FS_RAW_SUPER(sbi)->crc = cpu_to_le32(crc);
+ }
+
/* write back-up superblock first */
bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
if (!bh)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index f70f8ac9c4f4..aa4b586569c1 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -112,7 +112,8 @@ struct f2fs_super_block {
struct f2fs_device devs[MAX_DEVICES]; /* device list */
__le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
__u8 hot_ext_count; /* # of hot file extension */
- __u8 reserved[314]; /* valid reserved region */
+ __u8 reserved[310]; /* valid reserved region */
+ __le32 crc; /* checksum of superblock */
} __packed;
/*
--
2.18.0
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [RFC PATCH] f2fs: support superblock checksum
Now we support crc32 checksum for superblock.
Signed-off-by: Junling Zheng
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/super.c | 28
include/linux/f2fs_fs.h | 3 ++-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4525f4f82af0..d404ede01fcd 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,6 +147,7 @@ struct f2fs_mount_info {
#define F2FS_FEATURE_INODE_CRTIME 0x0100
#define F2FS_FEATURE_LOST_FOUND0x0200
#define F2FS_FEATURE_VERITY0x0400 /* reserved */
+#define F2FS_FEATURE_SB_CHKSUM 0x0800
#define F2FS_HAS_FEATURE(sb, mask) \
((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bd57be470e23..dbb1a2bb9e8e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2149,6 +2149,8 @@ static int sanity_check_raw_super(struct f2fs_sb_info
*sbi,
(bh->b_data + F2FS_SUPER_OFFSET);
struct super_block *sb = sbi->sb;
unsigned int blocksize;
+ size_t crc_offset = 0;
+ __u32 crc = 0;
if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
f2fs_msg(sb, KERN_INFO,
@@ -2283,6 +2285,23 @@ static int sanity_check_raw_super(struct f2fs_sb_info
*sbi,
if (sanity_check_area_boundary(sbi, bh))
return 1;
+ if (F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
+ crc_offset = le32_to_cpu(raw_super->checksum_offset);
+ if (crc_offset !=
+ offsetof(struct f2fs_super_block, crc)) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid SB checksum offset: %lu",
+ crc_offset);
+ return 1;
+ }
+ crc = le32_to_cpu(raw_super->crc);
+ if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid SB checksum value: %u", crc);
+ return 1;
+ }
+ }
+
return 0;
}
@@ -2568,6 +2587,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;
+ __u32 crc = 0;
int err;
if ((recover && f2fs_readonly(sbi->sb)) ||
@@ -2576,6 +2596,14 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool
recover)
return -EROFS;
}
+ /* we should update superblock crc here */
+ if (!recover &&
+ F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
+ crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
+ offsetof(struct f2fs_super_block, crc));
+ F2FS_RAW_SUPER(sbi)->crc = crc;
+ }
+
/* write back-up superblock first */
bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
if (!bh)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index f70f8ac9c4f4..aa4b586569c1 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -112,7 +112,8 @@ struct f2fs_super_block {
struct f2fs_device devs[MAX_DEVICES]; /* device list */
__le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
__u8 hot_ext_count; /* # of hot file extension */
- __u8 reserved[314]; /* valid reserved region */
+ __u8 reserved[310]; /* valid reserved region */
+ __le32 crc; /* checksum of superblock */
} __packed;
/*
--
2.18.0
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [RFC PATCH 3/3] f2fs-tools: introduce sb checksum
This patch introduced superblock checksum.
Signed-off-by: Junling Zheng
---
fsck/mount.c | 52 ++
fsck/resize.c | 11 ++
include/f2fs_fs.h | 6 +-
mkfs/f2fs_format.c | 9
4 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/fsck/mount.c b/fsck/mount.c
index 957f531..b29af3c 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
DISP_u32(sb, node_ino);
DISP_u32(sb, meta_ino);
DISP_u32(sb, cp_payload);
+ DISP_u32(sb, crc);
DISP("%-.256s", sb, version);
printf("\n");
}
@@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
MSG(0, "%s", " lost_found");
}
+ if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
+ MSG(0, "%s", " sb_checksum");
+ }
MSG(0, "\n");
MSG(0, "Info: superblock encrypt level = %d, salt = ",
sb->encryption_level);
@@ -602,6 +606,20 @@ int sanity_check_raw_super(struct f2fs_super_block *sb,
u64 offset)
if (sanity_check_area_boundary(sb, offset))
return -1;
+
+ if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
+ if (CHKSUM_OFFSET_SB != get_sb(checksum_offset)) {
+ MSG(0, "\tInvalid SB CRC offset: %u\n",
+ get_sb(checksum_offset));
+ return -1;
+ }
+ if (f2fs_crc_valid(get_sb(crc), sb,
+ get_sb(checksum_offset))) {
+ MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
+ return -1;
+ }
+ }
+
return 0;
}
@@ -609,6 +627,7 @@ int validate_super_block(struct f2fs_sb_info *sbi, int
block)
{
u64 offset;
char buf[F2FS_BLKSIZE];
+ u32 old_crc, new_crc;
sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
@@ -646,6 +665,18 @@ int validate_super_block(struct f2fs_sb_info *sbi, int
block)
memcpy(sbi->raw_super->version,
c.version, VERSION_LEN);
+ if (le32_to_cpu(sbi->raw_super->feature) &
+ F2FS_FEATURE_SB_CHKSUM) {
+ /* Recalculate CRC */
+ old_crc = le32_to_cpu(sbi->raw_super->crc);
+ new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC,
+ sbi->raw_super,
+
CHKSUM_OFFSET_SB);
+ sbi->raw_super->crc = cpu_to_le32(new_crc);
+ MSG(0, "Info: update SB CRC successfully"
+ "(0x%x --> 0x%x)\n", old_crc, new_crc);
+ }
+
ret = dev_write(sbi->raw_super, offset,
sizeof(struct f2fs_super_block));
ASSERT(ret >= 0);
@@ -2453,6 +2484,7 @@ static int check_sector_size(struct f2fs_super_block *sb)
int index;
u_int32_t log_sectorsize, log_sectors_per_block;
u_int8_t *zero_buff;
+ u32 old_crc, new_crc;
log_sectorsize = log_base_2(c.sector_size);
log_sectors_per_block = log_base_2(c.sectors_per_blk);
@@ -2467,6 +2499,16 @@ static int check_sector_size(struct f2fs_super_block *sb)
set_sb(log_sectorsize, log_sectorsize);
set_sb(log_sectors_per_block, log_sectors_per_block);
+ if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
+ /* Recalculate CRC */
+ old_crc = get_sb(crc);
+ new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
+ CHKSUM_OFFSET_SB);
+ set_sb(crc, new_crc);
+ MSG(0, "Info: update SB CRC successfully"
+ "(0x%x --> 0x%x)\n", old_crc, new_crc);
+ }
+
memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
for (index = 0; index < 2; index++) {
@@ -2485,6 +2527,7 @@ static int check_sector_size(struct f2fs_super_block *sb)
static void tune_sb_features(struct f2fs_sb_info *sbi)
{
int sb_changed = 0;
+ u32 old_crc, new_crc;
struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
if (!(sb->feature & cpu_to_le32(F2FS_FEATURE_ENCRYPT)) &&
@@ -2499,6 +2542,15 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
if (!sb_changed)
return;
+ if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
+ /* Recalculate CRC */
+ old_crc = get_sb(crc);
+
[f2fs-dev] [RFC PATCH 2/3] f2fs-tools: rename CHECKSUM_OFFSET to CHKSUM_OFFSET_CP
This patch renamed CHECKSUM_OFFSET to CHKSUM_OFFSET_CP.
Signed-off-by: Junling Zheng
---
fsck/fsck.c| 4 ++--
fsck/mount.c | 4 ++--
fsck/resize.c | 8
include/f2fs_fs.h | 6 +++---
mkfs/f2fs_format.c | 14 +++---
5 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/fsck/fsck.c b/fsck/fsck.c
index d550403..da2fab4 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2010,8 +2010,8 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
set_cp(valid_node_count, fsck->chk.valid_node_cnt);
set_cp(valid_inode_count, fsck->chk.valid_inode_cnt);
- crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
- *((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+ crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHKSUM_OFFSET_CP);
+ *((__le32 *)((unsigned char *)cp + CHKSUM_OFFSET_CP)) =
cpu_to_le32(crc);
cp_blk_no = get_sb(cp_blkaddr);
if (sbi->cur_cp == 2)
diff --git a/fsck/mount.c b/fsck/mount.c
index 2c2473d..957f531 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2252,8 +2252,8 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
flags = update_nat_bits_flags(sb, cp, flags);
set_cp(ckpt_flags, flags);
- crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
- *((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+ crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHKSUM_OFFSET_CP);
+ *((__le32 *)((unsigned char *)cp + CHKSUM_OFFSET_CP)) =
cpu_to_le32(crc);
cp_blk_no = get_sb(cp_blkaddr);
if (sbi->cur_cp == 2)
diff --git a/fsck/resize.c b/fsck/resize.c
index fe8a61a..94580db 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -90,10 +90,10 @@ static int get_new_sb(struct f2fs_super_block *sb)
* It requires more pages for cp.
*/
if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
- max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct
f2fs_checkpoint) + 1;
+ max_nat_bitmap_size = CHKSUM_OFFSET_CP - sizeof(struct
f2fs_checkpoint) + 1;
set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
} else {
- max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct
f2fs_checkpoint) + 1
+ max_nat_bitmap_size = CHKSUM_OFFSET_CP - sizeof(struct
f2fs_checkpoint) + 1
- max_sit_bitmap_size;
set_sb(cp_payload, 0);
}
@@ -520,8 +520,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
(unsigned char *)cp);
new_cp->checkpoint_ver = cpu_to_le64(cp_ver + 1);
- crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CHECKSUM_OFFSET);
- *((__le32 *)((unsigned char *)new_cp + CHECKSUM_OFFSET)) =
+ crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CHKSUM_OFFSET_CP);
+ *((__le32 *)((unsigned char *)new_cp + CHKSUM_OFFSET_CP)) =
cpu_to_le32(crc);
/* Write a new checkpoint in the other set */
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 2c086a9..dbeeb6a 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -278,7 +278,7 @@ static inline uint64_t bswap_64(uint64_t val)
#define PAGE_CACHE_SIZE4096
#define BITS_PER_BYTE 8
#define F2FS_SUPER_MAGIC 0xF2F52010 /* F2FS Magic Number */
-#define CHECKSUM_OFFSET4092
+#define CHKSUM_OFFSET_CP 4092
#define MAX_PATH_LEN 64
#define MAX_DEVICES8
@@ -682,9 +682,9 @@ struct f2fs_checkpoint {
} __attribute__((packed));
#define MAX_SIT_BITMAP_SIZE_IN_CKPT\
- (CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+ (CHKSUM_OFFSET_CP - sizeof(struct f2fs_checkpoint) + 1 - 64)
#define MAX_BITMAP_SIZE_IN_CKPT\
- (CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+ (CHKSUM_OFFSET_CP - sizeof(struct f2fs_checkpoint) + 1)
/*
* For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4b88d93..038a589 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -342,12 +342,12 @@ static int f2fs_prepare_super_block(void)
* It requires more pages for cp.
*/
if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
- max_nat_bitmap_size = CHECKSUM_OFFSET -
+ max_nat_bitmap_size = CHKSUM_OFFSET_CP -
sizeof(struct f2fs_checkpoint) + 1;
set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
} else {
max_nat_bitmap_size =
- CHECKSUM_OFFSET - sizeof(struct
f2fs_checkpoint) + 1
+
[f2fs-dev] [bug report] f2fs: issue small discard by LBA order
Hello Chao Yu,
The patch 20ee4382322c: "f2fs: issue small discard by LBA order" from
Jul 8, 2018, leads to the following Smatch warning:
fs/f2fs/segment.c:1277 __issue_discard_cmd_orderly()
warn: 'dc' was already freed.
fs/f2fs/segment.c
1260 while (dc) {
1261 struct rb_node *node;
1262
1263 if (dc->state != D_PREP)
1264 goto next;
1265
1266 if (dpolicy->io_aware && !is_idle(sbi)) {
1267 io_interrupted = true;
1268 break;
1269 }
1270
1271 dcc->next_pos = dc->lstart + dc->len;
1272 __submit_discard_cmd(sbi, dpolicy, dc, );
^^
Smatch is complaining that __submit_discard_cmd() frees dc on the
error path.
1273
1274 if (issued >= dpolicy->max_requests)
1275 break;
1276 next:
1277 node = rb_next(>rb_node);
^^^
Dereference.
1278 dc = rb_entry_safe(node, struct discard_cmd, rb_node);
1279 }
1280
1281 blk_finish_plug();
1282
See also:
fs/f2fs/segment.c:2550 __issue_discard_cmd_range() warn: 'dc' was already freed.
regards,
dan carpenter
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
