On Mon, Jan 09, 2006 at 04:43:08AM +0000, Paul Janzen wrote:
> Paul Janzen <[EMAIL PROTECTED]> writes:
> > In mv643xx_eth_start_xmit:
> > [...]
> > spin_lock_irqsave(&mp->lock, flags);
> > [...]
> > /* Since hardware can't handle unaligned fragments smaller
> > * than 9 bytes, if we find any, we linearize the skb
> > * and start again. */
> > [...]
> > skb_linearize(skb, GFP_ATOMIC);
> > [...]
> >
> > which ends up calling kunmap_skb_frag(vaddr), which, when
> > CONFIG_HIGHMEM=y, calls local_bh_enable with interrupts off.
>
> A patch for this problem is enclosed. I believe it also solves a
> potential deadlock if skb_linearize() returns -ENOMEM.
>
> Signed-off-by: Paul Janzen <[EMAIL PROTECTED]>
Thank you Paul. Good find and fix. I have one question below.
> --- a/drivers/net/mv643xx_eth.c 2005-12-24 15:47:48.000000000 -0800
> +++ b/drivers/net/mv643xx_eth.c 2006-01-08 20:30:25.000000000 -0800
> @@ -1093,6 +1093,27 @@ static int mv643xx_poll(struct net_devic
> }
> #endif
>
> +/* Hardware can't handle unaligned fragments smaller than 9 bytes.
> + * This helper function detects that case. When I've seen it, it's
> + * always been the first frag (probably near the end of the page), but
> + * we check all frags to be safe.
> + */
By the way, the above comment is mine, but since then, I have seen small,
unaligned fragments beyond the first frag, so I'll remove the last
sentence.
> +static inline unsigned int has_tiny_unaligned_frags(struct sk_buff *skb)
> +{
> + unsigned int frag;
> + skb_frag_t *fragp;
> +
> + for (frag = 0; frag < skb_shinfo(skb)->nr_frags; frag++) {
> + fragp = &skb_shinfo(skb)->frags[frag];
> + if (fragp->size <= 8 && fragp->page_offset & 0x7)
> + return 1;
> +
> + }
> + return 0;
> +}
> +
> +
> /*
> * mv643xx_eth_start_xmit
> *
> @@ -1136,12 +1157,22 @@ static int mv643xx_eth_start_xmit(struct
> return 1;
> }
>
> + if (has_tiny_unaligned_frags(skb)) {
> + if ((skb_linearize(skb, GFP_ATOMIC) != 0)
> + || has_tiny_unaligned_frags(skb)) {
There is no way that skb_linearize can return without removing all frags
from the skb. So the extra call to has_tiny_unaligned_frags() is
unnecessary, right?
-Dale
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
