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. Thanks, Mike > On Tue, 26 Jul 2022, Mike Snitzer wrote: > > > From: Nathan Huckleberry <nhuck@xxxxxxxxxx> > > > > Add an optional flag that ensures dm_bufio_client does not sleep > > (primary focus is to service dm_bufio_get without sleeping). This > > allows the dm-bufio cache to be queried from interrupt context. > > > > To ensure that dm-bufio does not sleep, dm-bufio must use a spinlock > > instead of a mutex. Additionally, to avoid deadlocks, special care > > must be taken so that dm-bufio does not sleep while holding the > > spinlock. > > > > But again: the scope of this no_sleep is initially confined to > > dm_bufio_get, so __alloc_buffer_wait_no_callback is _not_ changed to > > avoid sleeping because __bufio_new avoids allocation for NF_GET. > > > > Signed-off-by: Nathan Huckleberry <nhuck@xxxxxxxxxx> > > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > --- > > drivers/md/dm-bufio.c | 22 +++++++++++++++++++--- > > include/linux/dm-bufio.h | 5 +++++ > > 2 files changed, 24 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c > > index ad5603eb12e3..486179d1831c 100644 > > --- a/drivers/md/dm-bufio.c > > +++ b/drivers/md/dm-bufio.c > > @@ -81,6 +81,8 @@ > > */ > > struct dm_bufio_client { > > struct mutex lock; > > + spinlock_t spinlock; > > + unsigned long spinlock_flags; > > > > struct list_head lru[LIST_SIZE]; > > unsigned long n_buffers[LIST_SIZE]; > > @@ -90,6 +92,7 @@ struct dm_bufio_client { > > s8 sectors_per_block_bits; > > void (*alloc_callback)(struct dm_buffer *); > > void (*write_callback)(struct dm_buffer *); > > + bool no_sleep; > > > > struct kmem_cache *slab_buffer; > > struct kmem_cache *slab_cache; > > @@ -167,17 +170,26 @@ struct dm_buffer { > > > > static void dm_bufio_lock(struct dm_bufio_client *c) > > { > > - mutex_lock_nested(&c->lock, dm_bufio_in_request()); > > + if (c->no_sleep) > > + spin_lock_irqsave_nested(&c->spinlock, c->spinlock_flags, dm_bufio_in_request()); > > + else > > + mutex_lock_nested(&c->lock, dm_bufio_in_request()); > > } > > > > static int dm_bufio_trylock(struct dm_bufio_client *c) > > { > > - return mutex_trylock(&c->lock); > > + if (c->no_sleep) > > + return spin_trylock_irqsave(&c->spinlock, c->spinlock_flags); > > + else > > + return mutex_trylock(&c->lock); > > } > > > > static void dm_bufio_unlock(struct dm_bufio_client *c) > > { > > - mutex_unlock(&c->lock); > > + if (c->no_sleep) > > + spin_unlock_irqrestore(&c->spinlock, c->spinlock_flags); > > + else > > + mutex_unlock(&c->lock); > > } > > > > /*----------------------------------------------------------------*/ > > @@ -1748,12 +1760,16 @@ struct dm_bufio_client *dm_bufio_client_create(struct block_device *bdev, unsign > > c->alloc_callback = alloc_callback; > > c->write_callback = write_callback; > > > > + if (flags & DM_BUFIO_CLIENT_NO_SLEEP) > > + c->no_sleep = true; > > + > > for (i = 0; i < LIST_SIZE; i++) { > > INIT_LIST_HEAD(&c->lru[i]); > > c->n_buffers[i] = 0; > > } > > > > mutex_init(&c->lock); > > + spin_lock_init(&c->spinlock); > > INIT_LIST_HEAD(&c->reserved_buffers); > > c->need_reserved_buffers = reserved_buffers; > > > > diff --git a/include/linux/dm-bufio.h b/include/linux/dm-bufio.h > > index e21480715255..15d9e15ca830 100644 > > --- a/include/linux/dm-bufio.h > > +++ b/include/linux/dm-bufio.h > > @@ -17,6 +17,11 @@ > > struct dm_bufio_client; > > struct dm_buffer; > > > > +/* > > + * Flags for dm_bufio_client_create > > + */ > > +#define DM_BUFIO_CLIENT_NO_SLEEP 0x1 > > + > > /* > > * Create a buffered IO cache on a given device > > */ > > -- > > 2.32.1 (Apple Git-133) > > > > -- > > dm-devel mailing list > > dm-devel@xxxxxxxxxx > > https://listman.redhat.com/mailman/listinfo/dm-devel > > > -- > dm-devel mailing list > dm-devel@xxxxxxxxxx > https://listman.redhat.com/mailman/listinfo/dm-devel > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel