On Thu 12-09-24 09:19:18, Luis Henriques wrote: > On Thu, Sep 12 2024, kernel test robot wrote: > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > head: 32ffa5373540a8d1c06619f52d019c6cdc948bb4 > > commit: ebc4b2c1ac92fc0f8bf3f5a9c285a871d5084a6b [5690/11210] ext4: fix incorrect tid assumption in ext4_fc_mark_ineligible() > > config: loongarch-randconfig-r063-20240911 (https://download.01.org/0day-ci/archive/20240912/202409120149.GdjqoVYQ-lkp@xxxxxxxxx/config) > > compiler: loongarch64-linux-gcc (GCC) 14.1.0 > > > > 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/202409120149.GdjqoVYQ-lkp@xxxxxxxxx/ > > > > cocci warnings: (new ones prefixed by >>) > >>> fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B > > > > vim +362 fs/ext4/fast_commit.c > > > > 332 > > 333 /* > > 334 * Mark file system as fast commit ineligible, and record latest > > 335 * ineligible transaction tid. This means until the recorded > > 336 * transaction, commit operation would result in a full jbd2 commit. > > 337 */ > > 338 void ext4_fc_mark_ineligible(struct super_block *sb, int reason, handle_t *handle) > > 339 { > > 340 struct ext4_sb_info *sbi = EXT4_SB(sb); > > 341 tid_t tid; > > 342 bool has_transaction = true; > > 343 bool is_ineligible; > > 344 > > 345 if (ext4_fc_disabled(sb)) > > 346 return; > > 347 > > 348 if (handle && !IS_ERR(handle)) > > 349 tid = handle->h_transaction->t_tid; > > 350 else { > > 351 read_lock(&sbi->s_journal->j_state_lock); > > 352 if (sbi->s_journal->j_running_transaction) > > 353 tid = sbi->s_journal->j_running_transaction->t_tid; > > 354 else > > 355 has_transaction = false; > > 356 read_unlock(&sbi->s_journal->j_state_lock); > > 357 } > > 358 spin_lock(&sbi->s_fc_lock); > > 359 is_ineligible = ext4_test_mount_flag(sb, EXT4_MF_FC_INELIGIBLE); > > 360 if (has_transaction && > > 361 (!is_ineligible || > > > 362 (is_ineligible && tid_gt(tid, sbi->s_fc_ineligible_tid)))) > > 363 sbi->s_fc_ineligible_tid = tid; > > This suggestion is obviously correct. However, my brain found it much > easier to write (and understand) this logic if written this way. > > Ted, want me to re-send this patch (or a fix for it), or are you happy > leaving it as is? I think I've already seen a patch for this. Yes [1]. Frankly I don't care much but I slightly prefer the shorter variant. [1] https://lore.kernel.org/all/20240912090722.4e7o4l462y6hccau@quack3 Honza > > Cheers, > -- > Luís > > > 364 ext4_set_mount_flag(sb, EXT4_MF_FC_INELIGIBLE); 365 > > spin_unlock(&sbi->s_fc_lock); 366 WARN_ON(reason >= > > EXT4_FC_REASON_MAX); 367 > > sbi->s_fc_stats.fc_ineligible_reason_count[reason]++; 368 } 369 > > > > -- > > 0-DAY CI Kernel Test Service > > https://github.com/intel/lkp-tests/wiki > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
