On Wed, Jul 27 2022 at 3:53P -0400, Nathan Huckleberry <nhuck@xxxxxxxxxx> wrote: > On Wed, Jul 27, 2022 at 8:48 AM Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > > > On Wed, Jul 27 2022 at 11:25P -0400, > > Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote: > > > > > Hi > > > > > > I'd like to ask - why not use dm_bufio_trylock from an interrupt context? > > > > > > I would just add a new function "dm_bufio_get_trylock" that is equivalent > > > to "dm_bufio_get", except that it uses dm_bufio_trylock - and if it fails, > > > fallback to process context. > > > > > > I think using dm_bufio_trylock would be less hacky than introducing a > > > new dm_bufio flag that changes mutex to spinlock. > > > > OK, I can drop the bufio hacks (patches 1 and 2) and replace with a > > dm_bufio_get_trylock and see if that resolves the cryptsetup testing > > issues Milan is reporting. But on the flip side I feel like trylock > > is far more prone to fail for at least one of a series of cached > > buffers needed (via _get). And so it'll punt to workqueue more often > > and _not_ provide the desired performance improvement. BUT.. we shall > > see. > > > > All said, I'm now dropping this patchset from the upcoming 5.20 merge. > > This all clearly needs more development time. > > > > Huck: I'll run with Mikulas's suggestion and try to get the cryptsetup > > tests passing. But I'll leave performance testing to you. > > > > Sounds like a reasonable fix. I expect that dm_bufio_get_trylock with > WQ_HIGHPRI will give similar performance gains. Hi, Just wanted to share where I am with this line of work: 1) I tried to use a semaphore instead of spinlock in bufio along with adding a dm_bufio_get_nowait that used dm_bufio_trylock. Turns out that wasn't valid because dm_bufio_release used down() and schedule, which isn't valid in tasklet. Mikulas and I discussed this situation and he proposed one way forward to still use semaphore but it'd require new dm-verity code that prepared a cookie (struct) pointer and issued a callback then drop the lock (so it'd avoid excessive locking). But I abandoned that due to the uncharted territory it was bringing us down. 2) Using Milan's cryptsetup branch and test procedure documented here: https://listman.redhat.com/archives/dm-devel/2022-July/051666.html I got the first "./verity-compat-test" to work with this branch: https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=dm-5.21 But then when I took the next step to always --use-tasklets I got hangs in the verity workqueue. I'm not sure how you got your code working, I cannot see any material difference between my dm-5.21 branch and your original patchset. If you'd like to have a look at the dm-5.21 branch to see if you can make it work that'd be appreciated. But I cannot put more time to this verity tasklet effort any more this week. Thanks, Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
