I think this patch is not needed. In the statement "BUG_ON(unlikely(journal_entry_is_inprogress(je)) && !from_replay);", the "unlikely" macro suggests that the first condition is likely to be false and therefore the compiler will move the test for the second condition to the cold section.
If you remove unlikely, the compiler will know that that one of the conditions will be false, but it won't know which one. Mikulas On Sat, 15 Sep 2018, Alasdair G Kergon wrote: > One for you to decide and respond - it's not *identical* of course... > > (I wonder if this was generated by a script rather than looking at what the > compiler actually does) > > ----- Forwarded message from Nicholas Mc Guire <[email protected]> ----- > > Date: Fri, 14 Sep 2018 09:37:03 +0200 > From: Nicholas Mc Guire <[email protected]> > Subject: [dm-devel] [PATCH] dm integrity: drop excess unlikely() from > BUG_ON() > To: Alasdair Kergon <[email protected]> > Cc: [email protected], [email protected], > Mike Snitzer <[email protected]>, > Nicholas Mc Guire <[email protected]> > > include/asm-generic/bug.h defines BUG_ON(condition) as > do { if (unlikely(condition)) BUG(); } while (0). > So BUG_ON already provides unlikely() on the condition thus > there is no point in having BUG_ON(unlikely(condition)), > thus simply drop the excess unlikely(). > > Signed-off-by: Nicholas Mc Guire <[email protected]> > --- > > Found during code review > > Patch was compile tested with: x86_64_defconfig + SCSI_LOWLEVEL=y, > DM_INTEGRITY=y > (with warnings from sparse and smatch - not related to the proposed > change though) > > Patch is against 4.19-rc3 (localversion-next is next-20180913) > > drivers/md/dm-integrity.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c > index 89ccb64..8574e73 100644 > --- a/drivers/md/dm-integrity.c > +++ b/drivers/md/dm-integrity.c > @@ -1948,7 +1948,8 @@ static void do_journal_write(struct dm_integrity_c *ic, > unsigned write_start, > > if (journal_entry_is_unused(je)) > continue; > - BUG_ON(unlikely(journal_entry_is_inprogress(je)) && > !from_replay); > + BUG_ON(journal_entry_is_inprogress(je) && > + !from_replay); > sec = journal_entry_get_sector(je); > if (unlikely(from_replay)) { > if (unlikely(sec & > (unsigned)(ic->sectors_per_block - 1))) { > @@ -1963,7 +1964,8 @@ static void do_journal_write(struct dm_integrity_c *ic, > unsigned write_start, > sector_t sec2, area2, offset2; > if (journal_entry_is_unused(je2)) > break; > - > BUG_ON(unlikely(journal_entry_is_inprogress(je2)) && !from_replay); > + BUG_ON(journal_entry_is_inprogress(je2) && > + !from_replay); > sec2 = journal_entry_get_sector(je2); > get_area_and_offset(ic, sec2, &area2, &offset2); > if (area2 != area || offset2 != offset + ((k - > j) << ic->sb->log2_sectors_per_block)) > -- > 2.1.4 > > -- > dm-devel mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/dm-devel > > ----- End forwarded message ----- > -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
