On 2017/7/6 0:56, Jaegeuk Kim wrote:
> On 07/05, Chao Yu wrote:
>> From: Chao Yu <[email protected]>
>>
>> Previously, we count all inode consumed blocks including inode block,
>> xattr block, index block, data block into i_blocks, for other generic
>> file systems, they won't count inode block into i_blocks, so for
>> userspace applications or quota system, it may detect incorrect block
>> count in inode.
>>
>> This patch changes to count all blocks into inode.i_blocks excluding
>> inode block, for on-disk i_blocks, we keep counting inode block for
>> backward compatibility.
> 
> Good catch!
> I fixed return value of inc_valid_node_count inline below.

Thanks for the review. I confound this patch with the quota one.

Anyway, let me send v2.

Thanks,

> 
>>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>>  fs/f2fs/f2fs.h  | 24 +++++++++++++-----------
>>  fs/f2fs/inode.c |  4 ++--
>>  fs/f2fs/node.c  | 16 ++++++----------
>>  3 files changed, 21 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index abf9eea67966..9d5b1e4ad5d4 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1355,8 +1355,6 @@ static inline int check_nid_range(struct f2fs_sb_info 
>> *sbi, nid_t nid)
>>      return 0;
>>  }
>>  
>> -#define F2FS_DEFAULT_ALLOCATED_BLOCKS       1
>> -
>>  /*
>>   * Check whether the inode has blocks or not
>>   */
>> @@ -1364,8 +1362,7 @@ static inline int F2FS_HAS_BLOCKS(struct inode *inode)
>>  {
>>      block_t xattr_block = F2FS_I(inode)->i_xattr_nid ? 1 : 0;
>>  
>> -    return (inode->i_blocks >> F2FS_LOG_SECTORS_PER_BLOCK) >
>> -                    (F2FS_DEFAULT_ALLOCATED_BLOCKS + xattr_block);
>> +    return (inode->i_blocks >> F2FS_LOG_SECTORS_PER_BLOCK) > xattr_block;
>>  }
>>  
>>  static inline bool f2fs_has_xattr_block(unsigned int ofs)
>> @@ -1552,7 +1549,7 @@ static inline block_t __start_sum_addr(struct 
>> f2fs_sb_info *sbi)
>>  }
>>  
>>  static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
>> -                                            struct inode *inode)
>> +                                    struct inode *inode, bool is_inode)
>>  {
>>      block_t valid_block_count;
>>      unsigned int valid_node_count;
>> @@ -1572,27 +1569,32 @@ static inline bool inc_valid_node_count(struct 
>> f2fs_sb_info *sbi,
>>              return false;
>>      }
>>  
>> -    if (inode)
>> -            f2fs_i_blocks_write(inode, 1, true);
>> +    if (inode) {
>> +            if (is_inode)
>> +                    f2fs_mark_inode_dirty_sync(inode, true);
>> +            else
>> +                    f2fs_i_blocks_write(inode, 1, true);
>> +    }
>>  
>>      sbi->total_valid_node_count++;
>>      sbi->total_valid_block_count++;
>>      spin_unlock(&sbi->stat_lock);
>>  
>>      percpu_counter_inc(&sbi->alloc_valid_block_count);
>> -    return true;
>> +    return 0;
> 
> Need to return true.
> 
> Thanks,
> 
>>  }
>>  
>>  static inline void dec_valid_node_count(struct f2fs_sb_info *sbi,
>> -                                            struct inode *inode)
>> +                                    struct inode *inode, bool is_inode)
>>  {
>>      spin_lock(&sbi->stat_lock);
>>  
>>      f2fs_bug_on(sbi, !sbi->total_valid_block_count);
>>      f2fs_bug_on(sbi, !sbi->total_valid_node_count);
>> -    f2fs_bug_on(sbi, !inode->i_blocks);
>> +    f2fs_bug_on(sbi, !is_inode && !inode->i_blocks);
>>  
>> -    f2fs_i_blocks_write(inode, 1, false);
>> +    if (!is_inode)
>> +            f2fs_i_blocks_write(inode, 1, false);
>>      sbi->total_valid_node_count--;
>>      sbi->total_valid_block_count--;
>>  
>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>> index 1ff5bd418d87..e42a7a8805dc 100644
>> --- a/fs/f2fs/inode.c
>> +++ b/fs/f2fs/inode.c
>> @@ -130,7 +130,7 @@ static int do_read_inode(struct inode *inode)
>>      i_gid_write(inode, le32_to_cpu(ri->i_gid));
>>      set_nlink(inode, le32_to_cpu(ri->i_links));
>>      inode->i_size = le64_to_cpu(ri->i_size);
>> -    inode->i_blocks = SECTOR_FROM_BLOCK(le64_to_cpu(ri->i_blocks));
>> +    inode->i_blocks = SECTOR_FROM_BLOCK(le64_to_cpu(ri->i_blocks) - 1);
>>  
>>      inode->i_atime.tv_sec = le64_to_cpu(ri->i_atime);
>>      inode->i_ctime.tv_sec = le64_to_cpu(ri->i_ctime);
>> @@ -268,7 +268,7 @@ int update_inode(struct inode *inode, struct page 
>> *node_page)
>>      ri->i_gid = cpu_to_le32(i_gid_read(inode));
>>      ri->i_links = cpu_to_le32(inode->i_nlink);
>>      ri->i_size = cpu_to_le64(i_size_read(inode));
>> -    ri->i_blocks = cpu_to_le64(SECTOR_TO_BLOCK(inode->i_blocks));
>> +    ri->i_blocks = cpu_to_le64(SECTOR_TO_BLOCK(inode->i_blocks) + 1);
>>  
>>      if (et) {
>>              read_lock(&et->lock);
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index fd57ffd88508..b9f14ba6441f 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -678,15 +678,11 @@ static void truncate_node(struct dnode_of_data *dn)
>>      struct node_info ni;
>>  
>>      get_node_info(sbi, dn->nid, &ni);
>> -    if (dn->inode->i_blocks == 0) {
>> -            f2fs_bug_on(sbi, ni.blk_addr != NULL_ADDR);
>> -            goto invalidate;
>> -    }
>>      f2fs_bug_on(sbi, ni.blk_addr == NULL_ADDR);
>>  
>>      /* Deallocate node address */
>>      invalidate_blocks(sbi, ni.blk_addr);
>> -    dec_valid_node_count(sbi, dn->inode);
>> +    dec_valid_node_count(sbi, dn->inode, dn->nid == dn->inode->i_ino);
>>      set_node_addr(sbi, &ni, NULL_ADDR, false);
>>  
>>      if (dn->nid == dn->inode->i_ino) {
>> @@ -694,7 +690,7 @@ static void truncate_node(struct dnode_of_data *dn)
>>              dec_valid_inode_count(sbi);
>>              f2fs_inode_synced(dn->inode);
>>      }
>> -invalidate:
>> +
>>      clear_node_page_dirty(dn->node_page);
>>      set_sbi_flag(sbi, SBI_IS_DIRTY);
>>  
>> @@ -1044,7 +1040,7 @@ struct page *new_node_page(struct dnode_of_data *dn,
>>      if (!page)
>>              return ERR_PTR(-ENOMEM);
>>  
>> -    if (unlikely(!inc_valid_node_count(sbi, dn->inode))) {
>> +    if (unlikely(!inc_valid_node_count(sbi, dn->inode, !ofs))) {
>>              err = -ENOSPC;
>>              goto fail;
>>      }
>> @@ -2207,14 +2203,14 @@ int recover_xattr_data(struct inode *inode, struct 
>> page *page, block_t blkaddr)
>>      get_node_info(sbi, prev_xnid, &ni);
>>      f2fs_bug_on(sbi, ni.blk_addr == NULL_ADDR);
>>      invalidate_blocks(sbi, ni.blk_addr);
>> -    dec_valid_node_count(sbi, inode);
>> +    dec_valid_node_count(sbi, inode, false);
>>      set_node_addr(sbi, &ni, NULL_ADDR, false);
>>  
>>  recover_xnid:
>>      /* 2: update xattr nid in inode */
>>      remove_free_nid(sbi, new_xnid);
>>      f2fs_i_xnid_write(inode, new_xnid);
>> -    if (unlikely(!inc_valid_node_count(sbi, inode)))
>> +    if (unlikely(!inc_valid_node_count(sbi, inode, false)))
>>              f2fs_bug_on(sbi, 1);
>>      update_inode_page(inode);
>>  
>> @@ -2272,7 +2268,7 @@ int recover_inode_page(struct f2fs_sb_info *sbi, 
>> struct page *page)
>>      new_ni = old_ni;
>>      new_ni.ino = ino;
>>  
>> -    if (unlikely(!inc_valid_node_count(sbi, NULL)))
>> +    if (unlikely(!inc_valid_node_count(sbi, NULL, true)))
>>              WARN_ON(1);
>>      set_node_addr(sbi, &new_ni, NEW_ADDR, false);
>>      inc_valid_inode_count(sbi);
>> -- 
>> 2.13.0.90.g1eb437020

------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to