[19/27] habanalabs: capture interrupt timestamp in handler

Commit Message

Oded Gabbay Feb. 12, 2023, 8:44 p.m. UTC

From: Ofir Bitton <[e-mail]>

In order for interrupt timestamp to be more accurate we should
capture it during the interrupt handling rather than in threaded
irq context.

Signed-off-by: Ofir Bitton <[e-mail]>
Reviewed-by: Oded Gabbay <[e-mail]>
Signed-off-by: Oded Gabbay <[e-mail]>
---
 drivers/accel/habanalabs/common/habanalabs.h | 2 ++
 drivers/accel/habanalabs/common/irq.c        | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Stanislaw Gruszka Feb. 16, 2023, 2:39 p.m. UTC | #1

On Sun, Feb 12, 2023 at 10:44:46PM +0200, Oded Gabbay wrote:
> From: Ofir Bitton <[e-mail]>
> 
> In order for interrupt timestamp to be more accurate we should
> capture it during the interrupt handling rather than in threaded
> irq context.

Why this is important to have this timestamp more accurate ? 
What actually 'more accurate' mean in this context ?

Regards
Stanislaw

Ofir Bitton Feb. 19, 2023, 12:42 p.m. UTC | #2

On Thu, Feb 16, 2023 16:39 PM, Stanislaw Gruszka <[e-mail]> wrote:
> On Sun, Feb 12, 2023 at 10:44:46PM +0200, Oded Gabbay wrote:
> > From: Ofir Bitton <[e-mail]>
> >
> > In order for interrupt timestamp to be more accurate we should capture
> > it during the interrupt handling rather than in threaded irq context.
> 
> Why this is important to have this timestamp more accurate ?

I agree that the time diff between taking the timestamp in the interrupt handler vs taking it in the
threaded irq context is negligible.

Having said that it is still important as we would like to have the same timestamp for events that finished together,
rather than having different timestamps when we process the events in the threaded irq handler.

> What actually 'more accurate' mean in this context ?
> 
> Regards
> Stanislaw

By 'more accurate' we mean closest to the MSIX interrupt.

Ofir.

Patch

diff --git a/drivers/accel/habanalabs/common/habanalabs.h b/drivers/accel/habanalabs/common/habanalabs.h
index 5624ea19ec0b..24ad15272040 100644
--- a/drivers/accel/habanalabs/common/habanalabs.h
+++ b/drivers/accel/habanalabs/common/habanalabs.h
@@ -1107,6 +1107,7 @@  enum hl_user_interrupt_type {
  * @type: user interrupt type
  * @wait_list_head: head to the list of user threads pending on this interrupt
  * @wait_list_lock: protects wait_list_head
+ * @timestamp: last timestamp taken upon interrupt
  * @interrupt_id: msix interrupt id
  */
 struct hl_user_interrupt {
@@ -1114,6 +1115,7 @@  struct hl_user_interrupt {
 	enum hl_user_interrupt_type	type;
 	struct list_head		wait_list_head;
 	spinlock_t			wait_list_lock;
+	ktime_t				timestamp;
 	u32				interrupt_id;
 };
 
diff --git a/drivers/accel/habanalabs/common/irq.c b/drivers/accel/habanalabs/common/irq.c
index c61c9a294ab8..716228291b46 100644
--- a/drivers/accel/habanalabs/common/irq.c
+++ b/drivers/accel/habanalabs/common/irq.c
@@ -280,7 +280,6 @@  static void handle_user_interrupt(struct hl_device *hdev, struct hl_user_interru
 	struct list_head *ts_reg_free_list_head = NULL;
 	struct timestamp_reg_work_obj *job;
 	bool reg_node_handle_fail = false;
-	ktime_t now = ktime_get();
 	int rc;
 
 	/* For registration nodes:
@@ -303,13 +302,13 @@  static void handle_user_interrupt(struct hl_device *hdev, struct hl_user_interru
 			if (pend->ts_reg_info.buf) {
 				if (!reg_node_handle_fail) {
 					rc = handle_registration_node(hdev, pend,
-								&ts_reg_free_list_head, now);
+							&ts_reg_free_list_head, intr->timestamp);
 					if (rc)
 						reg_node_handle_fail = true;
 				}
 			} else {
 				/* Handle wait target value node */
-				pend->fence.timestamp = now;
+				pend->fence.timestamp = intr->timestamp;
 				complete_all(&pend->fence.completion);
 			}
 		}
@@ -335,6 +334,10 @@  static void handle_user_interrupt(struct hl_device *hdev, struct hl_user_interru
  */
 irqreturn_t hl_irq_handler_user_interrupt(int irq, void *arg)
 {
+	struct hl_user_interrupt *user_int = arg;
+
+	user_int->timestamp = ktime_get();
+
 	return IRQ_WAKE_THREAD;
 }