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 3F0A3C43217 for ; Mon, 28 Nov 2022 11:30:38 +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:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PsFDgnONVCHhBD6pFIK9zu1aoomnmbI6QWGTpYjc9Tw=; b=eoDiv9+I3dDxzx f0PHJYzBSsFyr3oyW6C8gf037RWbS9VEWYrgdJ2+EkWmlb1STidxupk9H/oLll2985WepBdAak4OY bfh+zyOUDXag4qIs3t3Uc6H1WUYxVCsSS94NnVODkCy66POOnAJc9+7mRM/qp6V0b92tDEw3wBVwE P4chkXvs0mJxkT3tdODe2mRp7fTspr54dXeL5KNC3r2qtBnBv8hZ/+ACt2/YaTZGWfki97tQZBVOU Kq5CnqT8JYbVf5WEb376s8RPmdIo2Wa8cvQbkn+P5uvO9nW4W988pxcpzkUVqXrgcHY+Xufj9URUo NffEMN9t9fTtBM3rvbMQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ozcLG-001JO0-UM; Mon, 28 Nov 2022 11:30:30 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ozcLD-001JNP-PL for linux-riscv@lists.infradead.org; Mon, 28 Nov 2022 11:30:29 +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 07B6660DB7; Mon, 28 Nov 2022 11:30:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 67B87C433D6; Mon, 28 Nov 2022 11:30:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669635026; bh=FlN6mZAjHFIwHMgmb20rPc7r9b/s3sZD9D4NcnTdWtk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=JwyLYpDvJrFyIj527Rqgk28NIFL1kwR8Qks+4zyjIpO7gFR8e3TsV6/JLLjAyLioU JWpREYT+FenPSAxZ/Is5Q5EuBsZDntf0iJiwh5dRPZkHwTdGCtX6COAzP5tE+oIdPt AJLBWkROaQLUchsvM8dpRM3K0q8vvYFPPE+1LPT+gqh81c7jd8Y6N4RukxLmkXggp+ qgwGjTNRDLADYkXfofbss6tCjluuDRfh35fD65IqakTC28pwy6ebQ5fQcboMWYyQFx e/DwH6e9h7XY07RAzZla4MVzF64OXeQ/OgJfbdne+oA9eA+1HBu9Wd3VSbDfG/it6c kKO/sQNkdPR8g== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1ozcL9-008zqv-Uu; Mon, 28 Nov 2022 11:30:24 +0000 Date: Mon, 28 Nov 2022 11:30:23 +0000 Message-ID: <86ilizmi40.wl-maz@kernel.org> From: Marc Zyngier To: Anup Patel Cc: Palmer Dabbelt , Paul Walmsley , Thomas Gleixner , Daniel Lezcano , Atish Patra , Alistair Francis , Anup Patel , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v12 3/7] genirq: Add mechanism to multiplex a single HW IPI In-Reply-To: References: <20221126173453.306088-1-apatel@ventanamicro.com> <20221126173453.306088-4-apatel@ventanamicro.com> <86k03fmkox.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: apatel@ventanamicro.com, palmer@dabbelt.com, paul.walmsley@sifive.com, tglx@linutronix.de, daniel.lezcano@linaro.org, atishp@atishpatra.org, Alistair.Francis@wdc.com, anup@brainfault.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221128_033027_924588_5BDDF650 X-CRM114-Status: GOOD ( 38.58 ) X-BeenThere: linux-riscv@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-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Mon, 28 Nov 2022 11:13:30 +0000, Anup Patel wrote: > > On Mon, Nov 28, 2022 at 4:04 PM Marc Zyngier wrote: > > > > On Sat, 26 Nov 2022 17:34:49 +0000, > > Anup Patel wrote: > > > > > > +static void ipi_mux_send_mask(struct irq_data *d, const struct cpumask *mask) > > > +{ > > > + u32 ibit = BIT(irqd_to_hwirq(d)); > > > + struct ipi_mux_cpu *icpu = this_cpu_ptr(ipi_mux_pcpu); > > > + struct cpumask *send_mask = &icpu->send_mask; > > > + unsigned long flags; > > > + int cpu; > > > + > > > + /* > > > + * We use send_mask as a per-CPU variable so disable local > > > + * interrupts to avoid being preempted. > > > + */ > > > + local_irq_save(flags); > > > > The correct way to avoid preemption is to use preempt_disable(), which > > is a lot cheaper than disabling interrupt on most architectures. > > Okay, I will update. > > > > > > + > > > + cpumask_clear(send_mask); > > > > This thing is likely to be unnecessarily expensive on very large > > systems, as it is proportional to the number of CPUs. > > > > > + > > > + for_each_cpu(cpu, mask) { > > > + icpu = per_cpu_ptr(ipi_mux_pcpu, cpu); > > > + atomic_or(ibit, &icpu->bits); > > > > The original code had an atomic_fetch_or_release() to allow eliding > > the IPI if the target interrupt was already pending. Why is that code > > gone? This is a pretty cheap and efficient optimisation. > > That optimization is causing RCU stalls on QEMU RISC-V virt > machine with large number of CPUs. Then there is a bug somewhere, either in the implementation of the atomic operations or in QEMU. Or maybe even in the original code (though this looks unlikely given how heavily this is used on actual HW - I'm typing this email from one of these machines, and I'd be pretty annoyed if I was missing IPIs). In any case, please don't paper over this. > > > > > > + > > > + /* > > > + * The atomic_or() above must complete before > > > + * the atomic_read() below to avoid racing with > > > + * ipi_mux_unmask(). > > > + */ > > > + smp_mb__after_atomic(); > > > + > > > + if (atomic_read(&icpu->enable) & ibit) > > > + cpumask_set_cpu(cpu, send_mask); > > > + } > > > + > > > + /* Trigger the parent IPI */ > > > + ipi_mux_send(send_mask); > > > > IPIs are very rarely made pending on more than a single CPU at a > > time. The overwhelming majority of them are targeting a single CPU. So > > accumulating bits to avoid doing two or more "send" actions only > > penalises the generic case. > > > > My conclusion is that this "send_mask" can probably be removed, > > together with the preemption fiddling. > > So, we should call ipi_mux_send() for one target CPU at a time ? I think so, as it matches my measurements from a few years ago. It also simplifies things significantly, leading to better performance for the common case. Add some instrumentation and see whether this is still the case though. > > > > > > + > > > + local_irq_restore(flags); > > > +} > > > + > > > +static const struct irq_chip ipi_mux_chip = { > > > + .name = "IPI Mux", > > > + .irq_mask = ipi_mux_mask, > > > + .irq_unmask = ipi_mux_unmask, > > > + .ipi_send_mask = ipi_mux_send_mask, > > > +}; > > > > OK, you have now dropped the superfluous pre/post handlers. But the > > need still exists. Case in point, the aic_handle_ipi() prologue and > > epilogue to the interrupt handling. I have suggested last time that > > the driver could provide the actual struct irq_chip in order to > > provide the callbacks it requires. > > The aic_handle_ipi() can simply call ipi_mux_process() between > the prologue and epilogue. Hmm. OK. That's not what I had in mind, but fair enough. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv