[openib-general] HP ZX1 and HP IB cards...

Roland Dreier roland at topspin.com
Mon Dec 6 10:59:51 PST 2004


    Grant> Yes - that works. Please commit.

I rewrote things in a way that seems cleaner to me -- what I actually
committed is below.  Please try one more time and make sure this still
fixes the problem.

    Grant> It mthca_create_eq() isn't in the perf path, I think it
    Grant> would be good to enforcement the requirement or at least
    Grant> check for it (e.g. WARN_ON).  I just don't want to make the
    Grant> performance patch longer.

Good idea, I'll do this.

    Grant> this wmb() isn't necessary since set_eq_ci() calls
    Grant> mthca_write64() and the latter *must* enforce wmb() for the
    Grant> MMIO write.

I'm not sure about that... mthca_write64() turns into __raw_writeq,
which may not be ordered.  Even if it were writeq(), the barrier is
after the write (eg on ppc64, the "sync" if after the store
operation).  We want to make sure that the updates to the ownership
bits in the EQ in host memory are complete before doing the MMIO write
to update the HCA's consumer pointer; this avoids the (pretty much
impossible) race where the HCA writes to the EQ entry and then our
ownership update happens later and overwrites the HCA's value.

    Grant> I'd also like to see doorbell[2] go away and just pass the
    Grant> two u32 values to mthca_write64().  Perhaps combine them
    Grant> explicitly instead depending on load/store model of the
    Grant> stack.

That makes sense.  I'll try to come up with an API that avoids shifts
on a 64-bit arch...

Thanks,
  Roland

Index: infiniband/hw/mthca/mthca_cmd.c
===================================================================
--- infiniband/hw/mthca/mthca_cmd.c	(revision 1310)
+++ infiniband/hw/mthca/mthca_cmd.c	(working copy)
@@ -293,6 +293,12 @@
 	complete(&context->done);
 }
 
+void mthca_cmd_complete(struct mthca_dev *dev, int ncomp)
+{
+	while (ncomp--)
+		up(&dev->cmd.event_sem);
+}
+
 static void event_timeout(unsigned long context_ptr)
 {
 	struct mthca_cmd_context *context =
@@ -357,7 +363,6 @@
 	dev->cmd.free_head = context - dev->cmd.context;
 	spin_unlock(&dev->cmd.context_lock);
 
-	up(&dev->cmd.event_sem);
 	return err;
 }
 
Index: infiniband/hw/mthca/mthca_eq.c
===================================================================
--- infiniband/hw/mthca/mthca_eq.c	(revision 1310)
+++ infiniband/hw/mthca/mthca_eq.c	(working copy)
@@ -219,6 +219,7 @@
 	struct mthca_eqe *eqe;
 	int disarm_cqn;
 	int work = 0;
+	int ncmd = 0;
 
 	while (1) {
 		if (!next_eqe_sw(eq))
@@ -274,6 +275,7 @@
 					be16_to_cpu(eqe->event.cmd.token),
 					eqe->event.cmd.status,
 					be64_to_cpu(eqe->event.cmd.out_param));
+			++ncmd;
 			break;
 
 		case MTHCA_EVENT_TYPE_PORT_CHANGE:
@@ -303,6 +305,9 @@
 		set_eq_ci(dev, eq->eqn, eq->cons_index);
 	}
 
+	if (ncmd)
+		mthca_cmd_complete(dev, ncmd);
+
 	eq_req_not(dev, eq->eqn);
 }
 
Index: infiniband/hw/mthca/mthca_cmd.h
===================================================================
--- infiniband/hw/mthca/mthca_cmd.h	(revision 1310)
+++ infiniband/hw/mthca/mthca_cmd.h	(working copy)
@@ -203,10 +203,9 @@
 
 int mthca_cmd_use_events(struct mthca_dev *dev);
 void mthca_cmd_use_polling(struct mthca_dev *dev);
-void mthca_cmd_event(struct mthca_dev *dev,
-		     u16 token,
-		     u8  status,
-		     u64 out_param);
+void mthca_cmd_event(struct mthca_dev *dev, u16 token,
+		     u8  status, u64 out_param);
+void mthca_cmd_complete(struct mthca_dev *dev, int ncomp);
 
 int mthca_SYS_EN(struct mthca_dev *dev, u8 *status);
 int mthca_SYS_DIS(struct mthca_dev *dev, u8 *status);



More information about the general mailing list