Received: by 2002:ac0:946b:0:0:0:0:0 with SMTP id j40csp2715738imj; Mon, 11 Feb 2019 07:24:59 -0800 (PST) X-Google-Smtp-Source: AHgI3IZEs67TO/668P6HSWvsDY05NBro3ciYK6+BP/j2NTAcWFWA1WKXydhiu52SnJQ/+PN38ceu X-Received: by 2002:a63:a5b:: with SMTP id z27mr5552132pgk.78.1549898699530; Mon, 11 Feb 2019 07:24:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1549898699; cv=none; d=google.com; s=arc-20160816; b=ylCYhdk2yUk3kBQiudu2gnyfWb3ZVyotHfKWTP0tUUKcqNJni61ShxFS4fy+uiorFy GHHvHG3Nnen7Azwjw6EiLKIYIfpApwh3xxXGyBcf711aJWKFFSnLfMdNX8nIezcjjJBX JMTnQhuTN0nkd+dSk5GONxi1+J1n9u7jWpg4P4fTOyQFKNpnkz9Bl4Jzlpy7cuuhS2bl FO3KSR2moVgIcv1FXHszXfQpEaHAsr5St9dwd6W2sSIaEAYyWuNhbGNJ1cz2HJMe9m6E 9xX1XPasUw+8I9BAj19NqMsuK4jsv33ougu8eszc3d3otU2qmRakn5BwrXoItLD3/0Z4 hCXQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=LyRjcRnMx3iyNNwIc/ekaY6w8saigqtJ1PppAijhY+A=; b=rBMxuxzFrVvfNGfQ6jtT/deWgKOlqdzYTXoHxi3mNO+hnFExpBPhvSn+Ecuuv3lXWK Wk6YufVw6v4FyqW7RxqzQC0M8s5APtgSlmAnHPG+38gna3pK3hIKiFP0Jxwew2sZD+bc E3pRRS7jrzcHyGWHy7lNllE0cA/Y6eyS/70IyMVOkZyCUKlvrh8DolllT85c+1xQvamj chxBUtUHD14c4rJx4upR/yfLW0WWQMoW27q0+NsFchmFLZ0RyL4daqsWxSnP4s5PxzV7 DU8BQtfWpgP2GvG4wQ6lau2x6hTxhFLbqbY7t5EUJ6nWZu+mTwMv1CcSicI6Ffiw4U3I 3MWg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="0/5hG+HM"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i90si11030722pli.135.2019.02.11.07.24.43; Mon, 11 Feb 2019 07:24:59 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="0/5hG+HM"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390531AbfBKPDB (ORCPT + 99 others); Mon, 11 Feb 2019 10:03:01 -0500 Received: from mail.kernel.org ([198.145.29.99]:51182 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390504AbfBKPC4 (ORCPT ); Mon, 11 Feb 2019 10:02:56 -0500 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 686FA222AB; Mon, 11 Feb 2019 15:02:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1549897374; bh=zNZvpH/iwldeJwo8dRiocKNXwtGYMp281U6OyDK/t3I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=0/5hG+HMA9apl0fRBqhgs3UtUuSHDSRSxbTuZ3qp6aITOqN//rgkvo2eneueMqjvo PNEeSBtlJkOGNJZos6543a8zo1G68Myj7mJaeZ0izFhvYE7a9OZeanl3KKhqgG/j98 5D9vnijTwkdpucuYDdiO/Kp6O4myM//zqPJvsPRI= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Lukas Wunner , Frank Pavlic , Martin Sperl , Florian Meier , Clive Messer , Matthias Reichl , Stefan Wahren , Florian Kauer , Vinod Koul Subject: [PATCH 4.14 180/205] dmaengine: bcm2835: Fix interrupt race on RT Date: Mon, 11 Feb 2019 15:19:38 +0100 Message-Id: <20190211141840.150061655@linuxfoundation.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190211141827.214852402@linuxfoundation.org> References: <20190211141827.214852402@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.14-stable review patch. If anyone has any objections, please let me know. ------------------ From: Lukas Wunner commit f7da7782aba92593f7b82f03d2409a1c5f4db91b upstream. If IRQ handlers are threaded (either because CONFIG_PREEMPT_RT_BASE is enabled or "threadirqs" was passed on the command line) and if system load is sufficiently high that wakeup latency of IRQ threads degrades, SPI DMA transactions on the BCM2835 occasionally break like this: ks8851 spi0.0: SPI transfer timed out bcm2835-dma 3f007000.dma: DMA transfer could not be terminated ks8851 spi0.0 eth2: ks8851_rdfifo: spi_sync() failed The root cause is an assumption made by the DMA driver which is documented in a code comment in bcm2835_dma_terminate_all(): /* * Stop DMA activity: we assume the callback will not be called * after bcm_dma_abort() returns (even if it does, it will see * c->desc is NULL and exit.) */ That assumption falls apart if the IRQ handler bcm2835_dma_callback() is threaded: A client may terminate a descriptor and issue a new one before the IRQ handler had a chance to run. In fact the IRQ handler may miss an *arbitrary* number of descriptors. The result is the following race condition: 1. A descriptor finishes, its interrupt is deferred to the IRQ thread. 2. A client calls dma_terminate_async() which sets channel->desc = NULL. 3. The client issues a new descriptor. Because channel->desc is NULL, bcm2835_dma_issue_pending() immediately starts the descriptor. 4. Finally the IRQ thread runs and writes BCM2835_DMA_INT to the CS register to acknowledge the interrupt. This clears the ACTIVE flag, so the newly issued descriptor is paused in the middle of the transaction. Because channel->desc is not NULL, the IRQ thread finalizes the descriptor and tries to start the next one. I see two possible solutions: The first is to call synchronize_irq() in bcm2835_dma_issue_pending() to wait until the IRQ thread has finished before issuing a new descriptor. The downside of this approach is unnecessary latency if clients desire rapidly terminating and re-issuing descriptors and don't have any use for an IRQ callback. (The SPI TX DMA channel is a case in point.) A better alternative is to make the IRQ thread recognize that it has missed descriptors and avoid finalizing the newly issued descriptor. So first of all, set the ACTIVE flag when acknowledging the interrupt. This keeps a newly issued descriptor running. If the descriptor was finished, the channel remains idle despite the ACTIVE flag being set. However the ACTIVE flag can then no longer be used to check whether the channel is idle, so instead check whether the register containing the current control block address is zero and finalize the current descriptor only if so. That way, there is no impact on latency and throughput if the client doesn't care for the interrupt: Only minimal additional overhead is introduced for non-cyclic descriptors as one further MMIO read is necessary per interrupt to check for idleness of the channel. Cyclic descriptors are sped up slightly by removing one MMIO write per interrupt. Fixes: 96286b576690 ("dmaengine: Add support for BCM2835") Signed-off-by: Lukas Wunner Cc: stable@vger.kernel.org # v3.14+ Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier Cc: Clive Messer Cc: Matthias Reichl Tested-by: Stefan Wahren Acked-by: Florian Kauer Signed-off-by: Vinod Koul Signed-off-by: Greg Kroah-Hartman --- drivers/dma/bcm2835-dma.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -421,7 +421,12 @@ static int bcm2835_dma_abort(void __iome long int timeout = 10000; cs = readl(chan_base + BCM2835_DMA_CS); - if (!(cs & BCM2835_DMA_ACTIVE)) + + /* + * A zero control block address means the channel is idle. + * (The ACTIVE flag in the CS register is not a reliable indicator.) + */ + if (!readl(chan_base + BCM2835_DMA_ADDR)) return 0; /* Write 0 to the active bit - Pause the DMA */ @@ -485,8 +490,15 @@ static irqreturn_t bcm2835_dma_callback( spin_lock_irqsave(&c->vc.lock, flags); - /* Acknowledge interrupt */ - writel(BCM2835_DMA_INT, c->chan_base + BCM2835_DMA_CS); + /* + * Clear the INT flag to receive further interrupts. Keep the channel + * active in case the descriptor is cyclic or in case the client has + * already terminated the descriptor and issued a new one. (May happen + * if this IRQ handler is threaded.) If the channel is finished, it + * will remain idle despite the ACTIVE flag being set. + */ + writel(BCM2835_DMA_INT | BCM2835_DMA_ACTIVE, + c->chan_base + BCM2835_DMA_CS); d = c->desc; @@ -494,11 +506,7 @@ static irqreturn_t bcm2835_dma_callback( if (d->cyclic) { /* call the cyclic callback */ vchan_cyclic_callback(&d->vd); - - /* Keep the DMA engine running */ - writel(BCM2835_DMA_ACTIVE, - c->chan_base + BCM2835_DMA_CS); - } else { + } else if (!readl(c->chan_base + BCM2835_DMA_ADDR)) { vchan_cookie_complete(&c->desc->vd); bcm2835_dma_start_desc(c); } @@ -806,11 +814,7 @@ static int bcm2835_dma_terminate_all(str list_del_init(&c->node); spin_unlock(&d->lock); - /* - * Stop DMA activity: we assume the callback will not be called - * after bcm_dma_abort() returns (even if it does, it will see - * c->desc is NULL and exit.) - */ + /* stop DMA activity */ if (c->desc) { bcm2835_dma_desc_free(&c->desc->vd); c->desc = NULL; @@ -818,8 +822,7 @@ static int bcm2835_dma_terminate_all(str /* Wait for stopping */ while (--timeout) { - if (!(readl(c->chan_base + BCM2835_DMA_CS) & - BCM2835_DMA_ACTIVE)) + if (!readl(c->chan_base + BCM2835_DMA_ADDR)) break; cpu_relax();