fsck.f2fs: don't allocate new blocks on unclean shutdown
PATCH 1/1 fsck.f2fs: don't allocate new blocks on unclean shutdown
We have to keep data for roll-forward recovery. Without this patch, we're able to lose there-in data by quota overwrites.
--- fsck/fsck.c | 14 ++++++++++++-- fsck/fsck.h | 4 ++-- fsck/mkquota.c | 3 ++- fsck/node.c | 9 ++++++++- fsck/quotaio.c | 3 ++- fsck/quotaio_tree.c | 26 ++++++++++++++++---------- fsck/quotaio_tree.h | 2 +- fsck/quotaio_v2.c | 2 +- fsck/segment.c | 21 +++++++++++++++++---- include/f2fs_fs.h | 1 + 10 files changed, 62 insertions(+), 23 deletions(-)
fsck/fsck.c +12 -2
@@ -1988,8 +1988,9 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)struct f2fs_super_block *sb =F2FS_RAW_SUPER (sbi);struct f2fs_checkpoint *cp =F2FS_CKPT (sbi);unsigned long long cp_blk_no; -u32 flags =CP_UMOUNT_FLAG ; +u32 flags = c.alloc_failed ?CP_FSCK_FLAG :CP_UMOUNT_FLAG ;block_t orphan_blks =0 ; +block_t cp_blocks;u32 i;int ret;u_int32_t crc =0 ;
@@ -2001,7 +2002,13 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)if (is_set_ckpt_flags (cp,CP_DISABLED_FLAG )) flags |=CP_DISABLED_FLAG ; -set_cp (cp_pack_total_block_count,8 + orphan_blks +get_sb (cp_payload)); +if (flags &CP_UMOUNT_FLAG ) + cp_blocks =8 ; +else + cp_blocks =5 ; + +set_cp (cp_pack_total_block_count, cp_blocks + + orphan_blks +get_sb (cp_payload)); flags =update_nat_bits_flags (sb, cp, flags); flags |=CP_NOCRC_RECOVERY_FLAG ;
@@ -2033,6 +2040,9 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)for (i =0 ; i <NO_CHECK_TYPE ; i++) {struct curseg_info *curseg =CURSEG_I (sbi, i); +if (!(flags &CP_UMOUNT_FLAG ) &&IS_NODESEG (i)) +continue ; + ret =dev_write_block (curseg->sum_blk, cp_blk_no++);ASSERT (ret >=0 ); }
fsck/fsck.h +2 -2
@@ -235,7 +235,7 @@ int f2fs_sload(struct f2fs_sb_info *);/* segment.c */ void reserve_new_block (struct f2fs_sb_info *,block_t *,struct f2fs_summary *,int ); -void new_data_block (struct f2fs_sb_info *,void *, +int new_data_block (struct f2fs_sb_info *,void *,struct dnode_of_data *,int );int f2fs_build_file (struct f2fs_sb_info *,struct dentry *);void f2fs_alloc_nid (struct f2fs_sb_info *,nid_t *,int );
@@ -248,7 +248,7 @@ u64 f2fs_read(struct f2fs_sb_info *, nid_t, u8 *, u64, pgoff_t);u64 f2fs_write (struct f2fs_sb_info *,nid_t ,u8 *,u64 ,pgoff_t );void f2fs_filesize_update (struct f2fs_sb_info *,nid_t ,u64 ); -void get_dnode_of_data (struct f2fs_sb_info *,struct dnode_of_data *, +int get_dnode_of_data (struct f2fs_sb_info *,struct dnode_of_data *,pgoff_t ,int );void make_dentry_ptr (struct f2fs_dentry_ptr *,struct f2fs_node *,void *,int );int f2fs_create (struct f2fs_sb_info *,struct dentry *);
fsck/mkquota.c +2 -1
@@ -53,7 +53,8 @@ static void write_dquots(dict_t *dict, struct quota_handle *qh)print_dquot ("write" , dq); dq->dq_h = qh;update_grace_times (dq); - qh->qh_ops->commit_dquot (dq); +if (qh->qh_ops->commit_dquot (dq)) +break ; } } }
fsck/node.c +8 -1
@@ -179,7 +179,7 @@ got:return level; } -void get_dnode_of_data (struct f2fs_sb_info *sbi,struct dnode_of_data *dn, +int get_dnode_of_data (struct f2fs_sb_info *sbi,struct dnode_of_data *dn,pgoff_t index,int mode) {int offset[4 ];
@@ -205,6 +205,12 @@ void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,for (i =1 ; i <= level; i++) {if (!nids[i] && mode ==ALLOC_NODE ) { +struct f2fs_checkpoint *cp =F2FS_CKPT (sbi); + +if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }f2fs_alloc_nid (sbi, &nids[i],0 ); dn->nid = nids[i];
@@ -247,4 +253,5 @@ void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn, dn->ofs_in_node = offset[level]; dn->data_blkaddr =datablock_addr (dn->node_blk, dn->ofs_in_node); dn->node_blkaddr = nblk[level]; +return 0 ; }
fsck/quotaio.c +2 -1
@@ -80,7 +80,8 @@ static unsigned int quota_write_nomount(struct quota_file *qf, written =f2fs_write (qf->sbi, qf->ino, buf, size, offset);if (qf->filesize < offset + written) qf->filesize = offset + written; - +if (written != size) +return -EIO ;return written; }
fsck/quotaio_tree.c +16 -10
@@ -291,7 +291,9 @@ static int do_insert_tree(struct quota_handle *h, struct dquot *dquot,if (newson && ret >=0 ) { ref[get_index (dquot->dq_id, depth)] =cpu_to_le32 (newblk); -write_blk (h, *treeblk, buf); + ret =write_blk (h, *treeblk, buf); +if (ret) + goto out_buf; }else if (newact && ret <0 ) {put_free_dqblk (h, buf, *treeblk); }
@@ -302,17 +304,20 @@ out_buf: }/* Wrapper for inserting quota structure into tree */ -static void dq_insert_tree (struct quota_handle *h,struct dquot *dquot) +static int dq_insert_tree (struct quota_handle *h,struct dquot *dquot) {unsigned int tmp =QT_TREEOFF ; +int err; -if (do_insert_tree (h, dquot, &tmp,0 ) <0 ) + err =do_insert_tree (h, dquot, &tmp,0 ); +if (err <0 )log_err ("Cannot write quota (id %u): %s" , (unsigned int ) dquot->dq_id,strerror (errno)); +return err; }/* Write dquot to file */ -void qtree_write_dquot (struct dquot *dquot) +int qtree_write_dquot (struct dquot *dquot) {errcode_t retval;unsigned int ret;
@@ -321,21 +326,22 @@ void qtree_write_dquot(struct dquot *dquot)struct qtree_mem_dqinfo *info = &dquot->dq_h->qh_info.u.v2_mdqi.dqi_qtree; -log_debug ("writing ddquot 1: off=%llu, info->dqi_entry_size=%u" , dquot->dq_dqb.u.v2_mdqb.dqb_off, info->dqi_entry_size); retval =quota_get_mem (info->dqi_entry_size, &ddquot);if (retval) { - errno =ENOMEM ;log_err ("Quota write failed (id %u): %s" , (unsigned int )dquot->dq_id,strerror (errno)); -return ; +return -ENOMEM ; }memset (ddquot,0 , info->dqi_entry_size);if (!dquot->dq_dqb.u.v2_mdqb.dqb_off) { -dq_insert_tree (dquot->dq_h, dquot); +if (dq_insert_tree (dquot->dq_h, dquot)) { +quota_free_mem (&ddquot); +return -EIO ; + } } info->dqi_ops->mem2disk_dqblk (ddquot, dquot); log_debug("writing ddquot 2: off=%llu, info->dqi_entry_size=%u",
@@ -345,12 +351,12 @@ void qtree_write_dquot(struct dquot *dquot) info->dqi_entry_size);if (ret != info->dqi_entry_size) { -if (ret >0 ) - errno =ENOSPC ;log_err ("Quota write failed (id %u): %s" , (unsigned int )dquot->dq_id,strerror (errno)); +return ret; }quota_free_mem (&ddquot); +return 0 ; }/* Free dquot entry in data block */
fsck/quotaio_tree.h +1 -1
@@ -58,7 +58,7 @@ struct qtree_mem_dqinfo {* manipulation */ }; -void qtree_write_dquot (struct dquot *dquot); +int qtree_write_dquot (struct dquot *dquot);struct dquot *qtree_read_dquot (struct quota_handle *h,qid_t id);void qtree_delete_dquot (struct dquot *dquot);int qtree_entry_unused (struct qtree_mem_dqinfo *info,char *disk);
fsck/quotaio_v2.c +1 -1
@@ -261,7 +261,7 @@ static int v2_commit_dquot(struct dquot *dquot) {qtree_delete_dquot (dquot); } else { -qtree_write_dquot (dquot); +return qtree_write_dquot (dquot); }return 0 ; }
fsck/segment.c +17 -4
@@ -61,12 +61,18 @@ void reserve_new_block(struct f2fs_sb_info *sbi, block_t *to,update_sum_entry (sbi, *to, sum); } -void new_data_block (struct f2fs_sb_info *sbi,void *block, +int new_data_block (struct f2fs_sb_info *sbi,void *block,struct dnode_of_data *dn,int type) {struct f2fs_summary sum;struct node_info ni;unsigned int blkaddr =datablock_addr (dn->node_blk, dn->ofs_in_node); +struct f2fs_checkpoint *cp =F2FS_CKPT (sbi); + +if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }ASSERT (dn->node_blk);memset (block,0 ,BLOCK_SZ );
@@ -80,6 +86,7 @@ void new_data_block(struct f2fs_sb_info *sbi, void *block,else if (blkaddr ==NEW_ADDR ) dn->idirty =1 ;set_data_blkaddr (dn); +return 0 ; } u64 f2fs_read(struct f2fs_sb_info *sbi, nid_t ino, u8 *buffer,
@@ -179,6 +186,7 @@ u64 f2fs_write(struct f2fs_sb_info *sbi, nid_t ino, u8 *buffer,block_t blkaddr;void * index_node =NULL ;int idirty =0 ; +int err;/* Memory allocation for block buffer and inode. */ blk_buffer =calloc (BLOCK_SZ ,2 );
@@ -196,8 +204,10 @@ u64 f2fs_write(struct f2fs_sb_info *sbi, nid_t ino, u8 *buffer,while (count >0 ) {if (remained_blkentries ==0 ) {set_new_dnode (&dn, inode,NULL , ino); -get_dnode_of_data (sbi, &dn,F2FS_BYTES_TO_BLK (offset), -ALLOC_NODE ); + err =get_dnode_of_data (sbi, &dn, +F2FS_BYTES_TO_BLK (offset),ALLOC_NODE ); +if (err) +break ; idirty |= dn.idirty;if (index_node)free (index_node);
@@ -209,7 +219,10 @@ u64 f2fs_write(struct f2fs_sb_info *sbi, nid_t ino, u8 *buffer, blkaddr =datablock_addr (dn.node_blk, dn.ofs_in_node);if (blkaddr ==NULL_ADDR || blkaddr ==NEW_ADDR ) { -new_data_block (sbi, blk_buffer, &dn,CURSEG_WARM_DATA ); + err =new_data_block (sbi, blk_buffer, + &dn,CURSEG_WARM_DATA ); +if (err) +break ; blkaddr = dn.data_blkaddr; }
include/f2fs_fs.h +1 -0
@@ -371,6 +371,7 @@ struct f2fs_configuration {int fix_on;int defset;int bug_on; +int alloc_failed;int auto_fix;int quota_fix;int preen_mode;
Signature · 1 line
2.19.0.605.g01d371f741-goog
Hi Jaegeuk,
fsck/node.c void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn, · line 213
Quoted 4 lines · Jaegeuk Kim
fsck/node.c +4
-0+if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }
When I test por_fsstress case w/ quota feature enabled, I suffer below error during repair:
Info: superblock features = f8 : extra_attr project_quota inode_checksum flexible_inline_xattr quota_ino
Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000
Info: total FS sectors = 2097152 (8192 MB)
Info: CKPT version = 452b7568
Info: checkpoint state = 846 : quota_need_fsck crc compacted_summary orphan_inodes sudden-power-off
[fsck_chk_quota_files:1784] Fixing Quota file ([ 0] ino [0x4]) [ERROR] quotaio_tree.c:83:write_blk:: Cannot write block (7188): Inappropriate ioctl for device
[ERROR] quotaio_tree.c:110:get_free_dqblk:: Cannot allocate new quota block (out of disk space).
I found new_data_block() will reject block allocation on an unclean umounted image, result in quota file repair failure.
With current implementation, I think we should mount that image again for recovery, then umount and run fsck on it again to repair quota file.
However, user didn't be noticed that how to fix this... I know there is no such report tho, I think we'd better fix this case in fsck, any thoughts? let allocation skip fsynced datas?
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Quoted 24 lines · Chao Yu
Hi Jaegeuk,
fsck/node.c void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn, · line 213
Quoted 4 lines · Jaegeuk Kim
fsck/node.c +4
-0+if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }When I test
por_fsstresscase w/ quota feature enabled, I suffer below error during repair:Info: superblock features = f8 :
extra_attrproject_quotainode_checksumflexible_inline_xattrquota_ino
Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000
Info: total FS sectors = 2097152 (8192 MB)
Info: CKPT version =452b7568
Info: checkpoint state = 846 :quota_need_fsckcrccompacted_summaryorphan_inodessudden-power-off
[fsck_chk_quota_files:1784] Fixing Quota file ([ 0] ino [0x4]) [ERROR] quotaio_tree.c:83:write_blk:: Cannot write block (7188): Inappropriate ioctl for device
[ERROR] quotaio_tree.c:110:get_free_dqblk:: Cannot allocate new quota block (out of disk space).
Quoted 5 lines
I found
new_data_block()will reject block allocation on an unclean umounted image, result in quota file repair failure.With current implementation, I think we should mount that image again for recovery, then umount and run fsck on it again to repair quota file.
Yes.
Quoted 4 lines
However, user didn't be noticed that how to fix this... I know there is no such report tho, I think we'd better fix this case in fsck, any thoughts? let allocation skip fsynced datas?
Actually, that was my TODO, but I had no time to fix it, but had to stop it to avoid further corruption.
Quoted 2 lines
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Quoted 34 lines · Jaegeuk Kim
Quoted 24 lines · Chao Yu
Hi Jaegeuk,
fsck/node.c void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn, · line 213
Quoted 4 lines · Jaegeuk Kim
fsck/node.c +4
-0+if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }When I test
por_fsstresscase w/ quota feature enabled, I suffer below error during repair:Info: superblock features = f8 :
extra_attrproject_quotainode_checksumflexible_inline_xattrquota_ino
Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000
Info: total FS sectors = 2097152 (8192 MB)
Info: CKPT version =452b7568
Info: checkpoint state = 846 :quota_need_fsckcrccompacted_summaryorphan_inodessudden-power-off
[fsck_chk_quota_files:1784] Fixing Quota file ([ 0] ino [0x4]) [ERROR] quotaio_tree.c:83:write_blk:: Cannot write block (7188): Inappropriate ioctl for device
[ERROR] quotaio_tree.c:110:get_free_dqblk:: Cannot allocate new quota block (out of disk space).Quoted 5 lines
I found
new_data_block()will reject block allocation on an unclean umounted image, result in quota file repair failure.With current implementation, I think we should mount that image again for recovery, then umount and run fsck on it again to repair quota file.
Yes.
Quoted 1 line
Quoted 6 lines
Quoted 3 lines
However, user didn't be noticed that how to fix this... I know there is no such report tho, I think we'd better fix this case in fsck, any thoughts? let allocation skip fsynced datas?
Actually, that was my TODO, but I had no time to fix it, but had to stop it to avoid further corruption.
Copied, maybe I can help to fix it. It looks it will be easy to skip fsyned datas during block allocation than porting recovery flow from kernel to tools.
Thanks,
Quoted 5 lines
Quoted 3 lines
Thanks,
.
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Quoted 39 lines · Chao Yu
Quoted 38 lines · Jaegeuk Kim
Quoted 24 lines · Chao Yu
Hi Jaegeuk,
fsck/node.c void get_dnode_of_data(struct f2fs_sb_info *sbi, struct dnode_of_data *dn, · line 213
Quoted 4 lines · Jaegeuk Kim
fsck/node.c +4
-0+if (!is_set_ckpt_flags (cp,CP_UMOUNT_FLAG )) { + c.alloc_failed =1 ; +return -EINVAL ; + }When I test
por_fsstresscase w/ quota feature enabled, I suffer below error during repair:Info: superblock features = f8 :
extra_attrproject_quotainode_checksumflexible_inline_xattrquota_ino
Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000
Info: total FS sectors = 2097152 (8192 MB)
Info: CKPT version =452b7568
Info: checkpoint state = 846 :quota_need_fsckcrccompacted_summaryorphan_inodessudden-power-off
[fsck_chk_quota_files:1784] Fixing Quota file ([ 0] ino [0x4]) [ERROR] quotaio_tree.c:83:write_blk:: Cannot write block (7188): Inappropriate ioctl for device
[ERROR] quotaio_tree.c:110:get_free_dqblk:: Cannot allocate new quota block (out of disk space).Quoted 5 lines
I found
new_data_block()will reject block allocation on an unclean umounted image, result in quota file repair failure.With current implementation, I think we should mount that image again for recovery, then umount and run fsck on it again to repair quota file.
Yes.
Quoted 4 lines
However, user didn't be noticed that how to fix this... I know there is no such report tho, I think we'd better fix this case in fsck, any thoughts? let allocation skip fsynced datas?
Quoted 5 lines
Quoted 2 lines
Actually, that was my TODO, but I had no time to fix it, but had to stop it to avoid further corruption.
Copied, maybe I can help to fix it. It looks it will be easy to skip fsyned datas during block allocation than porting recovery flow from kernel to tools.
Cool, thanks. :)
Quoted 8 lines
Thanks,
Quoted 5 lines
Quoted 3 lines
Thanks,
.
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel