On 3/23/24 6:51 PM, Heming Zhao wrote: > Hello Joseph, > > Please see my comment, only one warning need to be fixed, others are harmless. > Do I re-send v4 patch or send a new patch? > To make sparse tool happy, we'd better fix all type conversion warnings and then send v4 version. Joseph > On 3/23/24 12:17, kernel test robot wrote: >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable >> head: e393dbc9246c3ae0160a07c8666838056eafbc44 >> commit: e393dbc9246c3ae0160a07c8666838056eafbc44 [38/38] ocfs2: improve write IO performance when fragmentation is high >> config: i386-randconfig-061-20240322 (https://download.01.org/0day-ci/archive/20240323/202403231258.mU2k0fih-lkp@xxxxxxxxx/config) >> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) >> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240323/202403231258.mU2k0fih-lkp@xxxxxxxxx/reproduce) >> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of >> the same patch/commit), kindly add following tags >> | Reported-by: kernel test robot <lkp@xxxxxxxxx> >> | Closes: https://lore.kernel.org/oe-kbuild-all/202403231258.mU2k0fih-lkp@xxxxxxxxx/ >> >> sparse warnings: (new ones prefixed by >>) >>>> fs/ocfs2/resize.c:127:46: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short [usertype] total_bits @@ got restricted __le16 [usertype] bg_bits @@ >> fs/ocfs2/resize.c:127:46: sparse: expected unsigned short [usertype] total_bits >> fs/ocfs2/resize.c:127:46: sparse: got restricted __le16 [usertype] bg_bits > > This warning need to be modified, there should do endian converting. > >>>> fs/ocfs2/resize.c:128:33: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [usertype] old_bg_contig_free_bits @@ got restricted __le16 [usertype] bg_contig_free_bits @@ >> fs/ocfs2/resize.c:128:33: sparse: expected unsigned short [usertype] old_bg_contig_free_bits >> fs/ocfs2/resize.c:128:33: sparse: got restricted __le16 [usertype] bg_contig_free_bits > > This warning is harmless, the old_bg_contig_free_bits does backup job. > later code logic just uses this value to do restore value job. So we > don't need to do endian converting job. > >>>> fs/ocfs2/resize.c:169:44: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] bg_contig_free_bits @@ got unsigned short [usertype] old_bg_contig_free_bits @@ >> fs/ocfs2/resize.c:169:44: sparse: expected restricted __le16 [usertype] bg_contig_free_bits >> fs/ocfs2/resize.c:169:44: sparse: got unsigned short [usertype] old_bg_contig_free_bits > > see my above comment. just use backup value. no need to convert endian. > >> fs/ocfs2/resize.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...): >> include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false >> include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false > > This warning belongs to mm part. >> -- >>>> fs/ocfs2/suballoc.c:2566:41: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [usertype] old_bg_contig_free_bits @@ got restricted __le16 [usertype] bg_contig_free_bits @@ >> fs/ocfs2/suballoc.c:2566:41: sparse: expected unsigned short [usertype] old_bg_contig_free_bits >> fs/ocfs2/suballoc.c:2566:41: sparse: got restricted __le16 [usertype] bg_contig_free_bits > > Like above, there code do backup job, no need to convert endian. >> fs/ocfs2/suballoc.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...): >> include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false >> include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false > There belongs to mm. > >> fs/ocfs2/suballoc.c:2493:17: sparse: sparse: context imbalance in 'ocfs2_block_group_clear_bits' - different lock contexts for basic block > > from [1], this warning is harmless. > > [1]: https://lwn.net/Articles/109066/ > > Thanks, > Heming >> >> vim +127 fs/ocfs2/resize.c >> >> 76 >> 77 static int ocfs2_update_last_group_and_inode(handle_t *handle, >> 78 struct inode *bm_inode, >> 79 struct buffer_head *bm_bh, >> 80 struct buffer_head *group_bh, >> 81 u32 first_new_cluster, >> 82 int new_clusters) >> 83 { >> 84 int ret = 0; >> 85 struct ocfs2_super *osb = OCFS2_SB(bm_inode->i_sb); >> 86 struct ocfs2_dinode *fe = (struct ocfs2_dinode *) bm_bh->b_data; >> 87 struct ocfs2_chain_list *cl = &fe->id2.i_chain; >> 88 struct ocfs2_chain_rec *cr; >> 89 struct ocfs2_group_desc *group; >> 90 u16 chain, num_bits, backups = 0; >> 91 u16 cl_bpc = le16_to_cpu(cl->cl_bpc); >> 92 u16 cl_cpg = le16_to_cpu(cl->cl_cpg); >> 93 u16 old_bg_clusters; >> 94 u16 contig_bits, old_bg_contig_free_bits; >> 95 >> 96 trace_ocfs2_update_last_group_and_inode(new_clusters, >> 97 first_new_cluster); >> 98 >> 99 ret = ocfs2_journal_access_gd(handle, INODE_CACHE(bm_inode), >> 100 group_bh, OCFS2_JOURNAL_ACCESS_WRITE); >> 101 if (ret < 0) { >> 102 mlog_errno(ret); >> 103 goto out; >> 104 } >> 105 >> 106 group = (struct ocfs2_group_desc *)group_bh->b_data; >> 107 >> 108 old_bg_clusters = le16_to_cpu(group->bg_bits) / cl_bpc; >> 109 /* update the group first. */ >> 110 num_bits = new_clusters * cl_bpc; >> 111 le16_add_cpu(&group->bg_bits, num_bits); >> 112 le16_add_cpu(&group->bg_free_bits_count, num_bits); >> 113 >> 114 /* >> 115 * check whether there are some new backup superblocks exist in >> 116 * this group and update the group bitmap accordingly. >> 117 */ >> 118 if (OCFS2_HAS_COMPAT_FEATURE(osb->sb, >> 119 OCFS2_FEATURE_COMPAT_BACKUP_SB)) { >> 120 backups = ocfs2_calc_new_backup_super(bm_inode, >> 121 group, >> 122 cl_cpg, old_bg_clusters, 1); >> 123 le16_add_cpu(&group->bg_free_bits_count, -1 * backups); >> 124 } >> 125 >> 126 contig_bits = ocfs2_find_max_contig_free_bits(group->bg_bitmap, >> > 127 group->bg_bits, 0); >> > 128 old_bg_contig_free_bits = group->bg_contig_free_bits; >> 129 group->bg_contig_free_bits = cpu_to_le16(contig_bits); >> 130 >> 131 ocfs2_journal_dirty(handle, group_bh); >> 132 >> 133 /* update the inode accordingly. */ >> 134 ret = ocfs2_journal_access_di(handle, INODE_CACHE(bm_inode), bm_bh, >> 135 OCFS2_JOURNAL_ACCESS_WRITE); >> 136 if (ret < 0) { >> 137 mlog_errno(ret); >> 138 goto out_rollback; >> 139 } >> 140 >> 141 chain = le16_to_cpu(group->bg_chain); >> 142 cr = (&cl->cl_recs[chain]); >> 143 le32_add_cpu(&cr->c_total, num_bits); >> 144 le32_add_cpu(&cr->c_free, num_bits); >> 145 le32_add_cpu(&fe->id1.bitmap1.i_total, num_bits); >> 146 le32_add_cpu(&fe->i_clusters, new_clusters); >> 147 >> 148 if (backups) { >> 149 le32_add_cpu(&cr->c_free, -1 * backups); >> 150 le32_add_cpu(&fe->id1.bitmap1.i_used, backups); >> 151 } >> 152 >> 153 spin_lock(&OCFS2_I(bm_inode)->ip_lock); >> 154 OCFS2_I(bm_inode)->ip_clusters = le32_to_cpu(fe->i_clusters); >> 155 le64_add_cpu(&fe->i_size, (u64)new_clusters << osb->s_clustersize_bits); >> 156 spin_unlock(&OCFS2_I(bm_inode)->ip_lock); >> 157 i_size_write(bm_inode, le64_to_cpu(fe->i_size)); >> 158 >> 159 ocfs2_journal_dirty(handle, bm_bh); >> 160 >> 161 out_rollback: >> 162 if (ret < 0) { >> 163 ocfs2_calc_new_backup_super(bm_inode, >> 164 group, >> 165 cl_cpg, old_bg_clusters, 0); >> 166 le16_add_cpu(&group->bg_free_bits_count, backups); >> 167 le16_add_cpu(&group->bg_bits, -1 * num_bits); >> 168 le16_add_cpu(&group->bg_free_bits_count, -1 * num_bits); >> > 169 group->bg_contig_free_bits = old_bg_contig_free_bits; >> 170 } >> 171 out: >> 172 if (ret) >> 173 mlog_errno(ret); >> 174 return ret; >> 175 } >> 176 >>
