From patchwork Thu Aug 25 14:46:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Utkin X-Patchwork-Id: 36665 X-Patchwork-Delegate: hverkuil@xs4all.nl Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bcwZp-0001HM-Vr; Thu, 25 Aug 2016 15:28:22 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-5) with esmtp id 1bcwZn-0005DL-8T; Thu, 25 Aug 2016 17:28:21 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933792AbcHYP2F (ORCPT + 1 other); Thu, 25 Aug 2016 11:28:05 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:35687 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933796AbcHYP2D (ORCPT ); Thu, 25 Aug 2016 11:28:03 -0400 Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id EDB0B205C8; Thu, 25 Aug 2016 10:46:06 -0400 (EDT) Received: from web1 ([10.202.2.211]) by compute7.internal (MEProxy); Thu, 25 Aug 2016 10:46:07 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=fastmail.com; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=Hahqa/9DPbU/1Tgg4D6MhIIxIQ8=; b=jtb8y3 iRHnXZ5l+ahBJgL2i1Qdwj+07IHmxXferfWur7KOEIXEOnXx9EIPaH/X4mkqmSKJ /9EbLyO7fiKfqeO8z+ZIM5kl618bM9a4I5EtfBXNbwFtrt8MXggzB4K4vMdtW8oG aCmEQfoQEuNWnKMgP7xJEeYdIvQqFR9nmrts8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=Hahqa/9DPbU/1Tg g4D6MhIIxIQ8=; b=d+rd/UZzr6dcVoXVHcvxYLacMdELS37VuUMvoQCRKX0g+/w ckpdd80bQP0X6AK30+C9axw2k/Z2aB+cnygSyOpPDDCsbwmiKJ7t9VTmGaE5Gq6d 5FQszlR0shtALatQ7KnHkt1OU5jraOaEy8p4xjfEvYmjT/2SROG0sTAmL2E8= Received: by mailuser.nyi.internal (Postfix, from userid 99) id C292F6A4BD; Thu, 25 Aug 2016 10:46:06 -0400 (EDT) Message-Id: <1472136366.1628907.705944009.76C1558A@webmail.messagingengine.com> X-Sasl-Enc: 6phudXRe+A4MXjImNo5Kpd4b9WOnk7L8tsDzYv8e6Mxp 1472136366 From: Andrey Utkin To: Mauro Carvalho Chehab Cc: Linux Media Mailing List , Mauro Carvalho Chehab , Bluecherry Maintainers , Andrey Utkin , Mauro Carvalho Chehab MIME-Version: 1.0 X-Mailer: MessagingEngine.com Webmail Interface - ajax-5ae55197 Subject: Re: [PATCH 1/2] [media] tw5864-core: remove double irq lock code Date: Thu, 25 Aug 2016 17:46:06 +0300 In-Reply-To: References: Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.8.25.152417 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DKIM_SIGNATURE 0, IN_REP_TO 0, LEGITIMATE_NEGATE 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, NO_URI_HTTPS 0, REFERENCES 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, WEBMAIL_SOURCE 0, WEBMAIL_XMAILER 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CC_NAME 0, __CC_NAME_DIFF_FROM_ACC 0, __CC_REAL_NAMES 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __DATE_TZ_RU 0, __FORWARDED_MSG 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_STRUCTURE_1 0, __PHISH_SPEAR_STRUCTURE_2 0, __REFERENCES 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_END 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __TO_NAME 0, __TO_NAME_DIFF_FROM_ACC 0, __TO_REAL_NAMES 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0' For some reason (maybe "unlisted recipients"?), my reply didn't get through to maillists. Resending my reply. On Wed, Aug 24, 2016, at 19:30, Mauro Carvalho Chehab wrote: > As warned by smatch: > drivers/media/pci/tw5864/tw5864-core.c:160 tw5864_h264_isr() error: double lock 'irqsave:flags' > drivers/media/pci/tw5864/tw5864-core.c:174 tw5864_h264_isr() error: double unlock 'irqsave:flags' > > Remove the IRQ duplicated lock. > > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/pci/tw5864/tw5864-core.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/media/pci/tw5864/tw5864-core.c > b/drivers/media/pci/tw5864/tw5864-core.c > index 440cd7bb8d04..e3d884e963c0 100644 > --- a/drivers/media/pci/tw5864/tw5864-core.c > +++ b/drivers/media/pci/tw5864/tw5864-core.c > @@ -157,12 +157,10 @@ static void tw5864_h264_isr(struct tw5864_dev *dev) > > cur_frame = next_frame; > > - spin_lock_irqsave(&input->slock, flags); > input->frame_seqno++; > input->frame_gop_seqno++; > if (input->frame_gop_seqno >= input->gop) > input->frame_gop_seqno = 0; > - spin_unlock_irqrestore(&input->slock, flags); > } else { > dev_err(&dev->pci->dev, > "Skipped frame on input %d because all buffers busy\n", Thank you very much for catching this issue, but NACK on the patch. These two lock operations are on different spinlocks. One for device, another one for input (a subordinate entity of device). What is superfluous here is second _irqsave. Also "flags" variable reuse is wrong. So what would be right, in my opinion, is the following (going to submit this patch): "Skipped frame on input %d because all buffers busy\n", --- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/media/pci/tw5864/tw5864-core.c b/drivers/media/pci/tw5864/tw5864-core.c index 440cd7b..1d43b96 100644 --- a/drivers/media/pci/tw5864/tw5864-core.c +++ b/drivers/media/pci/tw5864/tw5864-core.c @@ -157,12 +157,12 @@ static void tw5864_h264_isr(struct tw5864_dev *dev) cur_frame = next_frame; - spin_lock_irqsave(&input->slock, flags); + spin_lock(&input->slock); input->frame_seqno++; input->frame_gop_seqno++; if (input->frame_gop_seqno >= input->gop) input->frame_gop_seqno = 0; - spin_unlock_irqrestore(&input->slock, flags); + spin_unlock(&input->slock); } else { dev_err(&dev->pci->dev,