Re: [PATCH] dm-integrity: don't attempt to commit journal on disabled device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dm-integrity: missed wakeup of endio_wait in
  integrity_commit() on error
--

commit 6f78b56af54c9b05dda8dbe4a7aa354299dc4892
Author: Mikulas Patocka <mpatocka@redhat.com>

dm-integrity: don't attempt to commit journal on disabled device

This commit skips committing the journal in integrity_commit() when
dm_integrity_failed() is true, jumping directly to release_flush_bios.

> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 802b377613b06..c50fcf2212365 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -2908,6 +2908,10 @@ static void integrity_commit(struct work_struct *w)
>  
>  	spin_lock_irq(&ic->endio_wait.lock);
>  	flushes = bio_list_get(&ic->flush_bio_list);
> +	if (unlikely(dm_integrity_failed(ic))) {
> +		spin_unlock_irq(&ic->endio_wait.lock);
> +		goto release_flush_bios;
> +	}

[Severity: Medium]
Can this leave tasks sleeping on ic->endio_wait hung forever?

In journal mode, when ic->free_sectors reaches 0, a write bio offloaded to
ic->wait_wq calls sleep_on_endio_wait() in dm_integrity_map_continue() and
waits on ic->endio_wait before looping back to retry: and rechecking
dm_integrity_failed():

drivers/md/dm-integrity.c:dm_integrity_map_continue() {
	...
			if (unlikely(!dio->range.n_sectors)) {
				if (from_map)
					goto offload_to_thread;
				sleep_on_endio_wait(ic);
				goto retry;
			}
	...
}

Previously, even after dm_integrity_failed() became non-zero,
integrity_commit() still advanced uncommitted sections to
ic->n_committed_sections and queued ic->writer_work, allowing
integrity_writer() to replenish ic->free_sectors and call
wake_up_locked(&ic->endio_wait).

Jumping directly to release_flush_bios without waking ic->endio_wait or
advancing uncommitted sections means integrity_writer() is never scheduled
if an I/O or checksum error occurs while the journal is full.  The worker
in sleep_on_endio_wait() then blocks indefinitely on ic->wait_wq, stalling
subsequent bios as well as device suspend or teardown.

Calling wake_up_locked(&ic->endio_wait) before unlocking
ic->endio_wait.lock would let any sleeping writer wake up and fail the bio
at retry:.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/fe63f092-ef07-ac3f-7878-22083541f79e@redhat.com?part=1