On Thu, Nov 29, 2012 at 03:47:57PM +0000, David Woodhouse wrote:
> On Thu, 2012-11-29 at 16:09 +0100, Krzysztof Mazur wrote:
> >
> > I don't like two thinks about this patch:
> >
> > - if allos_skb(sizeof(*header), GFP_ATOMIC) at beginning of
> > pclose() fails we will crash
> >
> > - if card wakes up after this timeout we will probably crash too
> >
> > That's why proposed different approach, but it has other problems.
>
> How about this variant on what you suggested. Yes, we can definitely
> remove everything that's in the queue... as long as we use
> skb_queue_walk_safe() instead of skb_queue_walk().
>
> We can use GFP_KERNEL instead of GFP_ATOMIC, which at least reduces the
> likelihood of failing to close the vcc.
>
> We end up waiting *only* if there is a packet which is *currently* being
> DMA'd to the card. And if the card doesn't take that within 5 seconds,
> it almost certainly never will. So I can live with that.
>
Yeah, that shouldn't happen.
> + if (!test_bit(ATM_VF_READY, &vcc->flags))
> + wake_up(&card->param_wq);
> + } else
according to CodingStyle:
+ } else {
> dev_kfree_skb_irq(oldskb);
> - }
+ }
Krzysiek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
