From patchwork Fri Jan 27 14:04:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 92648 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp249929qgi; Fri, 27 Jan 2017 06:06:55 -0800 (PST) X-Received: by 10.84.232.141 with SMTP id i13mr12752703plk.119.1485526015636; Fri, 27 Jan 2017 06:06:55 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [65.50.211.133]) by mx.google.com with ESMTPS id o3si4580139pfk.8.2017.01.27.06.06.55 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Jan 2017 06:06:55 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 65.50.211.133 as permitted sender) client-ip=65.50.211.133; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 65.50.211.133 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cX7B0-0007bL-DC; Fri, 27 Jan 2017 14:06:54 +0000 Received: from mail-lf0-f44.google.com ([209.85.215.44]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cX7AZ-0007Z2-CW for linux-arm-kernel@lists.infradead.org; Fri, 27 Jan 2017 14:06:29 +0000 Received: by mail-lf0-f44.google.com with SMTP id n124so162052484lfd.2 for ; Fri, 27 Jan 2017 06:06:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=7Y+q3djCguQjt+X5MqOofxdUCTNMh2eM+in5UOxckUE=; b=JFR3TXLWiy3KLBGChfxaozPU9SuDOeRaZID2dzAldcSMESnI1Gsjc3sBPCrXGGUKCL E73CSRaIj32NqbnaiT9aqexB8ilDhZfL+JfUFYm4pGaf2hziPbzhUUjrypI9k3197xPk TEfxzJb1VrIYO7b4Ai+7FgPa+jjZu4d/AWdjk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=7Y+q3djCguQjt+X5MqOofxdUCTNMh2eM+in5UOxckUE=; b=HNLC8doDScW5uRj5PPKWXA7Yl05QesrWRPXAkmTs4VYnXTAolMhExgUiVeguMHaYXo 1KeNUrrZF2Sb3uyvdFAyGUWNPB16pvAfPb316acEw4+xRTuWrWgw6c5Xvtvh9Jp4YCGm clZAvX8xsiMyEu74zYDSmB/Xhi1za+FvQ5fdS0XX0sEp/fHs5IXVOMIzdbvHZ6jwoqOd hw9ygOsg/NGPcswD9LkPhW5+vJPN7d4LhtOVY5/r4NicHmXV2/saCjaNAIAt2OtJP9BU EguJ3Y5h/lygd9WmPpXuIuZsMeb3jt9+bKDTaBq6EU95AqLHXsgz4WE+LRnz7QNa0ajm Xq9g== X-Gm-Message-State: AIkVDXLBGhtA1P4e4/HC+o9R6J5Lx2fuqHZTEOlTEuhWYAZcRrMBPNC/VNpS7Xry+ljlWNzV X-Received: by 10.25.135.130 with SMTP id j124mr2779688lfd.11.1485525904717; Fri, 27 Jan 2017 06:05:04 -0800 (PST) Received: from gnarp.ideon.se ([85.235.10.227]) by smtp.gmail.com with ESMTPSA id e18sm1325276lfg.22.2017.01.27.06.05.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Jan 2017 06:05:03 -0800 (PST) From: Linus Walleij To: Ulf Hansson , linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Srinivas Kandagatla Subject: [PATCH] mmc: core/mmci: restore pre/post_req behaviour Date: Fri, 27 Jan 2017 15:04:54 +0100 Message-Id: <20170127140454.7990-1-linus.walleij@linaro.org> X-Mailer: git-send-email 2.9.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170127_060627_768652_22A9D4A4 X-CRM114-Status: GOOD ( 21.07 ) X-Spam-Score: -1.5 (-) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-1.5 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.5 RCVD_IN_SORBS_SPAM RBL: SORBS: sender is a spam source [209.85.215.44 listed in dnsbl.sorbs.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.215.44 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.215.44 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 RCVD_IN_MSPIKE_WL Mailspike good senders X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linus Walleij , Russell King MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org commit 64b12a68a9f74bb32d8efd7af1ad8a2ba02fc884 "mmc: core: fix prepared requests while doing bkops" is fixing a bug in the wrong way. A bug in the MMCI device driver is fixed by amending the MMC core. Thinking about it: what the pre- and post-callbacks are doing is to essentially map and unmap SG lists for DMA transfers. Why would we not be able to do that just because a BKOPS command is sent inbetween? Having to unprepare/prepare the next asynchronous request for DMA seems wrong. Looking the backtrace in that commit we can see what the real problem actually is: mmci_data_irq() is calling mmci_dma_unmap() twice which is goung to call arm_dma_unmap_sg() twice and v7_dma_inv_range() twice for the same sglist and that will crash. This happens because a request is prepared, then a BKOPS is sent. The IRQ completing the BKOPS command goes through mmci_data_irq() and thinks that a DMA operation has just been completed because dma_inprogress() reports true. It then proceeds to unmap the sglist. But that was wrong! dma_inprogress() should NOT be true because no DMA was actually in progress! We had just prepared the sglist, and the DMA channel dma_current has been configured, but NOT started! Because of this, the sglist is already unmapped when we get our actual data completion IRQ, and we are unmapping the sglist once more, and we get this crash. Therefore, we need to revert this solution pushing the problem to the core and causing problems, and instead augment the implementation such that dma_inprogress() only reports true if some DMA has actually been started. After this we can keep the request prepared during the BKOPS and we need not unprepare/reprepare it. Fixes: 64b12a68a9f7 ("mmc: core: fix prepared requests while doing bkops") Cc: Srinivas Kandagatla Signed-off-by: Linus Walleij --- This was found when trying to refactor the stack to complete requests from the mmc_request_done() call: pretty tricky to do this when you don't have the previous and next-to-run asynchronous request available. Luckily I think it is just a bug fix done the wrong way. I will build further patches on this one. --- drivers/mmc/core/core.c | 9 --------- drivers/mmc/host/mmci.c | 7 ++++++- drivers/mmc/host/mmci.h | 3 ++- 3 files changed, 8 insertions(+), 11 deletions(-) -- 2.9.3 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ed1768cf464a..a160c3a7777a 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -676,16 +676,7 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host, ((mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1) || (mmc_resp_type(host->areq->mrq->cmd) == MMC_RSP_R1B)) && (host->areq->mrq->cmd->resp[0] & R1_EXCEPTION_EVENT)) { - - /* Cancel the prepared request */ - if (areq) - mmc_post_req(host, areq->mrq, -EINVAL); - mmc_start_bkops(host->card, true); - - /* prepare the request again */ - if (areq) - mmc_pre_req(host, areq->mrq); } } diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 01a804792f30..bc5fd04acd0f 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -507,6 +507,7 @@ static void mmci_dma_data_error(struct mmci_host *host) { dev_err(mmc_dev(host->mmc), "error during DMA transfer!\n"); dmaengine_terminate_all(host->dma_current); + host->dma_in_progress = false; host->dma_current = NULL; host->dma_desc_current = NULL; host->data->host_cookie = 0; @@ -565,6 +566,7 @@ static void mmci_dma_finalize(struct mmci_host *host, struct mmc_data *data) mmci_dma_release(host); } + host->dma_in_progress = false; host->dma_current = NULL; host->dma_desc_current = NULL; } @@ -665,6 +667,7 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl) dev_vdbg(mmc_dev(host->mmc), "Submit MMCI DMA job, sglen %d blksz %04x blks %04x flags %08x\n", data->sg_len, data->blksz, data->blocks, data->flags); + host->dma_in_progress = true; dmaengine_submit(host->dma_desc_current); dma_async_issue_pending(host->dma_current); @@ -740,8 +743,10 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq, if (host->dma_desc_current == next->dma_desc) host->dma_desc_current = NULL; - if (host->dma_current == next->dma_chan) + if (host->dma_current == next->dma_chan) { + host->dma_in_progress = false; host->dma_current = NULL; + } next->dma_desc = NULL; next->dma_chan = NULL; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 56322c6afba4..4a8bef1aac8f 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -245,8 +245,9 @@ struct mmci_host { struct dma_chan *dma_tx_channel; struct dma_async_tx_descriptor *dma_desc_current; struct mmci_host_next next_data; + bool dma_in_progress; -#define dma_inprogress(host) ((host)->dma_current) +#define dma_inprogress(host) ((host)->dma_in_progress) #else #define dma_inprogress(host) (0) #endif