I'm not sure I understand what this patch is trying to do. > Added list_del to ipoib_mcast_free() and path_free() to remove the neigh from > the path's neigh_list link list before freeing the memory it consumes. > This makes the code consistent with ipoib_cm_handle_tx_wc() and other > locations where the list_del() preceeds the call to ipoib_neigh_free().
It looks like this is not needed -- in both cases, the code walks a list of neigh structs and frees them, and then frees the list itself a few lines later. So what does it fix to delete the neigh structs from the list? > Similarly, at list_del() now preceeds path_free() to remove the path from > ipoib_dev_priv's path_list. Similarly in ipoib_flush_paths() the code is walking a local remove_list that is on the stack and will be thrown away when the function returns -- why does it matter if we remove the paths from that list before freeing them? > Additionally, the fields neigh->ah and neigh->list were not being initialized > upon allocation. The patch insures that these two remaining fields are > initialized correctly. Again, I don't see how this fixes anything; neigh->list does not need to be initialized, since it is only used to add the neigh struct to other lists (AFAICT). And also AFAICT, every place that allocates a neigh always initializes neigh->ah on every code path. It might be cleaner to set neigh->ah to NULL but I don't see how this could fix something. Pradeep seems to believe that this patch fixes some crashes but I would like to understand the real cause before blindly applying this patch. Which part(s) are the real fix, and what is the race or other bug it fixes? - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
