[dm-devel] Fixes 6890e9b8c7d0a1062bbf4f854b6be3723836ad9a
Mike Snitzer
snitzer at redhat.com
Thu Aug 4 20:22:22 UTC 2022
On Wed, Aug 03 2022 at 2:29P -0400,
Nathan Huckleberry <nhuck at google.com> wrote:
> The previous patch had a bug when hash verification failed in the
> tasklet. This happened because the tasklet has already advanced the
> bvec_iter when it falls back to the work-queue implementation. When the
> work-queue job begins running, it attempts to restart verifiying at
> block i, but the bvec_iter is at block i+1.
>
> This causes several failures, the most noticeable is that there are not
> enough bytes in the bvec_iter to finish verification. Since there are
> not enough bytes to finish, the work queue gets stuck in an infinite
> loop in verity_for_io_block.
>
> To fix this, we can make a local copy of the bvec_iter struct in
> verity_verify_io. This requires that we restart verification from the
> first block when deferring to a work-queue rather than starting from the
> last block verified in the tasklet.
>
> This patch also fixes what appears to be a memory leak when FEC is
> enabled. The call to verity_fec_init_io should only be made if we are
> in a work-queue since a tasklet does not use FEC and is unable to free
> the memory.
>
> Signed-off-by: Nathan Huckleberry <nhuck at google.com>
Thanks for the detailed header, helped me appreciate the details of
your incremental fixes. I folded your fixes in to the original commit
that adds the "try_verify_in_tasklet" feature. I also layered on some
verity_verify_io() optimizations in new commits. Lastly, I added the
use of WQ_HIGHPRI if "try_verify_in_tasklet" feature is used.
The end result passes Milan's testsuite with and without --use-tasklets.
I've staged the changes in linux-next, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next
There is a chance I'll send another 6.0 pull request to Linus with
these changes tomorrow. We've done quite a bit of work here (_before_
the 6.0 merge window opened) so I do think it'd be best to get these
changes upstream sooner rather than later.
If anyone disagrees with sending these changes upstream now please
speak-up!
Thanks,
Mike
More information about the dm-devel
mailing list