Re: [PATCH v2] f2fs: rebuild nat_bits during umount
On 12/24, Chao Yu wrote: > On 2018/12/13 16:50, Chao Yu wrote: > > If all free_nat_bitmap are available, we can rebuild nat_bits from > > free_nat_bitmap entirely during umount, let's make another chance > > to reenable nat_bits for image. > > Jaegeuk, > > Could you please have a try with this patch? > > I just did xfstest on it with below method, and there is no data corruption > reported from fsck: Yup, will do later. > > - let fill_super preload all NAT blocks to enable free_nid_bitmap entirely. > - don't add nat_bits during mkfs image. > > Thanks, > > > > > Signed-off-by: Chao Yu > > --- > > v2: > > - fix bugs in f2fs_enable_nat_bits() > > - add kmsg to record nat_bits {en,dis}abling operations > > - fix to validate {full,empty}_nat_bits in __get_nat_bitmaps() > > > > fs/f2fs/checkpoint.c | 20 +--- > > fs/f2fs/f2fs.h | 21 ++--- > > fs/f2fs/node.c | 72 +++- > > 3 files changed, 82 insertions(+), 31 deletions(-) > > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > > index 623536b8e0c0..fa0eb56f1703 100644 > > --- a/fs/f2fs/checkpoint.c > > +++ b/fs/f2fs/checkpoint.c > > @@ -1225,12 +1225,22 @@ static void update_ckpt_flags(struct f2fs_sb_info > > *sbi, struct cp_control *cpc) > > struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); > > unsigned long flags; > > > > - spin_lock_irqsave(>cp_lock, flags); > > + if (cpc->reason & CP_UMOUNT) { > > + if (le32_to_cpu(ckpt->cp_pack_total_block_count) > > > + sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) { > > + clear_ckpt_flags(sbi, CP_NAT_BITS_FLAG); > > + f2fs_msg(sbi->sb, KERN_NOTICE, > > + "Disable nat_bits due to no space"); > > + } else if (!is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG) && > > + f2fs_nat_bitmap_enabled(sbi)) { > > + f2fs_enable_nat_bits(sbi); > > + set_ckpt_flags(sbi, CP_NAT_BITS_FLAG); > > + f2fs_msg(sbi->sb, KERN_NOTICE, > > + "Rebuild and enable nat_bits"); > > + } > > + } > > > > - if ((cpc->reason & CP_UMOUNT) && > > - le32_to_cpu(ckpt->cp_pack_total_block_count) > > > - sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) > > - disable_nat_bits(sbi, false); > > + spin_lock_irqsave(>cp_lock, flags); > > > > if (cpc->reason & CP_TRIMMED) > > __set_ckpt_flags(ckpt, CP_TRIMMED_FLAG); > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 7cec897146a3..869a36b6a28f 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -1618,25 +1618,6 @@ static inline void clear_ckpt_flags(struct > > f2fs_sb_info *sbi, unsigned int f) > > spin_unlock_irqrestore(>cp_lock, flags); > > } > > > > -static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock) > > -{ > > - unsigned long flags; > > - > > - /* > > -* In order to re-enable nat_bits we need to call fsck.f2fs by > > -* set_sbi_flag(sbi, SBI_NEED_FSCK). But it may give huge cost, > > -* so let's rely on regular fsck or unclean shutdown. > > -*/ > > - > > - if (lock) > > - spin_lock_irqsave(>cp_lock, flags); > > - __clear_ckpt_flags(F2FS_CKPT(sbi), CP_NAT_BITS_FLAG); > > - kfree(NM_I(sbi)->nat_bits); > > - NM_I(sbi)->nat_bits = NULL; > > - if (lock) > > - spin_unlock_irqrestore(>cp_lock, flags); > > -} > > - > > static inline bool enabled_nat_bits(struct f2fs_sb_info *sbi, > > struct cp_control *cpc) > > { > > @@ -2952,6 +2933,7 @@ int f2fs_truncate_inode_blocks(struct inode *inode, > > pgoff_t from); > > int f2fs_truncate_xattr_node(struct inode *inode); > > int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, > > unsigned int seq_id); > > +bool f2fs_nat_bitmap_enabled(struct f2fs_sb_info *sbi); > > int f2fs_remove_inode_page(struct inode *inode); > > struct page *f2fs_new_inode_page(struct inode *inode); > > struct page *f2fs_new_node_page(struct dnode_of_data *dn, unsigned int > > ofs); > > @@ -2975,6 +2957,7 @@ int f2fs_recover_xattr_data(struct inode *inode, > > struct page *page); > > int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page); > > int f2fs_restore_node_summary(struct f2fs_sb_info *sbi, > > unsigned int segno, struct f2fs_summary_block *sum); > > +void f2fs_enable_nat_bits(struct f2fs_sb_info *sbi); > > int f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control > > *cpc); > > int f2fs_build_node_manager(struct f2fs_sb_info *sbi); > > void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi); > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > > index 30a4427aaa94..9f55f2cb0dff 100644 > > --- a/fs/f2fs/node.c > > +++ b/fs/f2fs/node.c > > @@ -2036,6 +2036,24 @@
Re: [PATCH v2] f2fs: rebuild nat_bits during umount
On 2018/12/13 16:50, Chao Yu wrote: > If all free_nat_bitmap are available, we can rebuild nat_bits from > free_nat_bitmap entirely during umount, let's make another chance > to reenable nat_bits for image. Jaegeuk, Could you please have a try with this patch? I just did xfstest on it with below method, and there is no data corruption reported from fsck: - let fill_super preload all NAT blocks to enable free_nid_bitmap entirely. - don't add nat_bits during mkfs image. Thanks, > > Signed-off-by: Chao Yu > --- > v2: > - fix bugs in f2fs_enable_nat_bits() > - add kmsg to record nat_bits {en,dis}abling operations > - fix to validate {full,empty}_nat_bits in __get_nat_bitmaps() > > fs/f2fs/checkpoint.c | 20 +--- > fs/f2fs/f2fs.h | 21 ++--- > fs/f2fs/node.c | 72 +++- > 3 files changed, 82 insertions(+), 31 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index 623536b8e0c0..fa0eb56f1703 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -1225,12 +1225,22 @@ static void update_ckpt_flags(struct f2fs_sb_info > *sbi, struct cp_control *cpc) > struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); > unsigned long flags; > > - spin_lock_irqsave(>cp_lock, flags); > + if (cpc->reason & CP_UMOUNT) { > + if (le32_to_cpu(ckpt->cp_pack_total_block_count) > > + sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) { > + clear_ckpt_flags(sbi, CP_NAT_BITS_FLAG); > + f2fs_msg(sbi->sb, KERN_NOTICE, > + "Disable nat_bits due to no space"); > + } else if (!is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG) && > + f2fs_nat_bitmap_enabled(sbi)) { > + f2fs_enable_nat_bits(sbi); > + set_ckpt_flags(sbi, CP_NAT_BITS_FLAG); > + f2fs_msg(sbi->sb, KERN_NOTICE, > + "Rebuild and enable nat_bits"); > + } > + } > > - if ((cpc->reason & CP_UMOUNT) && > - le32_to_cpu(ckpt->cp_pack_total_block_count) > > - sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) > - disable_nat_bits(sbi, false); > + spin_lock_irqsave(>cp_lock, flags); > > if (cpc->reason & CP_TRIMMED) > __set_ckpt_flags(ckpt, CP_TRIMMED_FLAG); > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 7cec897146a3..869a36b6a28f 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1618,25 +1618,6 @@ static inline void clear_ckpt_flags(struct > f2fs_sb_info *sbi, unsigned int f) > spin_unlock_irqrestore(>cp_lock, flags); > } > > -static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock) > -{ > - unsigned long flags; > - > - /* > - * In order to re-enable nat_bits we need to call fsck.f2fs by > - * set_sbi_flag(sbi, SBI_NEED_FSCK). But it may give huge cost, > - * so let's rely on regular fsck or unclean shutdown. > - */ > - > - if (lock) > - spin_lock_irqsave(>cp_lock, flags); > - __clear_ckpt_flags(F2FS_CKPT(sbi), CP_NAT_BITS_FLAG); > - kfree(NM_I(sbi)->nat_bits); > - NM_I(sbi)->nat_bits = NULL; > - if (lock) > - spin_unlock_irqrestore(>cp_lock, flags); > -} > - > static inline bool enabled_nat_bits(struct f2fs_sb_info *sbi, > struct cp_control *cpc) > { > @@ -2952,6 +2933,7 @@ int f2fs_truncate_inode_blocks(struct inode *inode, > pgoff_t from); > int f2fs_truncate_xattr_node(struct inode *inode); > int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, > unsigned int seq_id); > +bool f2fs_nat_bitmap_enabled(struct f2fs_sb_info *sbi); > int f2fs_remove_inode_page(struct inode *inode); > struct page *f2fs_new_inode_page(struct inode *inode); > struct page *f2fs_new_node_page(struct dnode_of_data *dn, unsigned int ofs); > @@ -2975,6 +2957,7 @@ int f2fs_recover_xattr_data(struct inode *inode, struct > page *page); > int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page); > int f2fs_restore_node_summary(struct f2fs_sb_info *sbi, > unsigned int segno, struct f2fs_summary_block *sum); > +void f2fs_enable_nat_bits(struct f2fs_sb_info *sbi); > int f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc); > int f2fs_build_node_manager(struct f2fs_sb_info *sbi); > void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi); > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index 30a4427aaa94..9f55f2cb0dff 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -2036,6 +2036,24 @@ static void __move_free_nid(struct f2fs_sb_info *sbi, > struct free_nid *i, > } > } > > +bool f2fs_nat_bitmap_enabled(struct f2fs_sb_info *sbi) > +{ > + struct f2fs_nm_info *nm_i = NM_I(sbi); >
[PATCH v2] f2fs: rebuild nat_bits during umount
If all free_nat_bitmap are available, we can rebuild nat_bits from free_nat_bitmap entirely during umount, let's make another chance to reenable nat_bits for image. Signed-off-by: Chao Yu --- v2: - fix bugs in f2fs_enable_nat_bits() - add kmsg to record nat_bits {en,dis}abling operations - fix to validate {full,empty}_nat_bits in __get_nat_bitmaps() fs/f2fs/checkpoint.c | 20 +--- fs/f2fs/f2fs.h | 21 ++--- fs/f2fs/node.c | 72 +++- 3 files changed, 82 insertions(+), 31 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 623536b8e0c0..fa0eb56f1703 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -1225,12 +1225,22 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc) struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); unsigned long flags; - spin_lock_irqsave(>cp_lock, flags); + if (cpc->reason & CP_UMOUNT) { + if (le32_to_cpu(ckpt->cp_pack_total_block_count) > + sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) { + clear_ckpt_flags(sbi, CP_NAT_BITS_FLAG); + f2fs_msg(sbi->sb, KERN_NOTICE, + "Disable nat_bits due to no space"); + } else if (!is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG) && + f2fs_nat_bitmap_enabled(sbi)) { + f2fs_enable_nat_bits(sbi); + set_ckpt_flags(sbi, CP_NAT_BITS_FLAG); + f2fs_msg(sbi->sb, KERN_NOTICE, + "Rebuild and enable nat_bits"); + } + } - if ((cpc->reason & CP_UMOUNT) && - le32_to_cpu(ckpt->cp_pack_total_block_count) > - sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks) - disable_nat_bits(sbi, false); + spin_lock_irqsave(>cp_lock, flags); if (cpc->reason & CP_TRIMMED) __set_ckpt_flags(ckpt, CP_TRIMMED_FLAG); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 7cec897146a3..869a36b6a28f 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1618,25 +1618,6 @@ static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f) spin_unlock_irqrestore(>cp_lock, flags); } -static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock) -{ - unsigned long flags; - - /* -* In order to re-enable nat_bits we need to call fsck.f2fs by -* set_sbi_flag(sbi, SBI_NEED_FSCK). But it may give huge cost, -* so let's rely on regular fsck or unclean shutdown. -*/ - - if (lock) - spin_lock_irqsave(>cp_lock, flags); - __clear_ckpt_flags(F2FS_CKPT(sbi), CP_NAT_BITS_FLAG); - kfree(NM_I(sbi)->nat_bits); - NM_I(sbi)->nat_bits = NULL; - if (lock) - spin_unlock_irqrestore(>cp_lock, flags); -} - static inline bool enabled_nat_bits(struct f2fs_sb_info *sbi, struct cp_control *cpc) { @@ -2952,6 +2933,7 @@ int f2fs_truncate_inode_blocks(struct inode *inode, pgoff_t from); int f2fs_truncate_xattr_node(struct inode *inode); int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, unsigned int seq_id); +bool f2fs_nat_bitmap_enabled(struct f2fs_sb_info *sbi); int f2fs_remove_inode_page(struct inode *inode); struct page *f2fs_new_inode_page(struct inode *inode); struct page *f2fs_new_node_page(struct dnode_of_data *dn, unsigned int ofs); @@ -2975,6 +2957,7 @@ int f2fs_recover_xattr_data(struct inode *inode, struct page *page); int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page); int f2fs_restore_node_summary(struct f2fs_sb_info *sbi, unsigned int segno, struct f2fs_summary_block *sum); +void f2fs_enable_nat_bits(struct f2fs_sb_info *sbi); int f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc); int f2fs_build_node_manager(struct f2fs_sb_info *sbi); void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi); diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 30a4427aaa94..9f55f2cb0dff 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -2036,6 +2036,24 @@ static void __move_free_nid(struct f2fs_sb_info *sbi, struct free_nid *i, } } +bool f2fs_nat_bitmap_enabled(struct f2fs_sb_info *sbi) +{ + struct f2fs_nm_info *nm_i = NM_I(sbi); + unsigned int i; + bool ret = true; + + down_read(_i->nat_tree_lock); + for (i = 0; i < nm_i->nat_blocks; i++) { + if (!test_bit_le(i, nm_i->nat_block_bitmap)) { + ret = false; + break; + } + } + up_read(_i->nat_tree_lock); + + return ret; +} + static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,