In message: Re: [PATCH] irqchip/gic-v3: change gic_ipi_track::lock type on 22/07/2024 Hao, Ruiqiang wrote:
> Bruce Ashfield > 2024-07-12 03:00 > I'd still like more explanation on this change and if it was tested in a > heavily loaded system (and for how long). Looking at the spinlock change and > the comments immediately below the patch, we have a write memory barrier AND > a > mention of ipis. When a rawlock is taken, we need to understand what happens > between the take and release. I'm seeing a lot of retry logic / looping, etc. > If that lock is held for too long or there's some way that the current thread > might be suspended, we could easily end up with high latency or a system > deadlock. > > The frequency of IPI loss is low. After half an hour of stress-ng testing, I > captured > only one instance of IPI retransmission. > This mechanism detects and resends IPIs, and despite the looping code, it does > not cause significant latency or deadlock. Sounds good. Thanks for the extra testing, this is something worth including in any -rt patch that changes locking. It is also worth mentioning that you've analyzed the code between the lock/unlock and can confirm that no blocking calls are being performed. I just realized that I don't see what branch is this for .. What is the target kernel/branch for the change ? Bruce > > Thanks, > Ruiqiang > ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ > From: Bruce Ashfield <[email protected]> > Sent: Friday, July 12, 2024 03:00 > To: Hao, Ruiqiang <[email protected]> > Cc: [email protected] <[email protected]> > Subject: Re: [PATCH] irqchip/gic-v3: change gic_ipi_track::lock type > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and > know > the content is safe. > > In message: [PATCH] irqchip/gic-v3: change gic_ipi_track::lock type > on 10/07/2024 Ruiqiang Hao wrote: > > > From: Ruiqiang Hao <[email protected]> > > > > When start with preempt-rt kernel, the following problem occurs. > > Change the type of gic_ipi_track::lock from spinlock_t to raw_spinlock_t > > to fix this. > > > > BUG: sleeping function called from invalid context at kernel/locking/ > spinlock_rt.c:46 > > in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/15 > > preempt_count: 6, expected: 0 > > RCU nest depth: 0, expected: 0 > > Preemption disabled at: > > [<ffff800008246370>] __create_object+0x110/0x380 > > CPU: 15 PID: 0 Comm: swapper/15 Tainted: G W > 6.1.90-rt18-yocto-preempt-rt #1 > > Hardware name: Marvell OcteonTX CN96XX board (DT) > > Call trace: > > dump_backtrace.part.0+0xe8/0xf4 > > show_stack+0x20/0x30 > > dump_stack_lvl+0x64/0x80 > > dump_stack+0x18/0x34 > > __might_resched+0x160/0x1c0 > > rt_spin_lock+0x38/0xd0 > > gic_write_sgi1r_retry+0x58/0x120 > > gic_ipi_send_mask+0x148/0x184 > > __ipi_send_mask+0x34/0x11c > > smp_cross_call+0x4c/0x10c > > arch_irq_work_raise+0x3c/0x4c > > __irq_work_queue_local+0x9c/0xb0 > > irq_work_queue+0x48/0x80 > > sugov_update_shared+0x258/0x260 > > enqueue_top_rt_rq+0x7c/0x124 > > enqueue_task_rt+0x1cc/0x2ec > > ttwu_do_activate+0x84/0x170 > > try_to_wake_up+0x21c/0x5c0 > > wake_up_process+0x20/0x30 > > irq_exit_rcu+0x134/0x140 > > el1_interrupt+0x38/0x70 > > el1h_64_irq_handler+0x18/0x2c > > el1h_64_irq+0x64/0x68 > > arch_cpu_idle+0x18/0x2c > > default_idle_call+0x40/0x1d0 > > do_idle+0x230/0x2a0 > > cpu_startup_entry+0x3c/0x4c > > secondary_start_kernel+0x120/0x14c > > __secondary_switched+0xb0/0xb4 > > > > Signed-off-by: Ruiqiang Hao <[email protected]> > > --- > > drivers/irqchip/irq-gic-v3-fixes.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/irqchip/irq-gic-v3-fixes.c b/drivers/irqchip/ > irq-gic-v3-fixes.c > > index 7d88e3295686..62d41cb4e9a4 100644 > > --- a/drivers/irqchip/irq-gic-v3-fixes.c > > +++ b/drivers/irqchip/irq-gic-v3-fixes.c > > @@ -31,7 +31,7 @@ enum ipi_msg_type { > > struct gic_ipi_track { > > atomic_t tx_count; > > atomic_t rx_count; > > - spinlock_t lock; > > + raw_spinlock_t lock; > > }; > > > > static struct gic_ipi_track gic_ipitrack[NR_CPUS][NR_IPIS]; > > @@ -79,7 +79,7 @@ void gic_write_sgi1r_retry(int dest_cpu, int irq, u64 val) > > { > > unsigned long flags; > > > > - spin_lock_irqsave(&gic_ipitrack[dest_cpu][irq].lock, flags); > > + raw_spin_lock_irqsave(&gic_ipitrack[dest_cpu][irq].lock, flags); > > Because this is only destined for a BSP branch, I'm not overly > concerned about the changes. > > BUT > > I'd still like more explanation on this change and if it was > tested in a heavily loaded system (and for how long). > > Looking at the spinlock change and the comments immediately > below the patch, we have a write memory barrier AND a mention > of ipis. > > When a rawlock is taken, we need to understand what happens > between the take and release. I'm seeing a lot of retry > logic / looping, etc. > > If that lock is held for too long or there's some way that > the current thread might be suspended, we could easily > end up with high latency or a system deadlock. > > Bruce > > > > wmb(); /* Ensure lock is acquired before we generate an IPI */ > > retry: > > gic_write_sgi1r(val); > > @@ -95,7 +95,7 @@ void gic_write_sgi1r_retry(int dest_cpu, int irq, u64 val) > > wmb(); /* Ensure the write is completed before we start again */ > > goto retry; > > out: > > - spin_unlock_irqrestore(&gic_ipitrack[dest_cpu][irq].lock, flags); > > + raw_spin_unlock_irqrestore(&gic_ipitrack[dest_cpu][irq].lock, flags); > > } > > > > static bool __maybe_unused gicv3_enable_quirk_otx(void *data) > > @@ -107,7 +107,7 @@ static bool __maybe_unused gicv3_enable_quirk_otx(void > *data) > > /* Initialize necessary lock */ > > for_each_possible_cpu(cpu) > > for (ipi = 0; ipi < NR_IPIS; ipi++) > > - spin_lock_init(&gic_ipitrack[cpu][ipi].lock); > > + raw_spin_lock_init(&gic_ipitrack[cpu][ipi].lock); > > > > return true; > > } > > -- > > 2.45.0 > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#14178): https://lists.yoctoproject.org/g/linux-yocto/message/14178 Mute This Topic: https://lists.yoctoproject.org/mt/107139152/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
