IPoIB: Fix multicast race between canceling and completing

ipoib_mcast_stop_thread currently tests mcast->query and if it is
NULL, does not perform wait_for_completion on the mcast and frees the
mcast object directly.

However, since both operations are done without locking, it is
possible that ipoib_mcast_join_complete is in progress on this mcast
object and has set mcast->query to NULL already.

Solve this by:
- taking priv->lock before we change mcast->query in ipoib_mcast_join_complete,
  and keeping it until we no longer need the mcast object
- taking priv->lock around mcast->query test in ipoib_mcast_stop_thread

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>
This commit is contained in:
Michael S. Tsirkin 2006-03-02 11:07:47 -08:00 committed by Roland Dreier
parent 54d07e2a1e
commit 9acf6a8570
1 changed files with 12 additions and 3 deletions

View File

@ -437,9 +437,11 @@ static void ipoib_mcast_join_complete(int status,
if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS) if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS; mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
mutex_lock(&mcast_mutex);
spin_lock_irq(&priv->lock);
mcast->query = NULL; mcast->query = NULL;
mutex_lock(&mcast_mutex);
if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) { if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) {
if (status == -ETIMEDOUT) if (status == -ETIMEDOUT)
queue_work(ipoib_workqueue, &priv->mcast_task); queue_work(ipoib_workqueue, &priv->mcast_task);
@ -448,6 +450,7 @@ static void ipoib_mcast_join_complete(int status,
mcast->backoff * HZ); mcast->backoff * HZ);
} else } else
complete(&mcast->done); complete(&mcast->done);
spin_unlock_irq(&priv->lock);
mutex_unlock(&mcast_mutex); mutex_unlock(&mcast_mutex);
return; return;
@ -635,21 +638,27 @@ int ipoib_mcast_stop_thread(struct net_device *dev, int flush)
if (flush) if (flush)
flush_workqueue(ipoib_workqueue); flush_workqueue(ipoib_workqueue);
spin_lock_irq(&priv->lock);
if (priv->broadcast && priv->broadcast->query) { if (priv->broadcast && priv->broadcast->query) {
ib_sa_cancel_query(priv->broadcast->query_id, priv->broadcast->query); ib_sa_cancel_query(priv->broadcast->query_id, priv->broadcast->query);
priv->broadcast->query = NULL; priv->broadcast->query = NULL;
spin_unlock_irq(&priv->lock);
ipoib_dbg_mcast(priv, "waiting for bcast\n"); ipoib_dbg_mcast(priv, "waiting for bcast\n");
wait_for_completion(&priv->broadcast->done); wait_for_completion(&priv->broadcast->done);
} } else
spin_unlock_irq(&priv->lock);
list_for_each_entry(mcast, &priv->multicast_list, list) { list_for_each_entry(mcast, &priv->multicast_list, list) {
spin_lock_irq(&priv->lock);
if (mcast->query) { if (mcast->query) {
ib_sa_cancel_query(mcast->query_id, mcast->query); ib_sa_cancel_query(mcast->query_id, mcast->query);
mcast->query = NULL; mcast->query = NULL;
spin_unlock_irq(&priv->lock);
ipoib_dbg_mcast(priv, "waiting for MGID " IPOIB_GID_FMT "\n", ipoib_dbg_mcast(priv, "waiting for MGID " IPOIB_GID_FMT "\n",
IPOIB_GID_ARG(mcast->mcmember.mgid)); IPOIB_GID_ARG(mcast->mcmember.mgid));
wait_for_completion(&mcast->done); wait_for_completion(&mcast->done);
} } else
spin_unlock_irq(&priv->lock);
} }
return 0; return 0;