Re: "unexpected unlock" when unlocking, conditional, lock in loop

[Josh Triplett <josh@joshtriplett.org>]

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