On Wed, Jul 18, 2018 at 02:58:06PM +0800, Qu Wenruo wrote:
>
>
>On 2018年07月18日 14:45, Lu Fengqi wrote:
>> It can be fetched from the transaction handle.
>> 
>> Signed-off-by: Lu Fengqi <lufq.f...@cn.fujitsu.com>
>> ---
>>  fs/btrfs/qgroup.c   | 13 ++++++-------
>>  fs/btrfs/qgroup.h   |  5 ++---
>>  fs/btrfs/tree-log.c |  2 +-
>>  3 files changed, 9 insertions(+), 11 deletions(-)
>> 
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index c85c1a0e933a..01add73cb2aa 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -1579,10 +1579,10 @@ int btrfs_qgroup_trace_extent_post(struct 
>> btrfs_fs_info *fs_info,
>>      return 0;
>>  }
>>  
>> -int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans,
>> -            struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes,
>> -            gfp_t gfp_flag)
>> +int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
>> +                          u64 num_bytes, gfp_t gfp_flag)
>>  {
>> +    struct btrfs_fs_info *fs_info = trans->fs_info;
>
>Just lines below, we do extra WARN_ON(trans == NULL).
>So if we really hit some case with NULL trans, this would cause a NULL
>pointer dereference.
>
>Although I have to admit, I'm a little paranoid about possible NULL
>trans passed in.
>So maybe it's a good timing to remove that WARN_ON() too?

Sorry, I didn't notice this WARN_ON(trans == NULL). However, I have
confirmed that the callers of btrfs_qgroup_trace_{extent, leaf_items,
subtree} should never pass NULL as trans. In my opinion the WARN_ON() can
be removed without any bad effect.

-- 
Thanks,
Lu

>
>Thanks,
>Qu
>
>>      struct btrfs_qgroup_extent_record *record;
>>      struct btrfs_delayed_ref_root *delayed_refs;
>>      int ret;
>> @@ -1644,8 +1644,8 @@ int btrfs_qgroup_trace_leaf_items(struct 
>> btrfs_trans_handle *trans,
>>  
>>              num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
>>  
>> -            ret = btrfs_qgroup_trace_extent(trans, fs_info, bytenr,
>> -                                            num_bytes, GFP_NOFS);
>> +            ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes,
>> +                                            GFP_NOFS);
>>              if (ret)
>>                      return ret;
>>      }
>> @@ -1796,8 +1796,7 @@ int btrfs_qgroup_trace_subtree(struct 
>> btrfs_trans_handle *trans,
>>                      btrfs_set_lock_blocking_rw(eb, BTRFS_READ_LOCK);
>>                      path->locks[level] = BTRFS_READ_LOCK_BLOCKING;
>>  
>> -                    ret = btrfs_qgroup_trace_extent(trans, fs_info,
>> -                                                    child_bytenr,
>> +                    ret = btrfs_qgroup_trace_extent(trans, child_bytenr,
>>                                                      fs_info->nodesize,
>>                                                      GFP_NOFS);
>>                      if (ret)
>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
>> index 385367989ed6..0215dc0b1710 100644
>> --- a/fs/btrfs/qgroup.h
>> +++ b/fs/btrfs/qgroup.h
>> @@ -212,9 +212,8 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_fs_info 
>> *fs_info,
>>   * Return <0 for error, like memory allocation failure or invalid parameter
>>   * (NULL trans)
>>   */
>> -int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans,
>> -            struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes,
>> -            gfp_t gfp_flag);
>> +int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
>> +                          u64 num_bytes, gfp_t gfp_flag);
>>  
>>  /*
>>   * Inform qgroup to trace all leaf items of data
>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>> index 7b7498f1f641..10f6a4223897 100644
>> --- a/fs/btrfs/tree-log.c
>> +++ b/fs/btrfs/tree-log.c
>> @@ -685,7 +685,7 @@ static noinline int replay_one_extent(struct 
>> btrfs_trans_handle *trans,
>>               * as the owner of the file extent changed from log tree
>>               * (doesn't affect qgroup) to fs/file tree(affects qgroup)
>>               */
>> -            ret = btrfs_qgroup_trace_extent(trans, fs_info,
>> +            ret = btrfs_qgroup_trace_extent(trans,
>>                              btrfs_file_extent_disk_bytenr(eb, item),
>>                              btrfs_file_extent_disk_num_bytes(eb, item),
>>                              GFP_NOFS);
>> 
>


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to