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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E89DCC433EF for ; Fri, 19 Nov 2021 02:01:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC7B161AA7 for ; Fri, 19 Nov 2021 02:01:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232035AbhKSCEL (ORCPT ); Thu, 18 Nov 2021 21:04:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231314AbhKSCEJ (ORCPT ); Thu, 18 Nov 2021 21:04:09 -0500 Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 090D1C061574 for ; Thu, 18 Nov 2021 18:01:09 -0800 (PST) Received: by mail-pg1-x535.google.com with SMTP id 206so2186437pgb.4 for ; Thu, 18 Nov 2021 18:01:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YlW2Zy9ay7isdEdR2sJFBb6Yrxl6VhtNhx4HmZuVK6A=; b=GZXlqQ5vzxixsSISTPtEuVd2qQB14SXiXP6iIubg1eHhfPllOJxfGbaUJhRZNxIt8f xvwbGQa9ttfVwNd5Mh0XUFEbIuejrr6nnae4JyVF4ibsnOOenzmPQk0ygRJZcSGoIjXb aHndH1EdBkkjTsaR+c716bGFxWwCzyPYJzqTnTpc1tO7sp74y0VTZlmE0agKbZASv1zg fv0vLsBtWpqjw6uqpK5BVrSY86GHI/u1YhkBn+DvhZu3aWGuQ6AQ0SpEyFJFIYmxB5gq 9TiGqihygqN/kv/+A016xNIYDQIf5X3ELvkLmfKg/p5htuLvRI4a1N4vn4crfsppH4un 8AUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YlW2Zy9ay7isdEdR2sJFBb6Yrxl6VhtNhx4HmZuVK6A=; b=g3WIGG/xMsU57jLwPa3eGtfPxCsN+4cS+yr95nX3VLXkX0tQ4gWeoOze0PCXM04QO6 ZzufoQdI1lByAhXJZRDGaBxsTSmOd3e7ByY4Dagw6AgTiwodGawRy3z3D0/i7cw/E9cY lC+WVHYx55eR2VQ9QoNWUyb50/F25txr2Aq58QomvQzg2qz92vWRdEoZJYknTaCJkFGg DAhsnh7WY2WVOVPLre/mMXRJ7ShBenpxWRJA8thwHHZUerD7O9e5VMbGcPGvOLgtGaM+ Ezg4Tb7Qe8ZEASLQH0lKrjps/ycXdaFbak281U0QDUkCoU7PjECTCQrh+EXhRQIe+p6V EZNQ== X-Gm-Message-State: AOAM531sDB0MOg+peucFbNcAVIQtq94hXmZHTBMI1PLcpFZQTiLQbMdV UW8osjkX/BCyoxYamnZrEg== X-Google-Smtp-Source: ABdhPJzM3RHWHbNCTfLAPff9NHMUdkIj19DZJhYtCam18/L58sI3C00nddIZQHSjQWBetdfIt5SHNA== X-Received: by 2002:a63:334d:: with SMTP id z74mr14556252pgz.468.1637287268602; Thu, 18 Nov 2021 18:01:08 -0800 (PST) Received: from piliu.users.ipa.redhat.com ([209.132.188.80]) by smtp.gmail.com with ESMTPSA id c3sm850013pfm.177.2021.11.18.18.01.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Nov 2021 18:01:08 -0800 (PST) Date: Fri, 19 Nov 2021 10:01:00 +0800 From: Pingfan Liu To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, "Paul E . McKenney" , Catalin Marinas , Will Deacon , Marc Zyngier , Thomas Gleixner , Joey Gouly , Sami Tolvanen , Julien Thierry , Yuichi Ito , rcu@vger.kernel.org Subject: Re: [PATCHv3 1/4] arm64: entry: judge nmi ealier to avoid deadlock in RCU Message-ID: References: <20211116082450.10357-1-kernelfans@gmail.com> <20211116082450.10357-2-kernelfans@gmail.com> <20211117113854.GA41542@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211117113854.GA41542@C02TD0UTHF1T.local> Precedence: bulk List-ID: X-Mailing-List: rcu@vger.kernel.org On Wed, Nov 17, 2021 at 11:38:54AM +0000, Mark Rutland wrote: > On Tue, Nov 16, 2021 at 04:24:47PM +0800, Pingfan Liu wrote: > > Linux kernel places strict semantics between NMI and maskable interrupt. > > So does the RCU component, else deadlock may happen. But the current > > arm64 entry code can partially breach this rule through calling > > rcu_nmi_enter(). > > > > *** how a deadlock can happen if NMI mistaken as IRQ *** > > > > rcu_nmi_enter() > > { > > > > if (rcu_dynticks_curr_cpu_in_eqs()) { > > > > if (!in_nmi()) > > rcu_dynticks_task_exit(); > > ... > > if (!in_nmi()) { > > instrumentation_begin(); > > rcu_cleanup_after_idle(); > > instrumentation_end(); > > } > > ... > > } else if (!in_nmi()) { > > instrumentation_begin(); > > rcu_irq_enter_check_tick(); > > } > > > > } > > > > If a NMI is mistaken as a maskable interrupt, rcu_irq_enter_check_tick() > > can hit a deadlock, which is demonstrated by the following scenario: > > > > note_gp_changes() runs in a task context > > { > > local_irq_save(flags); // this protects against irq, but not NMI > > Note: speecifically, this masks IRQs via ICC_PMR_EL1. > > > rnp = rdp->mynode; > > ... > > raw_spin_trylock_rcu_node(rnp) > > -------> broken in by (p)NMI, without taking __nmi_enter() > > AFAICT the broken case described here *cannot* happen. > > If we take a pNMI here, we'll go: > > el1h_64_irq() > -> el1h64_irq_handler() > -> el1_interrupt() > > ... where el1_interrupt is: > > | static void noinstr el1_interrupt(struct pt_regs *regs, > | void (*handler)(struct pt_regs *)) > | { > | write_sysreg(DAIF_PROCCTX_NOIRQ, daif); > | > | if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs)) > | __el1_pnmi(regs, handler); > | else > | __el1_irq(regs, handler); > | } > > ... and interrupts_enabled() is: > > | #define interrupts_enabled(regs) \ > | (!((regs)->pstate & PSR_I_BIT) && irqs_priority_unmasked(regs)) > > ... and irqs_priority_unmasked is: > > | #define irqs_priority_unmasked(regs) \ > | (system_uses_irq_prio_masking() ? \ > | (regs)->pmr_save == GIC_PRIO_IRQON : \ > | true) > > ... irqs_priority_unmasked(regs) *must* return false for this case, > since the local_irq_save() above must have set the PMR to > GIC_PRIO_IRQOFF in the interrupted context. > > Thus, we *must* call __el1_pnmi() in this case, where __el1_pnmi() is: > Yes, you are right. And I made a mistake to think it will call __el1_irq() > | static __always_inline void __el1_pnmi(struct pt_regs *regs, > | void (*handler)(struct pt_regs *)) > | { > | arm64_enter_nmi(regs); > | do_interrupt_handler(regs, handler); > | arm64_exit_nmi(regs); > | } > > Note that arm64_{enter,exit}_nmi() handle the __nmi_{enter,exit}() calls around > do_interupt_handler(). > > > rcu_nmi_enter() > > ->__rcu_irq_enter_check_tick() > > ->raw_spin_lock_rcu_node(rdp->mynode); > > deadlock happens!!! > > > > } > > As above, I don't think this can go wrong as you describe, and don't believe > that this can deadlock. > > > *** On arm64, how pNMI mistaken as IRQ *** > > > > On arm64, pNMI is an analogue to NMI. In essence, it is a higher > > priority interrupt but not disabled by local_irq_disable(). > > > > In current implementation > > 1) If a pNMI from a context where IRQs were masked, it can be recognized > > as nmi, and calls __nmi_enter() immediately. This is no problem. > > Agreed, this case works correctly. > > > 2) But it causes trouble if a pNMI from a context where IRQs were > > unmasked, and temporarily regarded as maskable interrupt. It is not > > treated as NMI, i.e. calling nmi_enter() until reading from GIC. > > > > __el1_irq() > > { > > irq_enter_rcu() ----> hit the deadlock bug > > What is "the deadlock bug"? The example you had above was for a context where > IRQs were *masked*, which is a different case. > As above, I made a mistake about the condition hence the code path. There is no problem in this case through calling __el1_pnmi() Sorry for the false alarm and thank you again for your detailed explaination. Regards, Pingfan 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4424EC433F5 for ; Fri, 19 Nov 2021 02:02:33 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 06F2161AA7 for ; Fri, 19 Nov 2021 02:02:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 06F2161AA7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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=TjReUT8HnxjaJ6B4Qfgts+yxSbWcMbhtKHh+fGwVdL8=; b=LrMpeo87IKwqTA Ev6tDUGPEDEI0J+Vp52JD2dO+ODiFSU4u2k1GzIh3yuluvwFtcnWqN+wwlnadFJiJwb7mMX2zgAWg qeq1ICzr+F71jJVi1Hat0xaFb+/pCekx+SwnLJHkP5ts4rbWAFgFO6SEA2RTncVa7eB3S3gNGyPU+ 4QHdL8II/tX4TqM4CmpyrLGtMmTceiVhAyNampKXWEG13xkX9+3eqMS63vXNK+sv8ThecINK0x4fE 4Qyrjdtd9mnpjj1/1WkH5xyI3fDn0lXBqETLTLlO10rJnhtTxb71YLbQjD0aHXk3v7AxelieqjYsf HjseH/bKkw2kFCd16KrA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mntDF-009Cag-RH; Fri, 19 Nov 2021 02:01:14 +0000 Received: from mail-pg1-x52b.google.com ([2607:f8b0:4864:20::52b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mntDC-009Ca2-12 for linux-arm-kernel@lists.infradead.org; Fri, 19 Nov 2021 02:01:11 +0000 Received: by mail-pg1-x52b.google.com with SMTP id 28so7206988pgq.8 for ; Thu, 18 Nov 2021 18:01:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YlW2Zy9ay7isdEdR2sJFBb6Yrxl6VhtNhx4HmZuVK6A=; b=GZXlqQ5vzxixsSISTPtEuVd2qQB14SXiXP6iIubg1eHhfPllOJxfGbaUJhRZNxIt8f xvwbGQa9ttfVwNd5Mh0XUFEbIuejrr6nnae4JyVF4ibsnOOenzmPQk0ygRJZcSGoIjXb aHndH1EdBkkjTsaR+c716bGFxWwCzyPYJzqTnTpc1tO7sp74y0VTZlmE0agKbZASv1zg fv0vLsBtWpqjw6uqpK5BVrSY86GHI/u1YhkBn+DvhZu3aWGuQ6AQ0SpEyFJFIYmxB5gq 9TiGqihygqN/kv/+A016xNIYDQIf5X3ELvkLmfKg/p5htuLvRI4a1N4vn4crfsppH4un 8AUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YlW2Zy9ay7isdEdR2sJFBb6Yrxl6VhtNhx4HmZuVK6A=; b=MrZ40jMIWZFWCojBh6HTW3A81TY98Ts37qblKaoGpL7pVWu5urlJl+rMItqfaeLP9d UFowU+6b1qmxalnIdH4wzRdow8EmPSFnFMecwQ3iFbiG8IMg0U7Zdi+gCSpDcsxJRNZ1 A4UGouBcheuDGCxt51r648gVFyR0bWzLnQ6VV53fmlLtQjsD8NdQKivWg1br2ilgjgxq 2URCyYjDdbWpfPbXHzs8g2aRhku10le2mdpYkq4zSyzwVj8LShTlNsXVONab9S1PN5Mp 7Sr71QBhjEGyWca4sMHOO5zrxq/i/Y5vQDsufuJuzVgf4e8H+pRnki+XUvtPczT9whKX f9xw== X-Gm-Message-State: AOAM533GwzPjLW7pHQW4q8oNaIrc/Pla/5UOTFS6DZr0aZaoJo83IEU6 SKbFALIL/FzPOYOyF/H1cQ== X-Google-Smtp-Source: ABdhPJzM3RHWHbNCTfLAPff9NHMUdkIj19DZJhYtCam18/L58sI3C00nddIZQHSjQWBetdfIt5SHNA== X-Received: by 2002:a63:334d:: with SMTP id z74mr14556252pgz.468.1637287268602; Thu, 18 Nov 2021 18:01:08 -0800 (PST) Received: from piliu.users.ipa.redhat.com ([209.132.188.80]) by smtp.gmail.com with ESMTPSA id c3sm850013pfm.177.2021.11.18.18.01.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Nov 2021 18:01:08 -0800 (PST) Date: Fri, 19 Nov 2021 10:01:00 +0800 From: Pingfan Liu To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, "Paul E . McKenney" , Catalin Marinas , Will Deacon , Marc Zyngier , Thomas Gleixner , Joey Gouly , Sami Tolvanen , Julien Thierry , Yuichi Ito , rcu@vger.kernel.org Subject: Re: [PATCHv3 1/4] arm64: entry: judge nmi ealier to avoid deadlock in RCU Message-ID: References: <20211116082450.10357-1-kernelfans@gmail.com> <20211116082450.10357-2-kernelfans@gmail.com> <20211117113854.GA41542@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211117113854.GA41542@C02TD0UTHF1T.local> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211118_180110_142785_154027CA X-CRM114-Status: GOOD ( 28.50 ) 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 Wed, Nov 17, 2021 at 11:38:54AM +0000, Mark Rutland wrote: > On Tue, Nov 16, 2021 at 04:24:47PM +0800, Pingfan Liu wrote: > > Linux kernel places strict semantics between NMI and maskable interrupt. > > So does the RCU component, else deadlock may happen. But the current > > arm64 entry code can partially breach this rule through calling > > rcu_nmi_enter(). > > > > *** how a deadlock can happen if NMI mistaken as IRQ *** > > > > rcu_nmi_enter() > > { > > > > if (rcu_dynticks_curr_cpu_in_eqs()) { > > > > if (!in_nmi()) > > rcu_dynticks_task_exit(); > > ... > > if (!in_nmi()) { > > instrumentation_begin(); > > rcu_cleanup_after_idle(); > > instrumentation_end(); > > } > > ... > > } else if (!in_nmi()) { > > instrumentation_begin(); > > rcu_irq_enter_check_tick(); > > } > > > > } > > > > If a NMI is mistaken as a maskable interrupt, rcu_irq_enter_check_tick() > > can hit a deadlock, which is demonstrated by the following scenario: > > > > note_gp_changes() runs in a task context > > { > > local_irq_save(flags); // this protects against irq, but not NMI > > Note: speecifically, this masks IRQs via ICC_PMR_EL1. > > > rnp = rdp->mynode; > > ... > > raw_spin_trylock_rcu_node(rnp) > > -------> broken in by (p)NMI, without taking __nmi_enter() > > AFAICT the broken case described here *cannot* happen. > > If we take a pNMI here, we'll go: > > el1h_64_irq() > -> el1h64_irq_handler() > -> el1_interrupt() > > ... where el1_interrupt is: > > | static void noinstr el1_interrupt(struct pt_regs *regs, > | void (*handler)(struct pt_regs *)) > | { > | write_sysreg(DAIF_PROCCTX_NOIRQ, daif); > | > | if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs)) > | __el1_pnmi(regs, handler); > | else > | __el1_irq(regs, handler); > | } > > ... and interrupts_enabled() is: > > | #define interrupts_enabled(regs) \ > | (!((regs)->pstate & PSR_I_BIT) && irqs_priority_unmasked(regs)) > > ... and irqs_priority_unmasked is: > > | #define irqs_priority_unmasked(regs) \ > | (system_uses_irq_prio_masking() ? \ > | (regs)->pmr_save == GIC_PRIO_IRQON : \ > | true) > > ... irqs_priority_unmasked(regs) *must* return false for this case, > since the local_irq_save() above must have set the PMR to > GIC_PRIO_IRQOFF in the interrupted context. > > Thus, we *must* call __el1_pnmi() in this case, where __el1_pnmi() is: > Yes, you are right. And I made a mistake to think it will call __el1_irq() > | static __always_inline void __el1_pnmi(struct pt_regs *regs, > | void (*handler)(struct pt_regs *)) > | { > | arm64_enter_nmi(regs); > | do_interrupt_handler(regs, handler); > | arm64_exit_nmi(regs); > | } > > Note that arm64_{enter,exit}_nmi() handle the __nmi_{enter,exit}() calls around > do_interupt_handler(). > > > rcu_nmi_enter() > > ->__rcu_irq_enter_check_tick() > > ->raw_spin_lock_rcu_node(rdp->mynode); > > deadlock happens!!! > > > > } > > As above, I don't think this can go wrong as you describe, and don't believe > that this can deadlock. > > > *** On arm64, how pNMI mistaken as IRQ *** > > > > On arm64, pNMI is an analogue to NMI. In essence, it is a higher > > priority interrupt but not disabled by local_irq_disable(). > > > > In current implementation > > 1) If a pNMI from a context where IRQs were masked, it can be recognized > > as nmi, and calls __nmi_enter() immediately. This is no problem. > > Agreed, this case works correctly. > > > 2) But it causes trouble if a pNMI from a context where IRQs were > > unmasked, and temporarily regarded as maskable interrupt. It is not > > treated as NMI, i.e. calling nmi_enter() until reading from GIC. > > > > __el1_irq() > > { > > irq_enter_rcu() ----> hit the deadlock bug > > What is "the deadlock bug"? The example you had above was for a context where > IRQs were *masked*, which is a different case. > As above, I made a mistake about the condition hence the code path. There is no problem in this case through calling __el1_pnmi() Sorry for the false alarm and thank you again for your detailed explaination. Regards, Pingfan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel