From patchwork Wed Apr 27 03:26:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jassi Brar X-Patchwork-Id: 66766 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp1956733qge; Tue, 26 Apr 2016 20:27:03 -0700 (PDT) X-Received: by 10.66.132.72 with SMTP id os8mr8585146pab.63.1461727622955; Tue, 26 Apr 2016 20:27:02 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id os10si2574631pab.228.2016.04.26.20.27.02; Tue, 26 Apr 2016 20:27:02 -0700 (PDT) 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=@gmail.com; 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; dmarc=pass (p=NONE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753130AbcD0D0z (ORCPT + 29 others); Tue, 26 Apr 2016 23:26:55 -0400 Received: from mail-io0-f173.google.com ([209.85.223.173]:32955 "EHLO mail-io0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753051AbcD0D0w (ORCPT ); Tue, 26 Apr 2016 23:26:52 -0400 Received: by mail-io0-f173.google.com with SMTP id f89so35100232ioi.0; Tue, 26 Apr 2016 20:26:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=mxPGPMaDZPseDz6R4mmEoVy4Oej7QRshM32iycRFsgI=; b=TwcS6wvYitmQaJnYIR913W/Ww2f8ciS/VsFR+lzfZONolQrKyjoqCRzQY18yDFK+1L DVEaaALCQpYODkzUwsiKGat6EKs+LRsj4rG9BDlPuaCQQLs1W9/Vpw/+W9zGFf4Di4Wi 3tHeojKT1L+hUUIp/S/B916Rs84xMjAimfk63cla84NzZJXdN/F13v3lGkBTo1FPEEXg ELHx022PJJyaBm4efPh+dI1VAQzw6pDBdbhBmhhD2B0FGuUPRWm6f5JY4EDNA2i7KeVL aiMFEYps16DJkWj1nLzFEBObPc/0N6dW3wRL2sxbFy13biKR/xAT/IO8sC/YHvVaseKU EGoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=mxPGPMaDZPseDz6R4mmEoVy4Oej7QRshM32iycRFsgI=; b=kTvpEOFL1VDmxjq1zOemhVjr/c63lzPBjLpdecLGPJpabpfI4AtsKdh2Rc0u/SMtPx jB9hQyMzQTe5hzX+hZIlk+UM/sqJVlUps/oZbfapsSJJJ/7eSSHiLJKX8JZkKMCc6g3/ opoyDN2p7u2ugmWePGRMJBRQYP+WKlfH6GCcpmo2SCfDZS5IyqaNMpdTVInraeKbqxkt m5a/YUtJlYwFqM7STyIAdBaHHUiyE0/FvcPSqpCW5ywalnO3lc5rG7BBjZNQ2BFnDSU0 NUrisVOXVHS59VtPyCAVgLT/swFNYELH8OqNjz0jV0BC5rfJ0+tjPcmhLMpCJ/UE6NLx satA== X-Gm-Message-State: AOPr4FWuSAHqZ1ZVtxD07oiQfI1ZpQU7RcTOxxjLFZg+o/E7nzhskG9YWLNVt49GOGY5higSYYmXAzna6/wU5A== MIME-Version: 1.0 X-Received: by 10.107.201.207 with SMTP id z198mr7600237iof.120.1461727611023; Tue, 26 Apr 2016 20:26:51 -0700 (PDT) Received: by 10.107.138.226 with HTTP; Tue, 26 Apr 2016 20:26:50 -0700 (PDT) In-Reply-To: References: Date: Wed, 27 Apr 2016 08:56:50 +0530 Message-ID: Subject: Re: [PATCH] dmaengine: pl330: Fix race in pl330_get_desc() From: Jassi Brar To: Robin Murphy Cc: Vinod Koul , Dan Williams , dmaengine@vger.kernel.org, Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 26, 2016 at 10:00 PM, Robin Murphy wrote: > The current logic in pl330_get_desc() contains a clear race condition, > whereby if the descriptor pool is empty, we will create a new > descriptor, add it to the pool with the lock held, *release the lock*, > then try to remove it from the pool again. > > Furthermore, if that second attempt then finds the pool empty because > someone else got there first, we conclude that something must have gone > terribly wrong and it's the absolute end of the world. > We may retry or simply increase the number of descriptors allocated in a batch from 1 to, say, NR_DEFAULT_DESC. /* Try again */ > ... > [ 709.328891] dma-pl330 7ff00000.dma: pl330_get_desc:2459 ALERT! > ** 10 printk messages dropped ** [ 709.352280] dma-pl330 7ff00000.dma: __pl330_prep_dma_memcpy:2493 Unable to fetch desc > ** 11 printk messages dropped ** [ 709.372930] dma-pl330 7ff00000.dma: pl330_get_desc:2459 ALERT! > ** 2 printk messages dropped ** [ 709.372953] dma-pl330 7ff00000.dma: pl330_get_desc:2459 ALERT! > ** 8 printk messages dropped ** [ 709.374157] dma-pl330 7ff00000.dma: __pl330_prep_dma_memcpy:2493 Unable to fetch desc > ** 41 printk messages dropped ** [ 709.393095] dmatest: dma0chan4-copy1: result #4545: 'prep error' with src_off=0x3a32 dst_off=0x46dd len=0xc5c3 (0) > ... > > Down with this sort of thing! The most sensible thing to do is avoid the > allocate-add-remove dance entirely and simply use the freshly-allocated > descriptor straight away. > ... but you still try to first pluck from the list. Instead of churning the code, I would suggest either check in a loop that we have a desc OR allocate 2 or NR_DEFAULT_DESC descriptors there. Probably we get more descriptors at the same cost of memory. > > I'm also seeing what looks like another occasional race under the same > conditions where pl330_tx_submit() blows up from dma_cookie_assign() > dereferencing a bogus tx->chan, but I think that's beyond my ability to > figure out right now. Similarly the storm of WARNs from pl330_issue_pending() > when using a large number of small buffers and dmatest.noverify=1. This > one was some obvious low-hanging fruit. > Sorry, that part of code has changed a lot since I wrote the driver, so more details will help me. Thanks. diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 372b435..7179a4d 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2449,7 +2449,7 @@ static struct dma_pl330_desc *pl330_get_desc(struct dma_pl330_chan *pch) /* If the DMAC pool is empty, alloc new */ if (!desc) { - if (!add_desc(pl330, GFP_ATOMIC, 1)) + if (!add_desc(pl330, GFP_ATOMIC, NR_DEFAULT_DESC)) return NULL;