From patchwork Sun Oct 7 01:56:57 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ed Cashin X-Patchwork-Id: 1560271 Return-Path: X-Original-To: patchwork-linux-sparse@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 8E25CDFE80 for ; Sun, 7 Oct 2012 02:02:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751343Ab2JGCCF (ORCPT ); Sat, 6 Oct 2012 22:02:05 -0400 Received: from server505g.appriver.com ([98.129.35.12]:2992 "EHLO server505.appriver.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751082Ab2JGCCE convert rfc822-to-8bit (ORCPT ); Sat, 6 Oct 2012 22:02:04 -0400 X-Greylist: delayed 301 seconds by postgrey-1.27 at vger.kernel.org; Sat, 06 Oct 2012 22:02:04 EDT X-Note-AR-ScanTimeLocal: 10/6/2012 8:56:59 PM X-Policy: GLOBAL - coraid.com X-Policy: GLOBAL - coraid.com X-Primary: ecashin@coraid.com X-Note: This Email was scanned by AppRiver SecureTide X-ALLOW: @coraid.com ALLOWED X-Virus-Scan: V- X-Note: Spam Tests Failed: X-Country-Path: UNKNOWN->UNITED STATES->UNITED STATES X-Note-Sending-IP: 98.129.35.1 X-Note-Reverse-DNS: smtp.exg5.exghost.com X-Note-Return-Path: ecashin@coraid.com X-Note: User Rule Hits: X-Note: Global Rule Hits: G321 G322 G323 G324 G328 G329 G340 G436 X-Note: Encrypt Rule Hits: X-Note: Mail Class: ALLOWEDSENDER X-Note: Headers Injected Received: from [98.129.35.1] (HELO smtp.exg5.exghost.com) by server505.appriver.com (CommuniGate Pro SMTP 5.4.4) with ESMTPS id 326453664; Sat, 06 Oct 2012 20:56:59 -0500 Received: from MBX24.exg5.exghost.com ([169.254.1.193]) by HT09-e5.exg5.exghost.com ([98.129.23.242]) with mapi; Sat, 6 Oct 2012 20:56:59 -0500 From: Ed Cashin To: Josh Triplett CC: "linux-sparse@vger.kernel.org" Date: Sat, 6 Oct 2012 20:56:57 -0500 Subject: Re: "unexpected unlock" when unlocking, conditional, lock in loop Thread-Topic: "unexpected unlock" when unlocking, conditional, lock in loop Thread-Index: Ac2kLwkGH2NvxXBJRsakXeh0qktjtA== Message-ID: <66AC2AD6-C0FA-4F60-850A-D8C9426184B8@coraid.com> References: <1349552876.20963@cat.he.net> <20121006202102.GA28179@leaf> In-Reply-To: <20121006202102.GA28179@leaf> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org On Oct 6, 2012, at 4:21 PM, Josh Triplett wrote: > On Sat, Oct 06, 2012 at 12:47:56PM -0700, ecashin@coraid.com wrote: ... >> static spinlock_t lk; >> static struct sk_buff_head q; >> int demofn(void); >> >> /* enters and returns with lk held */ >> int demofn(void) >> { >> struct sk_buff *skb; >> >> while ((skb = skb_dequeue(&q))) { >> spin_unlock_irq(&lk); >> #if 1 >> dev_queue_xmit(skb); >> #else >> if (dev_queue_xmit(skb) == NET_XMIT_DROP && net_ratelimit()) >> pr_warn("informative warning\n"); >> #endif >> spin_lock_irq(&lk); >> } >> return 0; >> } > > Sparse should *always* generate a context warning here; odd that it does > not in both cases. I see. > The right fix: annotate the function to explicitly say it starts and > stops with that lock held. That should make the warning go away in > both cases. OK. From the sparse man page section on context, along with include/linux/compiler.h, it sounds like the way to do exactly that would be something unusual: int demofn(void) __attribute__((context(&lk,1,1))) ... but using that in demo.c causes sparse to warn me that it's ignoring that attribute, so I doubt that can be what you mean. Were you thinking of changes like the ones below? These changes stop the warnings, but it bothers me that they imply that the function is called without the lock held, __attribute__((context(x,0,1))), when that's not really true. [ecashin@marino linux]$ diff -u drivers/block/aoe/demo.c.20121006 drivers/block/aoe/demo.c [ecashin@marino linux]$ Thanks very much. --- drivers/block/aoe/demo.c.20121006 2012-10-06 21:12:11.769751545 -0400 +++ drivers/block/aoe/demo.c 2012-10-06 21:51:01.453595477 -0400 @@ -5,10 +5,11 @@ int demofn(void); /* enters with lk held */ -int demofn(void) +int demofn(void) __acquires(&lk) { struct sk_buff *skb; + __acquire(lk); while ((skb = skb_dequeue(&q))) { spin_unlock_irq(&lk); #if 0