From: Mike Snitzer <email@example.com>
To: Nathan Huckleberry <firstname.lastname@example.org>
Cc: Eric Biggers <email@example.com>,
firstname.lastname@example.org, Mike Snitzer <email@example.com>,
Milan Broz <firstname.lastname@example.org>,
Sami Tolvanen <email@example.com>
Subject: Re: [dm-devel] [PATCH v2 0/6] dm verity: optionally use tasklets
Date: Wed, 3 Aug 2022 12:17:02 -0400 [thread overview]
Message-ID: <YuqffhXctdt9vM0i@redhat.com> (raw)
On Tue, Aug 02 2022 at 9:39P -0400,
Nathan Huckleberry <firstname.lastname@example.org> wrote:
> On Wed, Jul 27, 2022 at 1:23 AM Milan Broz <email@example.com> wrote:
> > On 27/07/2022 01:04, Mike Snitzer wrote:
> > > On Tue, Jul 26 2022 at 5:44P -0400,
> > > Milan Broz <firstname.lastname@example.org> wrote:
> > >
> > >> On 26/07/2022 18:09, Mike Snitzer wrote:
> > >>> Hi,
> > >>>
> > >>> Please see this updated patchset that reflects what I've staged for
> > >>> the 5.20 merge window, see:
> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.20
> > >>>
> > >>> My only outstanding question, from previous v1 patchset, is: should
> > >>> the verify_wq be created using WQ_HIGHPRI instead of WQ_CPU_INTENSIVE?
> > >>> (I doubt it has a significant impact on performance but if you have
> > >>> insight on why you made that change, and if it meaningful, I'd happily
> > >>> apply the change).
> > >>>
> > >>> I've tested using cryptsetup's testsuite (which has dm-verity tests)
> > >>> but I haven't tested the "try_verify_in_tasklet" feature.
> > >
> > > I wasn't lying above: I haven't tested the "try_verify_in_tasklet"
> > > feature. I just figured I didn't break what Huck posted by cleaning
> > > it up while reviewing closely ;)
> > :) What I am trying to avoid is that these patches reach Linux tree
> > until properly tested even in non-standard configurations (like FEC enabled).
> > Currently we have not even enough HW for GitLab runners for upstream cryptsetup
> > CI testing and regression like these will cause *huge* pain for us later.
> > >> I added new veritysetup option --use-tasklets for testing to a new branch
> > >> https://gitlab.com/cryptsetup/cryptsetup/-/commits/verify-tasklet
> > >>
> > >> I tried to run verity-compat-test (on that branch above), not used the flag yet,
> > >> just in one simple option flag test (see the branch).
> > >
> > > OK, thanks for doing this work, really appreciate it. How is it I
> > > would initiate this test using your "verify-tasklet" branch?
> > Basically, just checkout that branch, compile it
> > (autogen.sh, configure, make, make check-programs) and then run verity test
> > cd tests; ./verity-compat-test
> > Even without adding the feature, FEC tests are skipped for some reason...
> > (Check for N/A that should not be there.)
> > Then you can easily enable "--use-tasklets" for every open, I would just
> > comment this line:
> > --- a/src/veritysetup.c
> > +++ b/src/veritysetup.c
> > @@ -184,7 +184,7 @@ static int _activate(const char *dm_device,
> > activate_flags |= CRYPT_ACTIVATE_IGNORE_ZERO_BLOCKS;
> > if (ARG_SET(OPT_CHECK_AT_MOST_ONCE_ID))
> > activate_flags |= CRYPT_ACTIVATE_CHECK_AT_MOST_ONCE;
> > - if (ARG_SET(OPT_USE_TASKLETS_ID))
> > +// if (ARG_SET(OPT_USE_TASKLETS_ID))
> > activate_flags |= CRYPT_ACTIVATE_TASKLETS;
> > compile it, and run the verity-compat-test again.
> > For me, it stucks on the first in-kernel verify (non-FEC) line.
> > Some trace below...
> I was able to fix this. There was a problem with falling back to a
> work-queue after FEC fails. This caused an infinite loop. I have
> dm-verity passing verity-compat-test with --use-tasklets; I'll send a
> v3 soon.
Great news. If your fix is confined to a small incremental fix that
needs folding into patch 3 of the v2 patchset: please just send the
dm-devel mailing list
next prev parent reply other threads:[~2022-08-03 16:17 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 16:09 [dm-devel] [PATCH v2 0/6] dm verity: optionally use tasklets Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 1/6] dm bufio: Add flags argument to dm_bufio_client_create Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 2/6] dm bufio: Add DM_BUFIO_CLIENT_NO_SLEEP flag Mike Snitzer
2022-07-27 15:25 ` Mikulas Patocka
2022-07-27 15:47 ` Mike Snitzer
2022-07-27 19:53 ` Nathan Huckleberry
2022-07-28 22:37 ` Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 3/6] dm verity: Add optional "try_verify_in_tasklet" feature Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 4/6] dm verity: allow optional args to alter primary args handling Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 5/6] dm bufio: conditionally enable branching for DM_BUFIO_CLIENT_NO_SLEEP Mike Snitzer
2022-07-26 16:09 ` [dm-devel] [PATCH v2 6/6] dm verity: conditionally enable branching for "try_verify_in_tasklet" Mike Snitzer
2022-07-26 20:18 ` [dm-devel] [PATCH v2 0/6] dm verity: optionally use tasklets Nathan Huckleberry
2022-07-26 21:44 ` Milan Broz
2022-07-26 23:04 ` Mike Snitzer
2022-07-27 8:23 ` Milan Broz
2022-08-03 1:39 ` Nathan Huckleberry
2022-08-03 16:17 ` Mike Snitzer [this message]
2022-08-03 18:29 ` [dm-devel] [PATCH] Fixes 6890e9b8c7d0a1062bbf4f854b6be3723836ad9a Nathan Huckleberry
2022-08-04 20:22 ` [dm-devel] " Mike Snitzer
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.