***SPAM*** Re: [ofa-general] [PATCH] IB/IPoIB: Decrease the time that invalid paths stay useless

[Moni Shoua]

Yossi Etigin wrote:
> 
>> @@ -360,12 +360,15 @@ void ipoib_mark_paths_invalid(struct net_device
>> *dev)
>>      spin_lock_irq(&priv->lock);
>>  
>>      list_for_each_entry_safe(path, tp, &priv->path_list, list) {
>> -        ipoib_dbg(priv, "mark path LID 0x%04x GID " IPOIB_GID_FMT "
>> invalid\n",
>> +        ipoib_dbg(priv, "mark path LID 0x%04x GID " IPOIB_GID_FMT "
>> stale\n",
>>              be16_to_cpu(path->pathrec.dlid),
>>              IPOIB_GID_ARG(path->pathrec.dgid));
>> -        path->valid =  0;
>> +        path->stale =  1;
>>      }
>>  
>> +    if (!list_empty(&priv->path_list))
>> +        queue_delayed_work(ipoib_workqueue, &priv->path_refresh_task,
>> +                    round_jiffies_relative(HZ));
>>      spin_unlock_irq(&priv->lock);
>>  }
>>  
> 
> What if there is already an outstanding path query on one
> of the paths you mark stale? ipoib_refresh_paths() will issue another
> query, making it two
> queries on the same path. Then, if you bring the device
> down (call ipoib_flush_paths()) it will wait for completion
> of one query, causing a crash.
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
> 
Thanks. You're right. I think that the change to the patch below should fix what you pointed at
Do you agree?

@@ -551,9 +557,29 @@ static int path_rec_start(struct net_device *dev,
                return path->query_id;
        }

+       path->stale =  0;
        return 0;
 }

+void ipoib_refresh_paths(struct work_struct *work)
+{
+       struct ipoib_dev_priv *priv =
+               container_of(work, struct ipoib_dev_priv, path_refresh_task.work);
+       struct net_device *dev = priv->dev;
+       struct ipoib_path *path, *tp;
+
+       spin_lock_irq(&priv->lock);
+       list_for_each_entry_safe(path, tp, &priv->path_list, list) {
+               ipoib_dbg(priv, "restart path LID 0x%04x GID " IPOIB_GID_FMT "\n",
+                       be16_to_cpu(path->pathrec.dlid),
+                       IPOIB_GID_ARG(path->pathrec.dgid));
+               if (path->stale && !path->query)                 <<<<<<<<<<<<<<<<  CHANGE IS HERE
+                       path_rec_start(dev, path);
+       }
+
+       spin_unlock_irq(&priv->lock);
+}
+
 static void neigh_add_path(struct sk_buff *skb, struct net_device *dev)
 {
        struct ipoib_dev_priv *priv = netdev_priv(dev);