Re: [PATCH V9 14/19] xfs: Introduce per-inode 64-bit extent counters

[Chandan Babu R <chandan.babu@oracle.com>]

On 07 Apr 2022 at 06:37, Dave Chinner wrote:
> On Thu, Apr 07, 2022 at 03:03:32AM +0800, kernel test robot wrote:
>> Hi Chandan,
>> 
>> Thank you for the patch! Perhaps something to improve:
>> 
>> [auto build test WARNING on xfs-linux/for-next]
>> [also build test WARNING on v5.18-rc1 next-20220406]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>> 
>> url:    https://github.com/intel-lab-lkp/linux/commits/Chandan-Babu-R/xfs-Extend-per-inode-extent-counters/20220406-174647
>> base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
>> config: i386-randconfig-s002 (https://download.01.org/0day-ci/archive/20220407/202204070218.QyD2PQPx-lkp@intel.com/config)
>> compiler: gcc-11 (Debian 11.2.0-19) 11.2.0
>> reproduce:
>>         # apt-get install sparse
>>         # sparse version: v0.6.4-dirty
>>         # https://github.com/intel-lab-lkp/linux/commit/28be4fd3f13d4ba2bcedceb8951cd3bfe852cba2
>>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>>         git fetch --no-tags linux-review Chandan-Babu-R/xfs-Extend-per-inode-extent-counters/20220406-174647
>>         git checkout 28be4fd3f13d4ba2bcedceb8951cd3bfe852cba2
>>         # save the config file to linux build tree
>>         mkdir build_dir
>>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash fs/xfs/
>> 
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>> 
>> 
>> sparse warnings: (new ones prefixed by >>)
>> >> fs/xfs/xfs_inode_item_recover.c:209:31: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be64 [usertype] di_v3_pad @@     got unsigned long long [usertype] di_v3_pad @@
>>    fs/xfs/xfs_inode_item_recover.c:209:31: sparse:     expected restricted __be64 [usertype] di_v3_pad
>>    fs/xfs/xfs_inode_item_recover.c:209:31: sparse:     got unsigned long long [usertype] di_v3_pad
>> 
>> vim +209 fs/xfs/xfs_inode_item_recover.c
>> 
>>    167	
>>    168	STATIC void
>>    169	xfs_log_dinode_to_disk(
>>    170		struct xfs_log_dinode	*from,
>>    171		struct xfs_dinode	*to,
>>    172		xfs_lsn_t		lsn)
>>    173	{
>>    174		to->di_magic = cpu_to_be16(from->di_magic);
>>    175		to->di_mode = cpu_to_be16(from->di_mode);
>>    176		to->di_version = from->di_version;
>>    177		to->di_format = from->di_format;
>>    178		to->di_onlink = 0;
>>    179		to->di_uid = cpu_to_be32(from->di_uid);
>>    180		to->di_gid = cpu_to_be32(from->di_gid);
>>    181		to->di_nlink = cpu_to_be32(from->di_nlink);
>>    182		to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
>>    183		to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
>>    184	
>>    185		to->di_atime = xfs_log_dinode_to_disk_ts(from, from->di_atime);
>>    186		to->di_mtime = xfs_log_dinode_to_disk_ts(from, from->di_mtime);
>>    187		to->di_ctime = xfs_log_dinode_to_disk_ts(from, from->di_ctime);
>>    188	
>>    189		to->di_size = cpu_to_be64(from->di_size);
>>    190		to->di_nblocks = cpu_to_be64(from->di_nblocks);
>>    191		to->di_extsize = cpu_to_be32(from->di_extsize);
>>    192		to->di_forkoff = from->di_forkoff;
>>    193		to->di_aformat = from->di_aformat;
>>    194		to->di_dmevmask = cpu_to_be32(from->di_dmevmask);
>>    195		to->di_dmstate = cpu_to_be16(from->di_dmstate);
>>    196		to->di_flags = cpu_to_be16(from->di_flags);
>>    197		to->di_gen = cpu_to_be32(from->di_gen);
>>    198	
>>    199		if (from->di_version == 3) {
>>    200			to->di_changecount = cpu_to_be64(from->di_changecount);
>>    201			to->di_crtime = xfs_log_dinode_to_disk_ts(from,
>>    202								  from->di_crtime);
>>    203			to->di_flags2 = cpu_to_be64(from->di_flags2);
>>    204			to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
>>    205			to->di_ino = cpu_to_be64(from->di_ino);
>>    206			to->di_lsn = cpu_to_be64(lsn);
>>    207			memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
>>    208			uuid_copy(&to->di_uuid, &from->di_uuid);
>>  > 209			to->di_v3_pad = from->di_v3_pad;
>
> Why not just explicitly write zero to the di_v3_pad field?
>

Yes, We can do that since the call to xfs_log_dinode_to_disk_iext_counters()
will update the values of union members correctly for v3 inodes with large
extent count feature enabled.

>>    210		} else {
>>    211			to->di_flushiter = cpu_to_be16(from->di_flushiter);
>>    212			memcpy(to->di_v2_pad, from->di_v2_pad, sizeof(to->di_v2_pad));
>
> Same here?
>

This field too can be set to zeroes explicitly.

-- 
chandan