* [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
@ 2015-01-14 19:52 Doug Ledford
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Doug Ledford @ 2015-01-14 19:52 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Erez Shitrit, Or Gerlitz, Doug Ledford
This patch series fixes the multicast join behavior problems introduced
by my previous patchset. In particular, the original code did not use
the send only join code from the multicast thread context, and so it
did not need to restart the multicast thread. After my previous patchset,
it does get called from the thread context, and so the send only join
completion areas need to restart the join thread but they don't. This
patchset makes them do so. It then adds in some cleanups for restarting
the thread, and fixes the fact that one delayed join holds up the entire
list of joins.
v3: Resend because the last send didn't register in patchworks properly
(because the subject-prefix was not on all of the emails, only the
first) and because the Cc: list didn't not pass from cover letter
to patches
v2: Added two new patches, the first creates a helper to restart the
multicast join thread and also adds using it in the two places where
it should have been used but wasn't, the second allows the joins to
proceed around a delayed join instead of stalling everything.
v1: Addressed the usage of the IPOIB_MCAST_RUN flag
Doug Ledford (3):
IB/ipoib: Fix failed multicast joins/sends
IB/ipoib: Add a helper to restart the multicast task
IB/ipoib: make delayed tasks not hold up everything
drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 94 ++++++++++++++++++--------
2 files changed, 66 insertions(+), 29 deletions(-)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3 FIX For-3.19 1/3] IB/ipoib: Fix failed multicast joins/sends
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2015-01-14 19:52 ` Doug Ledford
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 2/3] IB/ipoib: Add a helper to restart the multicast task Doug Ledford
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Doug Ledford @ 2015-01-14 19:52 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Erez Shitrit, Or Gerlitz, Doug Ledford
The usage of IPOIB_MCAST_RUN as a flag is inconsistent. In some places
it is used to mean "our device is administratively allowed to send
multicast joins/leaves/packets" and in other places it means "our
multicast join task thread is currently running and will process your
request if you put it on the queue". However, this latter meaning is in
fact flawed as there is a race condition between the join task testing
the mcast list and finding it empty of remaining work, dropping the
mcast mutex and also the priv->lock spinlock, and clearing the
IPOIB_MCAST_RUN flag. Further, there are numerous locations that use
the flag in the former fashion, and when all tasks complete and the task
thread clears the RUN flag, all of those other locations will fail to
ever again queue any work. This results in the interface coming up fine
initially, but having problems adding new multicast groups after the
first round of groups have all been added and the RUN flag is cleared by
the join task thread when it thinks it is done. To resolve this issue,
convert all locations in the code to treat the RUN flag as an indicator
that the multicast portion of this interface is in fact administratively
up and joins/leaves/sends can be performed. There is no harm (other
than a slight performance penalty) to never clearing this flag and using
it in this fashion as it simply means that a few places that used to
micro-optimize how often this task was queued on a work queue will now
queue the task a few extra times. We can address that suboptimal
behavior in future patches.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index bc50dd0d0e4..91b8fe118ec 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -630,8 +630,6 @@ void ipoib_mcast_join_task(struct work_struct *work)
}
ipoib_dbg_mcast(priv, "successfully joined all multicast groups\n");
-
- clear_bit(IPOIB_MCAST_RUN, &priv->flags);
}
int ipoib_mcast_start_thread(struct net_device *dev)
@@ -641,8 +639,8 @@ int ipoib_mcast_start_thread(struct net_device *dev)
ipoib_dbg_mcast(priv, "starting multicast thread\n");
mutex_lock(&mcast_mutex);
- if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task, 0);
+ set_bit(IPOIB_MCAST_RUN, &priv->flags);
+ queue_delayed_work(priv->wq, &priv->mcast_task, 0);
mutex_unlock(&mcast_mutex);
return 0;
@@ -725,7 +723,7 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid));
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
- if (!test_and_set_bit(IPOIB_MCAST_RUN, &priv->flags))
+ if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
queue_delayed_work(priv->wq, &priv->mcast_task, 0);
}
@@ -951,7 +949,8 @@ void ipoib_mcast_restart_task(struct work_struct *work)
/*
* Restart our join task if needed
*/
- ipoib_mcast_start_thread(dev);
+ if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
+ queue_delayed_work(priv->wq, &priv->mcast_task, 0);
rtnl_unlock();
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V3 FIX For-3.19 2/3] IB/ipoib: Add a helper to restart the multicast task
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 1/3] IB/ipoib: Fix failed multicast joins/sends Doug Ledford
@ 2015-01-14 19:52 ` Doug Ledford
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 3/3] IB/ipoib: make delayed tasks not hold up everything Doug Ledford
2015-01-15 9:19 ` [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow Erez Shitrit
3 siblings, 0 replies; 11+ messages in thread
From: Doug Ledford @ 2015-01-14 19:52 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Erez Shitrit, Or Gerlitz, Doug Ledford
Add a simple helper to do the right thing when restarting
the multicast thread. Call that helper from all the places
that need to restart the thread. Add two places that didn't
restart the thread, but should have.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 46 ++++++++++++++++----------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 91b8fe118ec..cb1e495bd74 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -66,6 +66,13 @@ struct ipoib_mcast_iter {
unsigned int send_only;
};
+static void __ipoib_mcast_continue_join_thread(struct ipoib_dev_priv *priv,
+ int delay)
+{
+ if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
+ queue_delayed_work(priv->wq, &priv->mcast_task, delay);
+}
+
static void ipoib_mcast_free(struct ipoib_mcast *mcast)
{
struct net_device *dev = mcast->dev;
@@ -270,6 +277,7 @@ ipoib_mcast_sendonly_join_complete(int status,
{
struct ipoib_mcast *mcast = multicast->context;
struct net_device *dev = mcast->dev;
+ struct ipoib_dev_priv *priv = netdev_priv(dev);
/*
* We have to take the mutex to force mcast_sendonly_join to
@@ -309,6 +317,7 @@ out:
complete(&mcast->done);
if (status == -ENETRESET)
status = 0;
+ __ipoib_mcast_continue_join_thread(priv, 0);
mutex_unlock(&mcast_mutex);
return status;
}
@@ -360,6 +369,7 @@ static int ipoib_mcast_sendonly_join(struct ipoib_mcast *mcast)
complete(&mcast->done);
ipoib_warn(priv, "ib_sa_join_multicast for sendonly join "
"failed (ret = %d)\n", ret);
+ __ipoib_mcast_continue_join_thread(priv, 0);
} else {
ipoib_dbg_mcast(priv, "no multicast record for %pI6, starting "
"sendonly join\n", mcast->mcmember.mgid.raw);
@@ -431,8 +441,7 @@ static int ipoib_mcast_join_complete(int status,
if (!status) {
mcast->backoff = 1;
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task, 0);
+ __ipoib_mcast_continue_join_thread(priv, 0);
/*
* Defer carrier on work to priv->wq to avoid a
@@ -454,6 +463,13 @@ static int ipoib_mcast_join_complete(int status,
mcast->backoff *= 2;
if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
+ /*
+ * XXX - This is wrong. *Our* join failed, but because the
+ * join thread does the joins in a serial fashion, if there
+ * are any joins behind ours waiting to complete, they should
+ * not be subjected to our backoff delay.
+ */
+ __ipoib_mcast_continue_join_thread(priv, mcast->backoff * HZ);
}
out:
spin_lock_irq(&priv->lock);
@@ -463,9 +479,6 @@ out:
complete(&mcast->done);
if (status == -ENETRESET)
status = 0;
- if (status && test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task,
- mcast->backoff * HZ);
spin_unlock_irq(&priv->lock);
mutex_unlock(&mcast_mutex);
@@ -532,10 +545,13 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast,
mcast->backoff *= 2;
if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
-
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task,
- mcast->backoff * HZ);
+ /*
+ * XXX - This is wrong. *Our* join failed, but because the
+ * join thread does the joins in a serial fashion, if there
+ * are any joins behind ours waiting to complete, they should
+ * not be subjected to our backoff delay.
+ */
+ __ipoib_mcast_continue_join_thread(priv, mcast->backoff * HZ);
}
mutex_unlock(&mcast_mutex);
}
@@ -573,9 +589,7 @@ void ipoib_mcast_join_task(struct work_struct *work)
if (!broadcast) {
ipoib_warn(priv, "failed to allocate broadcast group\n");
mutex_lock(&mcast_mutex);
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task,
- HZ);
+ __ipoib_mcast_continue_join_thread(priv, HZ);
mutex_unlock(&mcast_mutex);
return;
}
@@ -723,8 +737,7 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid));
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task, 0);
+ __ipoib_mcast_continue_join_thread(priv, 0);
}
if (!mcast->ah) {
@@ -947,10 +960,9 @@ void ipoib_mcast_restart_task(struct work_struct *work)
ipoib_mcast_free(mcast);
}
/*
- * Restart our join task if needed
+ * Restart our join task thread if needed
*/
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
- queue_delayed_work(priv->wq, &priv->mcast_task, 0);
+ __ipoib_mcast_continue_join_thread(priv, 0);
rtnl_unlock();
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V3 FIX For-3.19 3/3] IB/ipoib: make delayed tasks not hold up everything
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 1/3] IB/ipoib: Fix failed multicast joins/sends Doug Ledford
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 2/3] IB/ipoib: Add a helper to restart the multicast task Doug Ledford
@ 2015-01-14 19:52 ` Doug Ledford
2015-01-15 9:19 ` [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow Erez Shitrit
3 siblings, 0 replies; 11+ messages in thread
From: Doug Ledford @ 2015-01-14 19:52 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Erez Shitrit, Or Gerlitz, Doug Ledford
Currently, the multicast join thread only processes one task at a
time. It does this by having itself be scheduled as a delayed work
item, when it runs, it finds one, and only one, piece of work to
process, it then kicks that off via either the normal join process
or the sendonly join process, and then it immediately exits. Both
of those process chains are responsible for restarting the task
when they have completed their specific action. This makes the entire
join process serial with only one join supposedly ever outstanding at
a time.
However, if we fail a join, and we need to initiate a backoff delay,
that delay holds up the entire join process for all joins, not just
the failed join. So modify the design such that we can have joins
in delay, and the multicast thread will ignore them until their time
comes around, and then we can process the rest of the queue without
waiting for the delayed items.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 85 +++++++++++++++++---------
2 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 8ba80a6d3a4..c79dcd5ee8a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -154,6 +154,7 @@ struct ipoib_mcast {
unsigned long created;
unsigned long backoff;
+ unsigned long delay_until;
unsigned long flags;
unsigned char logcount;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index cb1e495bd74..9291b2d569e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -67,10 +67,34 @@ struct ipoib_mcast_iter {
};
static void __ipoib_mcast_continue_join_thread(struct ipoib_dev_priv *priv,
+ struct ipoib_mcast *mcast,
int delay)
{
- if (test_bit(IPOIB_MCAST_RUN, &priv->flags))
+ if (test_bit(IPOIB_MCAST_RUN, &priv->flags)) {
+ /*
+ * Mark this mcast for its delay and set a timer to kick the
+ * thread when the delay completes
+ */
+ if (mcast && delay) {
+ mcast->backoff *= 2;
+ if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
+ mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
+ mcast->delay_until = jiffies + (mcast->backoff * HZ);
+ queue_delayed_work(priv->wq, &priv->mcast_task,
+ mcast->backoff * HZ);
+ } else if (delay) {
+ /* Special case of retrying after a failure to
+ * allocate the broadcast multicast group, wait
+ * 1 second and try again
+ */
+ queue_delayed_work(priv->wq, &priv->mcast_task, HZ);
+ }
+ /*
+ * But also rekick the thread immediately for any other
+ * tasks in queue behind this one
+ */
queue_delayed_work(priv->wq, &priv->mcast_task, delay);
+ }
}
static void ipoib_mcast_free(struct ipoib_mcast *mcast)
@@ -110,6 +134,7 @@ static struct ipoib_mcast *ipoib_mcast_alloc(struct net_device *dev,
mcast->dev = dev;
mcast->created = jiffies;
+ mcast->delay_until = jiffies;
mcast->backoff = 1;
INIT_LIST_HEAD(&mcast->list);
@@ -309,6 +334,10 @@ ipoib_mcast_sendonly_join_complete(int status,
dev_kfree_skb_any(skb_dequeue(&mcast->pkt_queue));
}
netif_tx_unlock_bh(dev);
+ } else {
+ /* Join completed, so reset any backoff parameters */
+ mcast->backoff = 1;
+ mcast->delay_until = jiffies;
}
out:
clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
@@ -317,7 +346,7 @@ out:
complete(&mcast->done);
if (status == -ENETRESET)
status = 0;
- __ipoib_mcast_continue_join_thread(priv, 0);
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
mutex_unlock(&mcast_mutex);
return status;
}
@@ -369,7 +398,7 @@ static int ipoib_mcast_sendonly_join(struct ipoib_mcast *mcast)
complete(&mcast->done);
ipoib_warn(priv, "ib_sa_join_multicast for sendonly join "
"failed (ret = %d)\n", ret);
- __ipoib_mcast_continue_join_thread(priv, 0);
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
} else {
ipoib_dbg_mcast(priv, "no multicast record for %pI6, starting "
"sendonly join\n", mcast->mcmember.mgid.raw);
@@ -441,7 +470,8 @@ static int ipoib_mcast_join_complete(int status,
if (!status) {
mcast->backoff = 1;
- __ipoib_mcast_continue_join_thread(priv, 0);
+ mcast->delay_until = jiffies;
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
/*
* Defer carrier on work to priv->wq to avoid a
@@ -460,16 +490,8 @@ static int ipoib_mcast_join_complete(int status,
}
}
- mcast->backoff *= 2;
- if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
- mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
- /*
- * XXX - This is wrong. *Our* join failed, but because the
- * join thread does the joins in a serial fashion, if there
- * are any joins behind ours waiting to complete, they should
- * not be subjected to our backoff delay.
- */
- __ipoib_mcast_continue_join_thread(priv, mcast->backoff * HZ);
+ /* Requeue this join task with a backoff delay */
+ __ipoib_mcast_continue_join_thread(priv, mcast, 1);
}
out:
spin_lock_irq(&priv->lock);
@@ -541,17 +563,8 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast,
complete(&mcast->done);
ret = PTR_ERR(mcast->mc);
ipoib_warn(priv, "ib_sa_join_multicast failed, status %d\n", ret);
-
- mcast->backoff *= 2;
- if (mcast->backoff > IPOIB_MAX_BACKOFF_SECONDS)
- mcast->backoff = IPOIB_MAX_BACKOFF_SECONDS;
- /*
- * XXX - This is wrong. *Our* join failed, but because the
- * join thread does the joins in a serial fashion, if there
- * are any joins behind ours waiting to complete, they should
- * not be subjected to our backoff delay.
- */
- __ipoib_mcast_continue_join_thread(priv, mcast->backoff * HZ);
+ /* Requeue this join task with a backoff delay */
+ __ipoib_mcast_continue_join_thread(priv, mcast, 1);
}
mutex_unlock(&mcast_mutex);
}
@@ -589,7 +602,13 @@ void ipoib_mcast_join_task(struct work_struct *work)
if (!broadcast) {
ipoib_warn(priv, "failed to allocate broadcast group\n");
mutex_lock(&mcast_mutex);
- __ipoib_mcast_continue_join_thread(priv, HZ);
+ /*
+ * Restart us after a 1 second delay to retry
+ * creating our broadcast group and attaching to
+ * it. Until this succeeds, this ipoib dev is
+ * completely stalled (multicast wise).
+ */
+ __ipoib_mcast_continue_join_thread(priv, NULL, 1);
mutex_unlock(&mcast_mutex);
return;
}
@@ -623,7 +642,9 @@ void ipoib_mcast_join_task(struct work_struct *work)
list_for_each_entry(mcast, &priv->multicast_list, list) {
if (IS_ERR_OR_NULL(mcast->mc) &&
!test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags) &&
- !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags)) {
+ !test_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags) &&
+ (mcast->backoff == 1 ||
+ time_after_eq(jiffies, mcast->delay_until))) {
/* Found the next unjoined group */
break;
}
@@ -632,7 +653,11 @@ void ipoib_mcast_join_task(struct work_struct *work)
mutex_unlock(&mcast_mutex);
if (&mcast->list == &priv->multicast_list) {
- /* All done */
+ /* All done, unless we have delayed work from
+ * backoff retransmissions, but we will get
+ * restarted when the time is right, so we are
+ * done for now
+ */
break;
}
@@ -737,7 +762,7 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid));
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
- __ipoib_mcast_continue_join_thread(priv, 0);
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
}
if (!mcast->ah) {
@@ -962,7 +987,7 @@ void ipoib_mcast_restart_task(struct work_struct *work)
/*
* Restart our join task thread if needed
*/
- __ipoib_mcast_continue_join_thread(priv, 0);
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
rtnl_unlock();
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 3/3] IB/ipoib: make delayed tasks not hold up everything Doug Ledford
@ 2015-01-15 9:19 ` Erez Shitrit
[not found] ` <DBXPR05MB067182B666A7F24EE23DC8ADB64E0-c2uBOMY7wQg6ranl7A9sk9qRiQSDpxhJvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
3 siblings, 1 reply; 11+ messages in thread
From: Erez Shitrit @ 2015-01-15 9:19 UTC (permalink / raw)
To: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: Amir Vadai, Eyal Perry, Or Gerlitz
Hi Doug,
Thank you for the quick response.
Now I can see 2 issues, that I want to draw your attention to:
1. if there is a mcg that the driver failed to join, the mc_task enters to endless loop of re-queue, and the log will be full with the next messages:
[682560.569826] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.580136] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.590364] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.600504] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.610627] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.620769] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.631082] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.640835] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
[682560.651033] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.660758] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
[682560.670923] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.680676] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
[682560.690898] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
[682560.700630] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
around 100 times a sec.
2. IPv6 still doesn't work for me, at the same case where it is not the first mcg in the list.
Thanks, Erez
-----Original Message-----
From: Doug Ledford [mailto:dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
Sent: Wednesday, January 14, 2015 9:53 PM
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: Amir Vadai; Eyal Perry; Erez Shitrit; Or Gerlitz; Doug Ledford
Subject: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
This patch series fixes the multicast join behavior problems introduced by my previous patchset. In particular, the original code did not use the send only join code from the multicast thread context, and so it did not need to restart the multicast thread. After my previous patchset, it does get called from the thread context, and so the send only join completion areas need to restart the join thread but they don't. This patchset makes them do so. It then adds in some cleanups for restarting the thread, and fixes the fact that one delayed join holds up the entire list of joins.
v3: Resend because the last send didn't register in patchworks properly
(because the subject-prefix was not on all of the emails, only the
first) and because the Cc: list didn't not pass from cover letter
to patches
v2: Added two new patches, the first creates a helper to restart the
multicast join thread and also adds using it in the two places where
it should have been used but wasn't, the second allows the joins to
proceed around a delayed join instead of stalling everything.
v1: Addressed the usage of the IPOIB_MCAST_RUN flag
Doug Ledford (3):
IB/ipoib: Fix failed multicast joins/sends
IB/ipoib: Add a helper to restart the multicast task
IB/ipoib: make delayed tasks not hold up everything
drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 94 ++++++++++++++++++--------
2 files changed, 66 insertions(+), 29 deletions(-)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
[not found] ` <DBXPR05MB067182B666A7F24EE23DC8ADB64E0-c2uBOMY7wQg6ranl7A9sk9qRiQSDpxhJvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2015-01-15 15:24 ` Doug Ledford
[not found] ` <1421335460.2484.21.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Doug Ledford @ 2015-01-15 15:24 UTC (permalink / raw)
To: Erez Shitrit
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Amir Vadai,
Eyal Perry, Or Gerlitz
[-- Attachment #1: Type: text/plain, Size: 4878 bytes --]
On Thu, 2015-01-15 at 09:19 +0000, Erez Shitrit wrote:
> Hi Doug,
>
> Thank you for the quick response.
>
> Now I can see 2 issues, that I want to draw your attention to:
>
> 1. if there is a mcg that the driver failed to join, the mc_task enters to endless loop of re-queue, and the log will be full with the next messages:
> [682560.569826] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.580136] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.590364] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.600504] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.610627] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.620769] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.631082] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.640835] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> [682560.651033] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.660758] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> [682560.670923] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.680676] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> [682560.690898] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> [682560.700630] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
>
> around 100 times a sec.
OK, this looks like the send only joins that fail are not setting a
fallback properly or something like that. There is a separate bug that
I've isolated that I'm going to fix, then I we can see if that fix
effects things here, as it very well might.
> 2. IPv6 still doesn't work for me, at the same case where it is not the first mcg in the list.
Can you give me some sort of instructions on how to replicate your
testing? Things are working for me here, but I don't have a complex
IPv6 setup and mine may be too simple to reproduce what you are seeing.
> Thanks, Erez
>
> -----Original Message-----
> From: Doug Ledford [mailto:dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Wednesday, January 14, 2015 9:53 PM
> To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> Cc: Amir Vadai; Eyal Perry; Erez Shitrit; Or Gerlitz; Doug Ledford
> Subject: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
>
> This patch series fixes the multicast join behavior problems introduced by my previous patchset. In particular, the original code did not use the send only join code from the multicast thread context, and so it did not need to restart the multicast thread. After my previous patchset, it does get called from the thread context, and so the send only join completion areas need to restart the join thread but they don't. This patchset makes them do so. It then adds in some cleanups for restarting the thread, and fixes the fact that one delayed join holds up the entire list of joins.
>
> v3: Resend because the last send didn't register in patchworks properly
> (because the subject-prefix was not on all of the emails, only the
> first) and because the Cc: list didn't not pass from cover letter
> to patches
>
> v2: Added two new patches, the first creates a helper to restart the
> multicast join thread and also adds using it in the two places where
> it should have been used but wasn't, the second allows the joins to
> proceed around a delayed join instead of stalling everything.
>
> v1: Addressed the usage of the IPOIB_MCAST_RUN flag
>
> Doug Ledford (3):
> IB/ipoib: Fix failed multicast joins/sends
> IB/ipoib: Add a helper to restart the multicast task
> IB/ipoib: make delayed tasks not hold up everything
>
> drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 94 ++++++++++++++++++--------
> 2 files changed, 66 insertions(+), 29 deletions(-)
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
[not found] ` <1421335460.2484.21.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2015-01-15 20:08 ` Erez Shitrit
[not found] ` <54B81E2B.9030101-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Erez Shitrit @ 2015-01-15 20:08 UTC (permalink / raw)
To: Doug Ledford, Erez Shitrit
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Amir Vadai,
Eyal Perry, Or Gerlitz
On 1/15/2015 5:24 PM, Doug Ledford wrote:
> On Thu, 2015-01-15 at 09:19 +0000, Erez Shitrit wrote:
>> Hi Doug,
>>
>> Thank you for the quick response.
>>
>> Now I can see 2 issues, that I want to draw your attention to:
>>
>> 1. if there is a mcg that the driver failed to join, the mc_task enters to endless loop of re-queue, and the log will be full with the next messages:
>> [682560.569826] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.580136] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.590364] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.600504] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.610627] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.620769] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.631082] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.640835] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
>> [682560.651033] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.660758] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
>> [682560.670923] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.680676] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
>> [682560.690898] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
>> [682560.700630] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
>>
>> around 100 times a sec.
> OK, this looks like the send only joins that fail are not setting a
> fallback properly or something like that. There is a separate bug that
> I've isolated that I'm going to fix, then I we can see if that fix
> effects things here, as it very well might.
>
>> 2. IPv6 still doesn't work for me, at the same case where it is not the first mcg in the list.
> Can you give me some sort of instructions on how to replicate your
> testing? Things are working for me here, but I don't have a complex
> IPv6 setup and mine may be too simple to reproduce what you are seeing.
I don't have a complex setup, i have 2 devices, and i do a regular ping6
from device with the full series in it, to some other device. nothing
special, the only thing i can say that in the list there is one sendonly
mcg (
ff12:601b:ffff:0000:0000:0000:0000:0016) that is at the first place in the list.
anyway, i think it connected to the first issue,because it at some endless loop with the first mcg, it doesn't have the chance to handle the other mcg's.
>
>> Thanks, Erez
>>
>> -----Original Message-----
>> From: Doug Ledford [mailto:dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
>> Sent: Wednesday, January 14, 2015 9:53 PM
>> To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>> Cc: Amir Vadai; Eyal Perry; Erez Shitrit; Or Gerlitz; Doug Ledford
>> Subject: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
>>
>> This patch series fixes the multicast join behavior problems introduced by my previous patchset. In particular, the original code did not use the send only join code from the multicast thread context, and so it did not need to restart the multicast thread. After my previous patchset, it does get called from the thread context, and so the send only join completion areas need to restart the join thread but they don't. This patchset makes them do so. It then adds in some cleanups for restarting the thread, and fixes the fact that one delayed join holds up the entire list of joins.
>>
>> v3: Resend because the last send didn't register in patchworks properly
>> (because the subject-prefix was not on all of the emails, only the
>> first) and because the Cc: list didn't not pass from cover letter
>> to patches
>>
>> v2: Added two new patches, the first creates a helper to restart the
>> multicast join thread and also adds using it in the two places where
>> it should have been used but wasn't, the second allows the joins to
>> proceed around a delayed join instead of stalling everything.
>>
>> v1: Addressed the usage of the IPOIB_MCAST_RUN flag
>>
>> Doug Ledford (3):
>> IB/ipoib: Fix failed multicast joins/sends
>> IB/ipoib: Add a helper to restart the multicast task
>> IB/ipoib: make delayed tasks not hold up everything
>>
>> drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
>> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 94 ++++++++++++++++++--------
>> 2 files changed, 66 insertions(+), 29 deletions(-)
>>
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
[not found] ` <54B81E2B.9030101-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
@ 2015-01-15 20:27 ` Doug Ledford
[not found] ` <1421353631.2484.31.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Doug Ledford @ 2015-01-15 20:27 UTC (permalink / raw)
To: Erez Shitrit
Cc: Erez Shitrit, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Amir Vadai,
Eyal Perry, Or Gerlitz
[-- Attachment #1: Type: text/plain, Size: 6514 bytes --]
On Thu, 2015-01-15 at 22:08 +0200, Erez Shitrit wrote:
> On 1/15/2015 5:24 PM, Doug Ledford wrote:
> > On Thu, 2015-01-15 at 09:19 +0000, Erez Shitrit wrote:
> >> Hi Doug,
> >>
> >> Thank you for the quick response.
> >>
> >> Now I can see 2 issues, that I want to draw your attention to:
> >>
> >> 1. if there is a mcg that the driver failed to join, the mc_task enters to endless loop of re-queue, and the log will be full with the next messages:
> >> [682560.569826] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.580136] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.590364] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.600504] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.610627] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.620769] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.631082] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.640835] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> >> [682560.651033] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.660758] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> >> [682560.670923] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.680676] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> >> [682560.690898] ib0: no multicast record for ff12:601b:ffff:0000:0000:0000:0000:0016, starting sendonly join
> >> [682560.700630] ib0: sendonly multicast join failed for ff12:601b:ffff:0000:0000:0000:0000:0016, status -22
> >>
> >> around 100 times a sec.
> > OK, this looks like the send only joins that fail are not setting a
> > fallback properly or something like that. There is a separate bug that
> > I've isolated that I'm going to fix, then I we can see if that fix
> > effects things here, as it very well might.
> >
> >> 2. IPv6 still doesn't work for me, at the same case where it is not the first mcg in the list.
> > Can you give me some sort of instructions on how to replicate your
> > testing? Things are working for me here, but I don't have a complex
> > IPv6 setup and mine may be too simple to reproduce what you are seeing.
> I don't have a complex setup, i have 2 devices, and i do a regular ping6
> from device with the full series in it, to some other device. nothing
> special, the only thing i can say that in the list there is one sendonly
> mcg (
>
> ff12:601b:ffff:0000:0000:0000:0000:0016) that is at the first place in the list.
> anyway, i think it connected to the first issue,because it at some endless loop with the first mcg, it doesn't have the chance to handle the other mcg's.
OK, well, I have this all working here. However, there is still one
lingering issue (not reported on this thread yet) that needs addressed,
so I don't yet consider the patchset complete. But, I'll post it as it
stands so far for you to try your tests again.
The outstanding issue is that it is possible for ipoib_mcast_flush_dev
to race with ipoib_mcast_join and cause ipoib_mcast_join to oops. It's
rare, I've only seen it once, but I was afraid that it was possible by
looking at the code, and now I have confirmation that it is indeed
possible. So, it needs to be fixed.
> >
> >> Thanks, Erez
> >>
> >> -----Original Message-----
> >> From: Doug Ledford [mailto:dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> >> Sent: Wednesday, January 14, 2015 9:53 PM
> >> To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> >> Cc: Amir Vadai; Eyal Perry; Erez Shitrit; Or Gerlitz; Doug Ledford
> >> Subject: [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow
> >>
> >> This patch series fixes the multicast join behavior problems introduced by my previous patchset. In particular, the original code did not use the send only join code from the multicast thread context, and so it did not need to restart the multicast thread. After my previous patchset, it does get called from the thread context, and so the send only join completion areas need to restart the join thread but they don't. This patchset makes them do so. It then adds in some cleanups for restarting the thread, and fixes the fact that one delayed join holds up the entire list of joins.
> >>
> >> v3: Resend because the last send didn't register in patchworks properly
> >> (because the subject-prefix was not on all of the emails, only the
> >> first) and because the Cc: list didn't not pass from cover letter
> >> to patches
> >>
> >> v2: Added two new patches, the first creates a helper to restart the
> >> multicast join thread and also adds using it in the two places where
> >> it should have been used but wasn't, the second allows the joins to
> >> proceed around a delayed join instead of stalling everything.
> >>
> >> v1: Addressed the usage of the IPOIB_MCAST_RUN flag
> >>
> >> Doug Ledford (3):
> >> IB/ipoib: Fix failed multicast joins/sends
> >> IB/ipoib: Add a helper to restart the multicast task
> >> IB/ipoib: make delayed tasks not hold up everything
> >>
> >> drivers/infiniband/ulp/ipoib/ipoib.h | 1 +
> >> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 94 ++++++++++++++++++--------
> >> 2 files changed, 66 insertions(+), 29 deletions(-)
> >>
> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> >> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH FIX For-3.19 v3 4/6] IB/ipoib: Handle -ENETRESET properly in our callback
[not found] ` <1421353631.2484.31.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2015-01-15 20:59 ` Doug Ledford
[not found] ` <f0d0830949eb57626baa20a1d311b8e4b4f7768d.1421355536.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Doug Ledford @ 2015-01-15 20:59 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Or Gerlitz, Erez Shitrit, Doug Ledford
When the core layer calls our callback with ENETRESET as the error, we
clear the status to 0 before returning indicating that we are going to
handle the error ourselves. This causes the core layer to not free the
mcast->mc structure, but we are releasing our reference to it by
clearing mcast->mc. So, preserve our reference to the multicast
structure so when we next run ipoib_mcast_dev_flush, it will be able
to properly release the mcast->mc entry at the right time in the right
way.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 9291b2d569e..e81ed117c30 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -315,8 +315,10 @@ ipoib_mcast_sendonly_join_complete(int status,
mutex_lock(&mcast_mutex);
/* We trap for port events ourselves. */
- if (status == -ENETRESET)
+ if (status == -ENETRESET) {
+ status = 0;
goto out;
+ }
if (!status)
status = ipoib_mcast_join_finish(mcast, &multicast->rec);
@@ -344,8 +346,6 @@ out:
if (status)
mcast->mc = NULL;
complete(&mcast->done);
- if (status == -ENETRESET)
- status = 0;
__ipoib_mcast_continue_join_thread(priv, NULL, 0);
mutex_unlock(&mcast_mutex);
return status;
@@ -462,8 +462,10 @@ static int ipoib_mcast_join_complete(int status,
mutex_lock(&mcast_mutex);
/* We trap for port events ourselves. */
- if (status == -ENETRESET)
+ if (status == -ENETRESET) {
+ status = 0;
goto out;
+ }
if (!status)
status = ipoib_mcast_join_finish(mcast, &multicast->rec);
@@ -499,8 +501,6 @@ out:
if (status)
mcast->mc = NULL;
complete(&mcast->done);
- if (status == -ENETRESET)
- status = 0;
spin_unlock_irq(&priv->lock);
mutex_unlock(&mcast_mutex);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH FIX For-3.19 v3 5/6] IB/ipoib: don't restart our thread on ENETRESET
[not found] ` <f0d0830949eb57626baa20a1d311b8e4b4f7768d.1421355536.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2015-01-15 20:59 ` Doug Ledford
2015-01-15 20:59 ` [PATCH FIX For-3.19 v3 6/6] IB/ipoib: remove unneeded locks Doug Ledford
1 sibling, 0 replies; 11+ messages in thread
From: Doug Ledford @ 2015-01-15 20:59 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Or Gerlitz, Erez Shitrit, Doug Ledford
Make the send only join completion handler behave like the normal join
completion handler in that we should restart the multicast join thread
whenever our join completes with either an error or success, but don't
restart the thread if we got a net reset event, let ipoib_event queue
up a device flush, which will then call ipoib_mcast_dev_flush to remove
all of the current mcast entries, and ipoib_mcase_restart_task to check
out current mcast entry list against the netdev list of mcast entries
we are supposed to have and requeue the ones we need to get back.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index e81ed117c30..34727254339 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -336,17 +336,18 @@ ipoib_mcast_sendonly_join_complete(int status,
dev_kfree_skb_any(skb_dequeue(&mcast->pkt_queue));
}
netif_tx_unlock_bh(dev);
+ __ipoib_mcast_continue_join_thread(priv, mcast, 1);
} else {
/* Join completed, so reset any backoff parameters */
mcast->backoff = 1;
mcast->delay_until = jiffies;
+ __ipoib_mcast_continue_join_thread(priv, NULL, 0);
}
out:
clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
if (status)
mcast->mc = NULL;
complete(&mcast->done);
- __ipoib_mcast_continue_join_thread(priv, NULL, 0);
mutex_unlock(&mcast_mutex);
return status;
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH FIX For-3.19 v3 6/6] IB/ipoib: remove unneeded locks
[not found] ` <f0d0830949eb57626baa20a1d311b8e4b4f7768d.1421355536.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-15 20:59 ` [PATCH FIX For-3.19 v3 5/6] IB/ipoib: don't restart our thread on ENETRESET Doug Ledford
@ 2015-01-15 20:59 ` Doug Ledford
1 sibling, 0 replies; 11+ messages in thread
From: Doug Ledford @ 2015-01-15 20:59 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-DgEjT+Ai2ygdnm+yROfE0A
Cc: Amir Vadai, Eyal Perry, Or Gerlitz, Erez Shitrit, Doug Ledford
During the various work I've done on this, some extra locking has crept
in when things weren't working right. This is one of those spots.
Remove the unneeded spinlocks. The mutex is enough to protect against
what we need to protect against.
Signed-off-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 34727254339..9df74f831e2 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -497,12 +497,10 @@ static int ipoib_mcast_join_complete(int status,
__ipoib_mcast_continue_join_thread(priv, mcast, 1);
}
out:
- spin_lock_irq(&priv->lock);
clear_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
if (status)
mcast->mc = NULL;
complete(&mcast->done);
- spin_unlock_irq(&priv->lock);
mutex_unlock(&mcast_mutex);
return status;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-01-15 20:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-14 19:52 [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow Doug Ledford
[not found] ` <cover.1421264928.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 1/3] IB/ipoib: Fix failed multicast joins/sends Doug Ledford
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 2/3] IB/ipoib: Add a helper to restart the multicast task Doug Ledford
2015-01-14 19:52 ` [PATCH V3 FIX For-3.19 3/3] IB/ipoib: make delayed tasks not hold up everything Doug Ledford
2015-01-15 9:19 ` [PATCH V3 FIX For-3.19 0/3] IB/ipoib: Fix multicast join flow Erez Shitrit
[not found] ` <DBXPR05MB067182B666A7F24EE23DC8ADB64E0-c2uBOMY7wQg6ranl7A9sk9qRiQSDpxhJvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2015-01-15 15:24 ` Doug Ledford
[not found] ` <1421335460.2484.21.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-15 20:08 ` Erez Shitrit
[not found] ` <54B81E2B.9030101-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2015-01-15 20:27 ` Doug Ledford
[not found] ` <1421353631.2484.31.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-15 20:59 ` [PATCH FIX For-3.19 v3 4/6] IB/ipoib: Handle -ENETRESET properly in our callback Doug Ledford
[not found] ` <f0d0830949eb57626baa20a1d311b8e4b4f7768d.1421355536.git.dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-01-15 20:59 ` [PATCH FIX For-3.19 v3 5/6] IB/ipoib: don't restart our thread on ENETRESET Doug Ledford
2015-01-15 20:59 ` [PATCH FIX For-3.19 v3 6/6] IB/ipoib: remove unneeded locks Doug Ledford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).