On Thu, Dec 09, 2004 at 04:05:36PM -0800, Grant Grundler wrote:
> Roland,
> Would you consider committing the set_ci patch and backing the ncmd patch out?
Roland,
Attached is a patch that does three things (LART me for combining them,
but I can split them up later if you want):
o reverse most of the mthca_cmd_complete() patch
o add set_ci logic so consumer index is updated inside the event loop
o eliminate "work" variable (nicely simplifies the code).
If I understood the code correctly, "work" was always being set
if next_eqe_sw(eq) was non-zero.
It is so expensive to get to the interrupt handler in the first place
that if nothing needs to be done:
a) the problem is NOT in the interrupt handler.
b) the spurious set_eq_ci() is light weight enough to be in the noise
I do understand the wmb()/set_eq_ci() disturb the PCI data flows but
we shouldn't be seeing spurious interrupts either. If they are happening
often enough to disturb performance, something else is wrong.
grant
Index: hw/mthca/mthca_cmd.c
===================================================================
--- hw/mthca/mthca_cmd.c (revision 1317)
+++ hw/mthca/mthca_cmd.c (working copy)
@@ -293,12 +293,6 @@
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 =
@@ -363,6 +357,7 @@
dev->cmd.free_head = context - dev->cmd.context;
spin_unlock(&dev->cmd.context_lock);
+ up(&dev->cmd.event_sem);
return err;
}
Index: hw/mthca/mthca_eq.c
===================================================================
--- hw/mthca/mthca_eq.c (revision 1317)
+++ hw/mthca/mthca_eq.c (working copy)
@@ -218,15 +218,10 @@
{
struct mthca_eqe *eqe;
int disarm_cqn;
- int work = 0;
- int ncmd = 0;
- while (1) {
- if (!next_eqe_sw(eq))
- break;
-
+ while (next_eqe_sw(eq)) {
+ int set_ci = 0;
eqe = get_eqe(eq, eq->cons_index);
- work = 1;
switch (eqe->type) {
case MTHCA_EVENT_TYPE_COMP:
@@ -275,7 +270,11 @@
be16_to_cpu(eqe->event.cmd.token),
eqe->event.cmd.status,
be64_to_cpu(eqe->event.cmd.out_param));
- ++ncmd;
+ /* cmd_event() may add more commands.
+ * The card will think the queue has overflowed if
+ * we don't tell it we've been processing events.
+ */
+ set_ci = 1;
break;
case MTHCA_EVENT_TYPE_PORT_CHANGE:
@@ -298,25 +297,25 @@
set_eqe_hw(eq, eq->cons_index);
eq->cons_index = (eq->cons_index + 1) & (eq->nent - 1);
- }
- if (work) {
- /*
- * This barrier makes sure that all updates to
- * ownership bits done by set_eqe_hw() hit memory
- * before the consumer index is updated. set_eq_ci()
- * allows the HCA to possibly write more EQ entries,
- * and we want to avoid the exceedingly unlikely
- * possibility of the HCA writing an entry and then
- * having set_eqe_hw() overwrite the owner field.
- */
- wmb();
- set_eq_ci(dev, eq->eqn, eq->cons_index);
+ if (set_ci) {
+ wmb(); /* see comment below */
+ set_eq_ci(dev, eq->eqn, eq->cons_index);
+ set_ci = 0;
+ }
}
- if (ncmd)
- mthca_cmd_complete(dev, ncmd);
-
+ /*
+ * This barrier makes sure that all updates to
+ * ownership bits done by set_eqe_hw() hit memory
+ * before the consumer index is updated. set_eq_ci()
+ * allows the HCA to possibly write more EQ entries,
+ * and we want to avoid the exceedingly unlikely
+ * possibility of the HCA writing an entry and then
+ * having set_eqe_hw() overwrite the owner field.
+ */
+ wmb();
+ set_eq_ci(dev, eq->eqn, eq->cons_index);
eq_req_not(dev, eq->eqn);
}
Index: hw/mthca/mthca_cmd.h
===================================================================
--- hw/mthca/mthca_cmd.h (revision 1317)
+++ hw/mthca/mthca_cmd.h (working copy)
@@ -205,7 +205,6 @@
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_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);
_______________________________________________
openib-general mailing list
[EMAIL PROTECTED]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
