Messages in this thread | | | Date | Thu, 29 Nov 2012 17:28:18 +0100 | From | Krzysztof Mazur <> | Subject | Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc |
| |
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
| |