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 06C46C6FD1C for ; Fri, 24 Mar 2023 18:10:06 +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=jUAfZ5fpnXJNawB49p+ExLP8mydETINm79gzZmKksrk=; b=blfKaSAkdPXQXb 4MpTHt+/S16Ha//3fxJey1YADynK1hYzQc1kvjcaeLhz78fFVXbD4pK8MGLgMHdRbg4nwZFUnvJhb 28BaGra8yvEbij228QcR/cALFZglaSFaviCJy/Z68Nn3z4OImAkjBwIhc3rsBGcRrQDbL40Lul/O6 vCmKt40X/OZOS9oO6CmdkGU83ZhFmaFuU4NvN1STVF+J9XSzmwE1hgieLMRyQCSgI9E47j6+sYdyY K/3Idwmp/UjMApp9moQztOvU7Y9+iwHJ2P1CkNeScvSr8uQQoxuLEsQ79e3eaaDnO+dy9NNRJsv/q lnYX6KmI36MsEnJi5TYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pflqm-005G0V-1W; Fri, 24 Mar 2023 18:09:16 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pflqj-005Fzf-2U for linux-arm-kernel@lists.infradead.org; Fri, 24 Mar 2023 18:09:15 +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 dfw.source.kernel.org (Postfix) with ESMTPS id 9EFF962C17; Fri, 24 Mar 2023 18:09:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6D4F5C433EF; Fri, 24 Mar 2023 18:09:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679681350; bh=kWAglP/3M/YpfurCGt08xf8SdrOMxcAlvmBWofnH+50=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=g7qKyXjwRyQ/7rfhwb+FcWl+TNUjHuR/K37ZoWIxgSdzD5KdAQ1zxecVVv8DaIp95 /O7OW+YssvLHLD/Vx7J7f5iWDJRtptE2oVGtFB4m9vzK9ZyniX31i7u9q7BQs9cweI tsauNVu2HhXXBo8zC6R1dhMCXwtjyGvbmkgaftxPHrQCgSzR4b/bfmR+yTsI+SnqfQ nV5yNLepx4kBuluss3Do9BcBeNZVTh6C9bkz0qb1Y/PPDrbzU1s7Jw4unb6p788AlL A5S7kdVbKT9cjV7ZmRYCSB+rKEoYpxcalZyja1y7g1Y3B0KNfT1RlFyYj4v836V087 Lhj+Rc8JELitQ== Date: Fri, 24 Mar 2023 18:09:05 +0000 From: Will Deacon To: Catalin Marinas Cc: "Bouska, Zdenek" , Thomas Gleixner , "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: <20230324180904.GA28266@willie-the-truck> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230324_110913_888222_ECF4AA0D X-CRM114-Status: GOOD ( 33.83 ) 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 05:01:28PM +0000, Catalin Marinas wrote: > 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. I had a pretty gross cpu_relax() implementation using wfe somewhere on LKML, so you could try that if you can dig it up. Generally though, LDXR/STXR and realtime don't mix super well. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel