From patchwork Wed Apr 11 12:03:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 10335371 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0AC1660134 for ; Wed, 11 Apr 2018 12:03:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E49F528346 for ; Wed, 11 Apr 2018 12:03:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D73F12884E; Wed, 11 Apr 2018 12:03:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 05FDA28346 for ; Wed, 11 Apr 2018 12:03:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751972AbeDKMDt (ORCPT ); Wed, 11 Apr 2018 08:03:49 -0400 Received: from osg.samsung.com ([64.30.133.232]:57060 "EHLO osg.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751797AbeDKMDs (ORCPT ); Wed, 11 Apr 2018 08:03:48 -0400 Received: from localhost (localhost [127.0.0.1]) by osg.samsung.com (Postfix) with ESMTP id C5D273F78F; Wed, 11 Apr 2018 05:03:47 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at dev.s-opensource.com Received: from osg.samsung.com ([127.0.0.1]) by localhost (localhost [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vHhkvxCzK3vZ; Wed, 11 Apr 2018 05:03:41 -0700 (PDT) Received: from smtp.s-opensource.com (unknown [179.95.62.207]) by osg.samsung.com (Postfix) with ESMTPSA id 912B93F787; Wed, 11 Apr 2018 05:03:41 -0700 (PDT) Received: from mchehab by smtp.s-opensource.com with local (Exim 4.90_1) (envelope-from ) id 1f6ETS-0004ap-PC; Wed, 11 Apr 2018 08:03:38 -0400 From: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab , Linux Media Mailing List , Mauro Carvalho Chehab , Daniel Scheller Subject: [PATCH] media: ddbridge: better handle optional spin locks at the code Date: Wed, 11 Apr 2018 08:03:37 -0400 Message-Id: <5156a3b987ae3698ff4c650a6395997f93ba093e.1523448215.git.mchehab@s-opensource.com> X-Mailer: git-send-email 2.14.3 To: unlisted-recipients:; (no To-header on input) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, ddbridge produces 4 warnings on sparse: drivers/media/pci/ddbridge/ddbridge-core.c:495:9: warning: context imbalance in 'ddb_output_start' - different lock contexts for basic block drivers/media/pci/ddbridge/ddbridge-core.c:510:9: warning: context imbalance in 'ddb_output_stop' - different lock contexts for basic block drivers/media/pci/ddbridge/ddbridge-core.c:525:9: warning: context imbalance in 'ddb_input_stop' - different lock contexts for basic block drivers/media/pci/ddbridge/ddbridge-core.c:560:9: warning: context imbalance in 'ddb_input_start' - different lock contexts for basic block Those are all false positives, but they result from the fact that there could potentially have some troubles at the locking schema, because the lock depends on a var (output->dma). I discussed that in priv with Sparse author and with the current maintainer. Both believe that sparse is doing the right thing, and that the proper fix would be to change the code to make it clearer that, when spin_lock_irq() is called, spin_unlock_irq() will be also called. That help not only static analyzers to better understand the code, but also humans that could need to take a look at the code. It was also pointed that gcc would likely be smart enough to optimize the code and produce the same result. I double checked: indeed, the size of the driver didn't change after this patch. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/ddbridge/ddbridge-core.c | 43 ++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index 4a2819d3e225..080e2189ca7f 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -458,13 +458,12 @@ static void calc_con(struct ddb_output *output, u32 *con, u32 *con2, u32 flags) *con2 = (nco << 16) | gap; } -static void ddb_output_start(struct ddb_output *output) +static void __ddb_output_start(struct ddb_output *output) { struct ddb *dev = output->port->dev; u32 con = 0x11c, con2 = 0; if (output->dma) { - spin_lock_irq(&output->dma->lock); output->dma->cbuf = 0; output->dma->coff = 0; output->dma->stat = 0; @@ -492,9 +491,18 @@ static void ddb_output_start(struct ddb_output *output) ddbwritel(dev, con | 1, TS_CONTROL(output)); - if (output->dma) { + if (output->dma) output->dma->running = 1; +} + +static void ddb_output_start(struct ddb_output *output) +{ + if (output->dma) { + spin_lock_irq(&output->dma->lock); + __ddb_output_start(output); spin_unlock_irq(&output->dma->lock); + } else { + __ddb_output_start(output); } } @@ -502,15 +510,13 @@ static void ddb_output_stop(struct ddb_output *output) { struct ddb *dev = output->port->dev; - if (output->dma) - spin_lock_irq(&output->dma->lock); - - ddbwritel(dev, 0, TS_CONTROL(output)); - if (output->dma) { + spin_lock_irq(&output->dma->lock); ddbwritel(dev, 0, DMA_BUFFER_CONTROL(output->dma)); output->dma->running = 0; spin_unlock_irq(&output->dma->lock); + } else { + ddbwritel(dev, 0, TS_CONTROL(output)); } } @@ -519,22 +525,21 @@ static void ddb_input_stop(struct ddb_input *input) struct ddb *dev = input->port->dev; u32 tag = DDB_LINK_TAG(input->port->lnr); - if (input->dma) - spin_lock_irq(&input->dma->lock); - ddbwritel(dev, 0, tag | TS_CONTROL(input)); if (input->dma) { + spin_lock_irq(&input->dma->lock); ddbwritel(dev, 0, DMA_BUFFER_CONTROL(input->dma)); input->dma->running = 0; spin_unlock_irq(&input->dma->lock); + } else { + ddbwritel(dev, 0, tag | TS_CONTROL(input)); } } -static void ddb_input_start(struct ddb_input *input) +static void __ddb_input_start(struct ddb_input *input) { struct ddb *dev = input->port->dev; if (input->dma) { - spin_lock_irq(&input->dma->lock); input->dma->cbuf = 0; input->dma->coff = 0; input->dma->stat = 0; @@ -557,9 +562,19 @@ static void ddb_input_start(struct ddb_input *input) if (input->port->type == DDB_TUNER_DUMMY) ddbwritel(dev, 0x000fff01, TS_CONTROL2(input)); - if (input->dma) { + if (input->dma) input->dma->running = 1; +} + +static void ddb_input_start(struct ddb_input *input) +{ + + if (input->dma) { + spin_lock_irq(&input->dma->lock); + __ddb_input_start(input); spin_unlock_irq(&input->dma->lock); + } else { + __ddb_input_start(input); } }