Skip to contentSkip to conversation

IB/ehca: Reduce number of spin_lock/spin_unlocks in ehca_poll_eqs

PATCH In review Anton Blanchard 3 participants 3 messages Nov 27, 2009 – Nov 30, 2009 1 message not in the archive InfiniBand

lore
Share & tools
lore

Anton Blanchard

author
#0 lore raw

PATCH 1/1 IB/ehca: Reduce number of spin_lock/spin_unlocks in ehca_poll_eqs

ftrace function_graph shows ehca_poll_eqs taking and dropping a spinlock a lot:

 16)               |          .ehca_poll_eqs() {
 16)   0.492 us    |            ._spin_lock();
 16)   0.504 us    |            ._spin_lock_irqsave();
 16)   0.502 us    |            ._spin_unlock_irqrestore();
 16)   0.490 us    |            ._spin_lock_irqsave();
 16)   0.488 us    |            ._spin_unlock_irqrestore();
 16)   0.488 us    |            ._spin_lock_irqsave();
 16)   0.488 us    |            ._spin_unlock_irqrestore();
 16)   0.490 us    |            ._spin_lock_irqsave();
 16)   0.492 us    |            ._spin_unlock_irqrestore();

Since it's all the same spinlock, we can just take and drop it once.

Signed-off-by Anton Blanchard <[email protected]>

---

drivers/infiniband/hw/ehca/ehca_main.c +5 -3
Index: linux.trees.git/drivers/infiniband/hw/ehca/ehca_main.c
===================================================================
@@ -957,15 +957,17 @@ void ehca_poll_eqs(unsigned long data)
 			int max = 3;
 			volatile u64 q_ofs, q_ofs2;
 			unsigned long flags;
+
 			spin_lock_irqsave(&eq->spinlock, flags);
+
 			q_ofs = eq->ipz_queue.current_q_offset;
-			spin_unlock_irqrestore(&eq->spinlock, flags);
 			do {
-				spin_lock_irqsave(&eq->spinlock, flags);
 				q_ofs2 = eq->ipz_queue.current_q_offset;
-				spin_unlock_irqrestore(&eq->spinlock, flags);
 				max--;
 			} while (q_ofs == q_ofs2 && max > 0);
+
+			spin_unlock_irqrestore(&eq->spinlock, flags);
+
 			if (q_ofs == q_ofs2)
 				ehca_process_eq(shca, 0);
 		}

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw
Quoted 5 lines
drivers/infiniband/hw/ehca/ehca_main.c +2 -0
+
 			spin_lock_irqsave(&eq->spinlock, flags);
+
 			q_ofs = eq->ipz_queue.current_q_offset;

drivers/infiniband/hw/ehca/ehca_main.c void ehca_poll_eqs(unsigned long data) · line 970

Quoted 10 lines
drivers/infiniband/hw/ehca/ehca_main.c +3 -3
-			spin_unlock_irqrestore(&eq->spinlock, flags);
 			do {
-				spin_lock_irqsave(&eq->spinlock, flags);
 				q_ofs2 = eq->ipz_queue.current_q_offset;
-				spin_unlock_irqrestore(&eq->spinlock, flags);
 				max--;
 			} while (q_ofs == q_ofs2 && max > 0);
+
+			spin_unlock_irqrestore(&eq->spinlock, flags);
+

Does this loop have to drop the lock to give the EQ code a chance to update the queue pointer? If I understand this correctly, the EQ polling code is just trying to have a fallback if interrupts don't work...

- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): adad430y450.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org

#3 lore raw

Hi Roland,
Yes, you're right. The loop below is a fallback to detect possibly missing irq events
by checking the eq pointer, which can be changed by irq handler. @Anton, for above reason we can not remove the lock/unlock within the loop. Thanks,
Nam

Roland Dreier <[email protected]> wrote on 30.11.2009 06:26:35:

Quoted 3 lines

[image removed]

Re: [PATCH] IB/ehca: Reduce number of spin_lock/spin_unlocks in

ehca_poll_eqs

Quoted 35 lines

Roland Dreier

to:

Anton Blanchard

30.11.2009 06:26

Cc:

Hoang-Nam Nguyen, Christoph Raisch, Stefan Roscher, Alexander Schmidt, Joachim Fenkes, linux-rdma

Quoted 4 lines
drivers/infiniband/hw/ehca/ehca_main.c +2 -0
+
          spin_lock_irqsave(&eq->spinlock, flags);
+
          q_ofs = eq->ipz_queue.current_q_offset;

drivers/infiniband/hw/ehca/ehca_main.c void ehca_poll_eqs(unsigned long data) · line 970

Quoted 10 lines
drivers/infiniband/hw/ehca/ehca_main.c +3 -3
-         spin_unlock_irqrestore(&eq->spinlock, flags);
          do {
-            spin_lock_irqsave(&eq->spinlock, flags);
             q_ofs2 = eq->ipz_queue.current_q_offset;
-            spin_unlock_irqrestore(&eq->spinlock, flags);
             max--;
          } while (q_ofs == q_ofs2 && max > 0);
+
+         spin_unlock_irqrestore(&eq->spinlock, flags);
+

Does this loop have to drop the lock to give the EQ code a chance to update the queue pointer? If I understand this correctly, the EQ polling code is just trying to have a fallback if interrupts don't work...

Quoted 1 line

- R.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html