[ewg] [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled.
This is necessary because, in a multicore environment, a race between
uverbs async handler and destroy QP could occur.
Signed-off-by: Stefan Roscher stefan.roscher at de.ibm.com
---
We are not sure if this should be fixed in the driver or in uverbs itself.
Roland, what's your opinion about this?
drivers/infiniband/hw/ehca/ehca_classes.h |2 ++
drivers/infiniband/hw/ehca/ehca_irq.c |4
drivers/infiniband/hw/ehca/ehca_qp.c |5 +
3 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/infiniband/hw/ehca/ehca_classes.h
b/drivers/infiniband/hw/ehca/ehca_classes.h
index 00bab60..1e9e99a 100644
--- a/drivers/infiniband/hw/ehca/ehca_classes.h
+++ b/drivers/infiniband/hw/ehca/ehca_classes.h
@@ -192,6 +192,8 @@ struct ehca_qp {
int mtu_shift;
u32 message_count;
u32 packet_count;
+ atomic_t nr_events; /* events seen */
+ wait_queue_head_t wait_completion;
};
#define IS_SRQ(qp) (qp-ext_type == EQPT_SRQ)
diff --git a/drivers/infiniband/hw/ehca/ehca_irq.c
b/drivers/infiniband/hw/ehca/ehca_irq.c
index ca5eb0c..ce1ab05 100644
--- a/drivers/infiniband/hw/ehca/ehca_irq.c
+++ b/drivers/infiniband/hw/ehca/ehca_irq.c
@@ -204,6 +204,8 @@ static void qp_event_callback(struct ehca_shca *shca, u64
eqe,
read_lock(ehca_qp_idr_lock);
qp = idr_find(ehca_qp_idr, token);
+ if (qp)
+ atomic_inc(qp-nr_events);
read_unlock(ehca_qp_idr_lock);
if (!qp)
@@ -223,6 +225,8 @@ static void qp_event_callback(struct ehca_shca *shca, u64
eqe,
if (fatal qp-ext_type == EQPT_SRQBASE)
dispatch_qp_event(shca, qp, IB_EVENT_QP_LAST_WQE_REACHED);
+ if (atomic_dec_and_test(qp-nr_events))
+ wake_up(qp-wait_completion);
return;
}
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c
b/drivers/infiniband/hw/ehca/ehca_qp.c
index 18fba92..d550200 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -566,6 +566,8 @@ static struct ehca_qp *internal_create_qp(
return ERR_PTR(-ENOMEM);
}
+ atomic_set(my_qp-nr_events, 0);
+ init_waitqueue_head(my_qp-wait_completion);
spin_lock_init(my_qp-spinlock_s);
spin_lock_init(my_qp-spinlock_r);
my_qp-qp_type = qp_type;
@@ -1934,6 +1936,9 @@ static int internal_destroy_qp(struct ib_device *dev,
struct ehca_qp *my_qp,
idr_remove(ehca_qp_idr, my_qp-token);
write_unlock_irqrestore(ehca_qp_idr_lock, flags);
+/* now wait until all pending events have completed */
+ wait_event(my_qp-wait_completion, !atomic_read(my_qp-nr_events));
+
h_ret = hipz_h_destroy_qp(shca-ipz_hca_handle, my_qp);
if (h_ret != H_SUCCESS) {
ehca_err(dev, hipz_h_destroy_qp() failed h_ret=%li
--
1.5.5
___
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled.
We are not sure if this should be fixed in the driver or in uverbs itself. Roland, what's your opinion about this? Would be nice to be able to fix it in uverbs but I don't see how. In particular a kernel consumer has to have the same guarantee that no async events will come in after destroy QP returns. And I don't see any way generic code can provide a guarantee about what low-level driver code may do internally. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled.
On Wednesday 07 May 2008 17:32:03 Roland Dreier wrote: We are not sure if this should be fixed in the driver or in uverbs itself. Roland, what's your opinion about this? Would be nice to be able to fix it in uverbs but I don't see how. In particular a kernel consumer has to have the same guarantee that no async events will come in after destroy QP returns. And I don't see any way generic code can provide a guarantee about what low-level driver code may do internally. I agree, that's why I posted the driver fix first. So, will you apply it next? Regards Stefan ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled.
I agree, that's why I posted the driver fix first. Glad we agree :) I thought about it a little more and really convinced myself that there is no good way for generic code to handle this race. So, will you apply it next? Yes, will apply it shortly. - R. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
[ewg] [PATCH ofed-1.3.1] IB/ipath - update ipath driver to include fixes being sent to 2.6.26
This patch includes a number of fixes for OFED-1.3.1 which have been or soon will be submitted to 2.6.26. They can be pulled or accessed at: git://git.openfabrics.org/~ralphc/linux-2.6/.git commit 21f5def290d9840ee16973d00c12aad4cf542bb5 Author: Ralph Campbell (QLogic) [EMAIL PROTECTED] Date: Wed May 7 11:14:55 2008 -0700 IB/ipath - rename patch so it is applied before the others. commit b1a9e3e6a81ac19f4241166e5853ae4e6a4d4c8f Author: Ralph Campbell (QLogic) [EMAIL PROTECTED] Date: Wed May 7 10:58:05 2008 -0700 IB/ipath - update ipath driver to include fixes being sent to 2.6.26 This patch includes a number of fixes which have been or soon will be submitted to 2.6.26. Signed-off-by: Ralph Campbell [EMAIL PROTECTED] ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
[ewg] Interesting Herbal Offers
While browsing our extensive product line we ensure your digital security. www railweather com We ensure your full satisfaction in the utmost safest fashion. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] [PATCH] IB/ehca: Protect QP against destroying until all async events for it are handled.
So I applied this, but thinking about it further, do you (theoretically at least) have the same problem with CQ and SRQ async events? - R. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
[ewg] Re: [ofa-general] Re: [PATCH] Request For Comments:
I'm just trying to define the scope of the issue here... so is there any conceivable real-life situation where neither a 0B read nor a 0B write would work, and the connection setup will have to use a 0B send? i'm not sure what you mean by real-life. For the rnics we have: nes - requires 0b write cxgb3 - requires 0b read amso1100 - won't work in p2p mode So there are none that I know of that require a send for this. I guess my question was whether we expect to ever need to worry about the 0B send case, or whether it's just theoretical. If no current NICs have a problem with read or write, and future NICs will be built to a future MPA spec, then it seems we don't have to worry about what happens if a 0B send is done as part of connection setup. The spurious CQE on connection failure and the private data breakage are serious obviously. The interoperability issues of this stuff seem pretty painful to me. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
[ewg] Re: [ofa-general] Re: [PATCH] Request For Comments:
Sean Hefty wrote: nes - requires 0b write cxgb3 - requires 0b read amso1100 - won't work in p2p mode I'm assuming by requires that you, uhm, mean requires, and nes couldn't do 0b reads, or cxgb3 0b writes. Well, I'm not sure about nes. But cxgb3 cannot deal with receiving a 0B write for the RTR because the FW doesn't see incoming writes, nor does the driver. nes may be able to request a 0b read, but I what I meant was they currently use a 0B write and not a read. So its possible to reduce the complexity if we just mandate 0B read for RTR. But it makes sense in my mind to allow the other message types... Its is painful. But without anything, you cannot run OMPI, IMPI or HPMPI on a iwarp cluster with mixed vendor rnics... Is there any requirement at the receiving side, versus the initiating side? That is, just because nes issues a 0b write, does the receiving HW care if a read or write shows up? Or is this restriction on both sides? The requirement is mostly driven from the receiving side. For cxgb3 it is anyway... The receiving side, ie the side that issues the rdma_accept will tell the sending side what RTR message to send, if any. So the MPA exchange will look like this: client sends MPA Start request with private data saying i can send an RTR if you want it. server moves connection into RDMA mode server sends MPA Start response with lets do RTR and send me X where X could be 0B write, 0B read request or 0B send. client moves connection into RDMA mode client sends X and then enables SQ processing (or indicate ESTABLISHED) Once server gets X it can enable SQ processing (or indicate ESTABLISHED) If X was a 0B read request, server sends 0B read response. Steve ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
