This is the second version after some of Roland's comments.
One comments however is still pending.
-----------------------------------------------------
This patch solves a race between work elements that are carried out after an
event occurs. When SM address handle becomes invalid and needs an update it is
handled by a work in the global workqueue. On the other hand this event is also
handled in ib_ipoib by queuing a work in the ipoib_workqueue that does mcast
join.
Although queuing is in the right order, it is done to 2 different workqueues
and so
there is no guarantee that the first to be queued is the first to be executed.
The patch sets the SM address handle to NULL and until update_sm_ah() is
called,
any request that needs sm_ah is replied with -EAGAIN return status.
For consumers, the patch doesn't make things worse. Before the patch, MADS are
sent to the
wrong SM so the request gets lost. Consumers can be improved if they examine
the return
code and respond to EAGAIN properly but even without an improvement the
situation
is not getting worse and in some cases it gets better.
If ib_sa_event() is called in during consumers work (e.g. ib_sa_path_rec_get())
and after
the check for NULL SM address handle the result would be as before the patch
and without a
risk of dereferencing a NULL pinter.
Signed-off-by: Moni Levy <[EMAIL PROTECTED]>
Signed-off-by: Moni Shoua <[EMAIL PROTECTED]>
---
drivers/infiniband/core/sa_query.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/core/sa_query.c
b/drivers/infiniband/core/sa_query.c
index cf474ec..7224bd1 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -361,7 +361,7 @@ static void update_sm_ah(struct work_struct *work)
{
struct ib_sa_port *port =
container_of(work, struct ib_sa_port, update_task);
- struct ib_sa_sm_ah *new_ah, *old_ah;
+ struct ib_sa_sm_ah *new_ah;
struct ib_port_attr port_attr;
struct ib_ah_attr ah_attr;
@@ -397,12 +397,9 @@ static void update_sm_ah(struct work_struct *work)
}
spin_lock_irq(&port->ah_lock);
- old_ah = port->sm_ah;
port->sm_ah = new_ah;
spin_unlock_irq(&port->ah_lock);
- if (old_ah)
- kref_put(&old_ah->ref, free_sm_ah);
}
static void ib_sa_event(struct ib_event_handler *handler, struct ib_event
*event)
@@ -413,9 +410,20 @@ static void ib_sa_event(struct ib_event_handler *handler,
struct ib_event *event
event->event == IB_EVENT_PKEY_CHANGE ||
event->event == IB_EVENT_SM_CHANGE ||
event->event == IB_EVENT_CLIENT_REREGISTER) {
- struct ib_sa_device *sa_dev;
- sa_dev = container_of(handler, typeof(*sa_dev), event_handler);
-
+ unsigned long flags;
+ struct ib_sa_device *sa_dev =
+ container_of(handler, typeof(*sa_dev), event_handler);
+ struct ib_sa_port *port =
+ &sa_dev->port[event->element.port_num -
sa_dev->start_port];
+ struct ib_sa_sm_ah *sm_ah;
+
+ spin_lock_irqsave(&port->ah_lock, flags);
+ sm_ah = port->sm_ah;
+ port->sm_ah = NULL;
+ spin_unlock_irqrestore(&port->ah_lock, flags);
+
+ if (sm_ah)
+ kref_put(&port->sm_ah->ref, free_sm_ah);
schedule_work(&sa_dev->port[event->element.port_num -
sa_dev->start_port].update_task);
}
@@ -519,6 +527,10 @@ static int alloc_mad(struct ib_sa_query *query, gfp_t
gfp_mask)
unsigned long flags;
spin_lock_irqsave(&query->port->ah_lock, flags);
+ if (!query->port->sm_ah) {
+ spin_unlock_irqrestore(&query->port->ah_lock, flags);
+ return -EAGAIN;
+ }
kref_get(&query->port->sm_ah->ref);
query->sm_ah = query->port->sm_ah;
spin_unlock_irqrestore(&query->port->ah_lock, flags);
_______________________________________________
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
