Re: [PATCH] md/raid5-ppl: Fix invalid context sleep in ppl_io_unit_finished() on PREEMPT_RT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

在 2025/8/18 19:54, Yunseong Kim 写道:
Hi Yu,

On 8/18/25 9:56 AM, Yu Kuai wrote:
Hi,

在 2025/08/17 19:31, Yunseong Kim 写道:
The function ppl_io_unit_finished() uses a local_irq_save()/spin_lock()
sequence. On a PREEMPT_RT enabled kernel, spin_lock() can sleep. Calling it
with interrupts disabled creates an atomic context where sleeping is
forbidden.

What? I believe spin_lock can never sleep.
I think you might have been a bit surprised by me sending a patch out of
the blue. It would be helpful to refer to the references below:

  On PREEMPT_RT kernels, these lock types are converted to sleeping locks:
   local_lock
   spinlock_t
   rwlock_t

Link: https://docs.kernel.org/locking/locktypes.html#sleeping-locks

Ensuring that the interrupt state is managed atomically with the lock
itself. The change is applied to both the 'log->io_list_lock' and
'ppl_conf->no_mem_stripes_lock' critical sections within the function.

Signed-off-by: Yunseong Kim <ysk@xxxxxxxxxxx>
---
   drivers/md/raid5-ppl.c | 12 ++++--------
   1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 56b234683ee6..650bd59ead72 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -553,15 +553,13 @@ static void ppl_io_unit_finished(struct ppl_io_unit *io)
         pr_debug("%s: seq: %llu\n", __func__, io->seq);
   -    local_irq_save(flags);
-
-    spin_lock(&log->io_list_lock);
+    spin_lock_irqsave(&log->io_list_lock, flags);
  The changes in spinlock_t and rwlock_t semantics on PREEMPT_RT kernels
  have a few implications. For example, on a non-PREEMPT_RT kernel the
  following code sequence works as expected:

  local_irq_disable();
  spin_lock(&lock);
and is fully equivalent to: spin_lock_irq(&lock); Same applies to rwlock_t and the _irqsave() suffix variants.

Link: https://docs.kernel.org/locking/locktypes.html#spinlock-t-and-rwlock-t

Yes, lessons are learned. Perhaps add a link tag in the commit message
just in case someone else will be confused?

Thanks,
Kuai


       list_del(&io->log_sibling);
-    spin_unlock(&log->io_list_lock);
+    spin_unlock_irqrestore(&log->io_list_lock, flags);
         mempool_free(io, &ppl_conf->io_pool);
   -    spin_lock(&ppl_conf->no_mem_stripes_lock);
+    spin_lock_irqsave(&ppl_conf->no_mem_stripes_lock, flags);
Please notice, local_irq_save + spin_lock is the same as
spin_lock_irqsave, I don't think your changes have any functonal
chagnes.
This issue has also been a problem in other subsystems, such as USB:

[BUG] usb: gadget: dummy_hcd: Sleeping function called from invalid
context in dummy_dequeue on PREEMPT_RT

Link: https://lore.kernel.org/lkml/20250816065933.EPwBJ0Sd@xxxxxxxxxxxxx/t/#u

I am currently contributing to the kernel to address the areas that need to
adapt to this paradigm shift so that the PREEMPT_RT Linux kernel can be
well supported. I have CC’d other RT people so they can also review
this part.

Thanks,
Kuai
Thank you!

Yunseong





[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux