* [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B
@ 2024-09-11 17:14 kernel test robot
2024-09-12 8:19 ` Luis Henriques
0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2024-09-11 17:14 UTC (permalink / raw)
To: Luis Henriques (SUSE)
Cc: oe-kbuild-all, Linux Memory Management List, Theodore Ts'o,
Jan Kara
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@intel.com/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@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409120149.GdjqoVYQ-lkp@intel.com/
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;
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B
2024-09-11 17:14 [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B kernel test robot
@ 2024-09-12 8:19 ` Luis Henriques
2024-09-12 9:46 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Luis Henriques @ 2024-09-12 8:19 UTC (permalink / raw)
To: kernel test robot
Cc: oe-kbuild-all, Linux Memory Management List, Theodore Ts'o,
Jan Kara
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@intel.com/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@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409120149.GdjqoVYQ-lkp@intel.com/
>
> 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?
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B
2024-09-12 8:19 ` Luis Henriques
@ 2024-09-12 9:46 ` Jan Kara
2024-09-12 10:08 ` Luis Henriques
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2024-09-12 9:46 UTC (permalink / raw)
To: Luis Henriques
Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
Theodore Ts'o, Jan Kara
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@intel.com/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@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202409120149.GdjqoVYQ-lkp@intel.com/
> >
> > 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@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B
2024-09-12 9:46 ` Jan Kara
@ 2024-09-12 10:08 ` Luis Henriques
0 siblings, 0 replies; 4+ messages in thread
From: Luis Henriques @ 2024-09-12 10:08 UTC (permalink / raw)
To: Jan Kara
Cc: kernel test robot, oe-kbuild-all, Linux Memory Management List,
Theodore Ts'o
On Thu, Sep 12 2024, Jan Kara wrote:
> 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@intel.com/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@intel.com>
>> > | Closes: https://lore.kernel.org/oe-kbuild-all/202409120149.GdjqoVYQ-lkp@intel.com/
>> >
>> > 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.
Oh! I totally missed that patch on the mailing-list. OK, so no action for
me then. Thanks!
> [1] https://lore.kernel.org/all/20240912090722.4e7o4l462y6hccau@quack3
(Minor nit: looks like this patch has a 'Closes:' tag pointing to a
bugzilla link which isn't accessible at the moment. But maybe it's just a
temporary '502'.)
Cheers,
--
Luís
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-12 10:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-11 17:14 [linux-next:master 5690/11210] fs/ext4/fast_commit.c:362:21-23: WARNING !A || A && B is equivalent to !A || B kernel test robot
2024-09-12 8:19 ` Luis Henriques
2024-09-12 9:46 ` Jan Kara
2024-09-12 10:08 ` Luis Henriques
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).