On Tuesday 03 February 2009 19:56, Yossi Etigin wrote:
> I think it comes from unicast_arp_send.
> Consider this scenario:
> - paths are flushed (opensm up/down).
> - unicast_arp_send() is called with a path in priv->path_list. path->valid is 
> 0.
> - path_rec_start() fails with -EAGAIN (-11) because alloc_mad() fails - no sm 
> ah (yet)
>   (see the prints just before the panic).
> - unicast_arp_send calls() path_free().
> - path memory is overwritten.
> - __ipoib_dev_flush() is called again.
> - mark_paths_invalid() tries to iterate over priv->path_list and gets kernel 
> panic
>   because path->list became invalid.

I think you are right.

In unicast_arp_send, we have the following code:
        path = __path_find(dev, phdr->hwaddr + 4);
        if (!path || !path->valid) {
                if (!path)
                        path = path_rec_create(dev, phdr->hwaddr + 4);
                if (path) {
                        /* put pseudoheader back on for next time */
                        skb_push(skb, sizeof *phdr);
                        __skb_queue_tail(&path->queue, skb);

                        if (path_rec_start(dev, path)) {
                                spin_unlock(&priv->lock);
                                path_free(dev, path);
                                return;
                        } else
                                __path_add(dev, path);
                } else {
                        ++dev->stats.tx_dropped;
                        dev_kfree_skb_any(skb);
                }

                spin_unlock(&priv->lock);
                return;
        }

It was originally written without the path->valid check in the "if", and so was 
based on the path record
being allocated within the "if".  In this case, the path record was not yet 
inserted into the path list.
When you added the "valid" processing, you did not take this into account.

You need code something like the following:

        path = __path_find(dev, phdr->hwaddr + 4);
        if (!path || !path->valid) {
                int had_path = 0;
                if (!path)
                        path = path_rec_create(dev, phdr->hwaddr + 4);
                else
                    had_path = 1;
                if (path) {
                        /* put pseudoheader back on for next time */
                        skb_push(skb, sizeof *phdr);
                        __skb_queue_tail(&path->queue, skb);

                        if (path_rec_start(dev, path)) {
                                if (had_path)
                                        /* detach from path list here under 
spinlock */
                                spin_unlock(&priv->lock);
                                path_free(dev, path);
                                return;
                        } else if (!had_path)
                                __path_add(dev, path);
                } else {
                        ++dev->stats.tx_dropped;
                        dev_kfree_skb_any(skb);
                }

                spin_unlock(&priv->lock);
                return;
        }

- Jack
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to