[Devel] [PATCH rh7] mutex: Disable irqs in spin_[un]lock_mutex()

Konstantin Khorenko khorenko at virtuozzo.com
Fri May 13 15:45:52 MSK 2022


We've got an NMI and it turned out that in kaio fastpath code
we do an unforgivable thing: we call mutex_trylock() in an IRQ context,
consequently we get a punishment, deadlock:

CPU 0 (kworker)                                 CPU 1 (ploop_fsync)
===============                                 ===================
ext4_io_submit [ext4]                           aio_run_iocb
 submit_bio                                      ext4_file_write_iter [ext4]
  generic_make_request                          ! mutex_unlock(&inode->i_mutex);
   ploop_make_request [ploop]                      __mutex_unlock_slowpath
!   spin_lock_irq(&plo->lock);                  // grabbed i_mutex->wait_lock, but no ret
    kaio_fastmap [pio_kaio]                        IRQ -> SOFTIRQ
     ext4_fastmap [ext4]                           ...
      mutex_unlock(&inode->i_mutex);               scsi_end_request
       __mutex_unlock_slowpath                      blk_update_request
!       spin_lock_mutex(&i_mutex->wait_lock)         bio_endio
                                                      ploop_fast_end_io [ploop]
                                                 !     spin_lock_irqsave(&plo->lock);

====================================================================
static __always_inline void queued_spin_lock(struct qspinlock *lock)
{
        u32 val;

        val = atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL);
// IRQ happened right here on CPU 1, val == 0
        if (likely(val == 0))
                return;
        queued_spin_lock_slowpath(lock, val);
}
====================================================================

Unfortunately this cannot be easily avoided (and following the rules):
fastpath is a hack and as we work with inode, we must hold its mutex,
and ploop_make_request() itself may be called in an IRQ context.

A workaround for this hack may be locking IRQ in
spin_[un]lock_mutex(mutex->wait_lock).
It's safe because in case CONFIG_DEBUG_MUTEXES spin_[un]lock_mutex()
do disable/enable irqs.

https://jira.sw.ru/browse/PSBM-139963
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 kernel/mutex.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/mutex.h b/kernel/mutex.h
index 4115fbf83b12..779af03f0ccf 100644
--- a/kernel/mutex.h
+++ b/kernel/mutex.h
@@ -10,9 +10,9 @@
  */
 
 #define spin_lock_mutex(lock, flags) \
-		do { spin_lock(lock); (void)(flags); } while (0)
+		do { spin_lock_irqsave(lock, flags); } while (0)
 #define spin_unlock_mutex(lock, flags) \
-		do { spin_unlock(lock); (void)(flags); } while (0)
+		do { spin_unlock_irqrestore(lock, flags); } while (0)
 #define mutex_remove_waiter(lock, waiter, ti) \
 		__list_del((waiter)->list.prev, (waiter)->list.next)
 
-- 
2.24.3



More information about the Devel mailing list