From b33b6fdca3f0380c101e2adae5644777d6530fe5 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Fri, 5 Aug 2022 22:58:33 -0400 Subject: [PATCH] dm bufio: simplify DM_BUFIO_CLIENT_NO_SLEEP locking Historically none of the bufio code runs in interrupt context but with the use of DM_BUFIO_CLIENT_NO_SLEEP a bufio client can, see: commit 5721d4e5a9cd ("dm verity: Add optional "try_verify_in_tasklet" feature") That said, the new tasklet usecase still doesn't require interrupts be disabled by bufio (let alone conditionally restore them). Yet with PREEMPT_RT, and falling back from tasklet to workqueue, care must be taken to properly synchronize between softirq and process context, otherwise ABBA deadlock may occur. While it is unnecessary to disable bottom-half preemption within a tasklet, we must consistently do so in process context to ensure locking is in the proper order. Fix these issues by switching from spin_lock_irq{save,restore} to using spin_{lock,unlock}_bh instead. Also remove the 'spinlock_flags' member in dm_bufio_client struct (that can be used unsafely if bufio must recurse on behalf of some caller, e.g. block layer's submit_bio). Fixes: 5721d4e5a9cd ("dm verity: Add optional "try_verify_in_tasklet" feature") Reported-by: Jens Axboe Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 0a38a7aef49c..bef8c6eb5bdb 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -83,7 +83,7 @@ struct dm_bufio_client { struct mutex lock; spinlock_t spinlock; - unsigned long spinlock_flags; + bool no_sleep; struct list_head lru[LIST_SIZE]; unsigned long n_buffers[LIST_SIZE]; @@ -93,8 +93,6 @@ 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; struct dm_io_client *dm_io; @@ -174,7 +172,7 @@ static DEFINE_STATIC_KEY_FALSE(no_sleep_enabled); static void dm_bufio_lock(struct dm_bufio_client *c) { if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) - spin_lock_irqsave_nested(&c->spinlock, c->spinlock_flags, dm_bufio_in_request()); + spin_lock_bh(&c->spinlock); else mutex_lock_nested(&c->lock, dm_bufio_in_request()); } @@ -182,7 +180,7 @@ static void dm_bufio_lock(struct dm_bufio_client *c) static int dm_bufio_trylock(struct dm_bufio_client *c) { if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) - return spin_trylock_irqsave(&c->spinlock, c->spinlock_flags); + return spin_trylock_bh(&c->spinlock); else return mutex_trylock(&c->lock); } @@ -190,7 +188,7 @@ static int dm_bufio_trylock(struct dm_bufio_client *c) static void dm_bufio_unlock(struct dm_bufio_client *c) { if (static_branch_unlikely(&no_sleep_enabled) && c->no_sleep) - spin_unlock_irqrestore(&c->spinlock, c->spinlock_flags); + spin_unlock_bh(&c->spinlock); else mutex_unlock(&c->lock); }