From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8CF09C6FD20 for ; Fri, 24 Mar 2023 17:02:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iOVnUeEDu7i2uBFciNrf18zaEb1Dwf9WJVLee0ibTeM=; b=i7pdHxq2Uvr1Gx DKOP0VLjWGf9EpvAC0eDUUbDaKeS/2il5oECqkbYf2ajBcKs5twJ2MjSMSPBBTxMdWqhsBamLqya6 j2curwlIriMMd1pXM6w8aHKdAvZRhhZnycfyBAbU6KsvikFw142+nEk7fmyCp/EAOli5o5kW494Ny GESScULsBrLWs4y2FL9iIku2/URY+YwR7v1TvBI0u2TWgXKoKnnkpoO2t1SPwJRyHt0kvyekBZ8S3 G+chfh98Wi4yPT8Ra/ceLG805LZTt/jItmOJFiTNhwWqWW6ihNu8dXJ4OJvSaTfZaoMCOlo+X1s+6 gxm9iAO3QBZwJfEy0RsA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pfknO-0052Zn-2M; Fri, 24 Mar 2023 17:01:42 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pfknL-0052Z3-0Q for linux-arm-kernel@lists.infradead.org; Fri, 24 Mar 2023 17:01:40 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 79816CE2713; Fri, 24 Mar 2023 17:01:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 297EEC433EF; Fri, 24 Mar 2023 17:01:31 +0000 (UTC) Date: Fri, 24 Mar 2023 17:01:28 +0000 From: Catalin Marinas To: "Bouska, Zdenek" Cc: Thomas Gleixner , Will Deacon , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "Kiszka, Jan" , "linux-rt-users@vger.kernel.org" , Nishanth Menon , Puranjay Mohan Subject: Re: Unfair qspinlocks on ARM64 without LSE atomics => 3ms delay in interrupt handling Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230324_100139_529491_DA0E1E8F X-CRM114-Status: GOOD ( 26.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 24, 2023 at 08:43:38AM +0000, Bouska, Zdenek wrote: > I have seen ~3 ms delay in interrupt handling on ARM64. > > I have traced it down to raw_spin_lock() call in handle_irq_event() in > kernel/irq/handle.c: > > irqreturn_t handle_irq_event(struct irq_desc *desc) > { > irqreturn_t ret; > > desc->istate &= ~IRQS_PENDING; > irqd_set(&desc->irq_data, IRQD_IRQ_INPROGRESS); > raw_spin_unlock(&desc->lock); > > ret = handle_irq_event_percpu(desc); > > --> raw_spin_lock(&desc->lock); > irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS); > return ret; > } > > It took ~3 ms for this raw_spin_lock() to lock. That's quite a large indeed. > During this time irq_finalize_oneshot() from kernel/irq/manage.c locks and > unlocks the same raw spin lock more than 1000 times: > > static void irq_finalize_oneshot(struct irq_desc *desc, > struct irqaction *action) > { > if (!(desc->istate & IRQS_ONESHOT) || > action->handler == irq_forced_secondary_handler) > return; > again: > chip_bus_lock(desc); > --> raw_spin_lock_irq(&desc->lock); > > /* > * Implausible though it may be we need to protect us against > * the following scenario: > * > * The thread is faster done than the hard interrupt handler > * on the other CPU. If we unmask the irq line then the > * interrupt can come in again and masks the line, leaves due > * to IRQS_INPROGRESS and the irq line is masked forever. > * > * This also serializes the state of shared oneshot handlers > * versus "desc->threads_oneshot |= action->thread_mask;" in > * irq_wake_thread(). See the comment there which explains the > * serialization. > */ > if (unlikely(irqd_irq_inprogress(&desc->irq_data))) { > --> raw_spin_unlock_irq(&desc->lock); > chip_bus_sync_unlock(desc); > cpu_relax(); > goto again; > } So this path is hammering the desc->lock location and another CPU cannot change it. As you found, the problem is not the spinlock algorithm but the atomic primitives. The LDXR/STXR constructs on arm64 are known to have this issue with STXR failing indefinitely. raw_spin_unlock() simply does an STLR and this clears the exclusive monitor that the other CPU may have set with LDXR but before the STXR. The queued spinlock only provides fairness if the CPU manages to get in the queue. > So I confirmed that atomic operations from > arch/arm64/include/asm/atomic_ll_sc.h can be quite slow when they are > contested from second CPU. > > Do you think that it is possible to create fair qspinlock implementation > on top of atomic instructions supported by ARM64 version 8 (no LSE atomic > instructions) without compromising performance in the uncontested case? > For example ARM64 could have custom queued_fetch_set_pending_acquire > implementation same as x86 has in arch/x86/include/asm/qspinlock.h. Is the > retry loop in irq_finalize_oneshot() ok together with the current ARM64 > cpu_relax() implementation for processor with no LSE atomic instructions? So is the queued_fetch_set_pending_acquire() where it gets stuck or the earlier atomic_try_cmpxchg_acquire() before entering on the slow path? I guess both can fail in a similar way. A longer cpu_relax() here would improve things (on arm64 this function is a no-op) but maybe Thomas or Will have a better idea. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel