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 <hofrat@xxxxxxxxx> -----
>
> Date: Fri, 14 Sep 2018 09:37:03 +0200
> From: Nicholas Mc Guire <hofrat@xxxxxxxxx>
> Subject: [PATCH] dm integrity: drop excess unlikely() from
> BUG_ON()
> To: Alasdair Kergon <agk@xxxxxxxxxx>
> Cc: dm-devel@xxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
> Mike Snitzer <snitzer@xxxxxxxxxx>,
> Nicholas Mc Guire <hofrat@xxxxxxxxx>
>
> 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 <hofrat@xxxxxxxxx>
> ---
>
> 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
> dm-devel@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/dm-devel
>
> ----- End forwarded message -----
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
