On Tue, Aug 02 2022 at 9:39P -0400, Nathan Huckleberry <nhuck@xxxxxxxxxx> wrote: > On Wed, Jul 27, 2022 at 1:23 AM Milan Broz <gmazyland@xxxxxxxxx> wrote: > > > > On 27/07/2022 01:04, Mike Snitzer wrote: > > > On Tue, Jul 26 2022 at 5:44P -0400, > > > Milan Broz <gmazyland@xxxxxxxxx> 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. > > Thanks, > Huck 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 incremental patch. Thanks, Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
