| Message ID | 66AC2AD6-C0FA-4F60-850A-D8C9426184B8@coraid.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sat, Oct 06, 2012 at 08:56:57PM -0500, Ed Cashin wrote: > 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. I did mean precisely that; I don't know why Sparse complains about that syntax. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Oct 6, 2012, at 10:39 PM, Josh Triplett wrote: > On Sat, Oct 06, 2012 at 08:56:57PM -0500, Ed Cashin wrote: ... >> 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. > > I did mean precisely that; I don't know why Sparse complains about that > syntax. Maybe there's a header I need. Searching with cscope and ctags for definitions of "context" doesn't seem to be the right kind of searching. The complaint looks like: CC [M] drivers/block/aoe/demo.o drivers/block/aoe/demo.c:9: warning: `context' attribute directive ignored drivers/block/aoe/demo.c:9: error: expected `,' or `;' before `{' token make[1]: *** [drivers/block/aoe/demo.o] Error 1 make: *** [drivers/block/aoe/aoe.ko] Error 2 ... for this code: 1 #include <linux/netdevice.h> 2 3 static spinlock_t lk; 4 static struct sk_buff_head q; 5 int demofn(void); 6 7 /* enters with lk held */ 8 int demofn(void) __attribute__((context(&lk,1,1))) 9 { 10 struct sk_buff *skb; 11 12 while ((skb = skb_dequeue(&q))) { 13 spin_unlock_irq(&lk); 14 if (dev_queue_xmit(skb) == NET_XMIT_DROP && net_ratelimit()) 15 pr_warn("informative warning\n"); 16 spin_lock_irq(&lk); 17 } 18 return 0; 19 } Thanks.
--- 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