On Oct 6, 2012, at 4:21 PM, Josh Triplett wrote:
> On Sat, Oct 06, 2012 at 12:47:56PM -0700, ecashin@xxxxxxxxxx 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
--- 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
[ecashin@marino linux]$
Thanks very much.
--
Ed Cashin
ecashin@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
