staging: greybus: Convert timers to use timer_setup()

PATCH In review Kees Cook 4 participants 14 messages Oct 24, 2017 – Nov 3, 2017

Subsystems (3)

Staging Greybus Greybus loopback driver

lore

Share & tools

lore

Kees Cook

author

2017-10-24 14:49 UTC #0 lore raw

PATCH v2 1/1 staging: greybus: Convert timers to use timer_setup()

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

Cc Greg Kroah-Hartman <[email protected]>

Cc "Bryan O'Donoghue" <[email protected]>

Cc Johan Hovold <[email protected]>

Cc Alex Elder <[email protected]>

Cc [email protected]

Signed-off-by Kees Cook <[email protected]>

---
v2: Added back "get" in timer code, thanks to Bryan. :)

---
 drivers/staging/greybus/loopback.c  | 19 +++++++++----------
 drivers/staging/greybus/operation.c |  7 +++----
 2 files changed, 12 insertions(+), 14 deletions(-)

drivers/staging/greybus/loopback.c +9 ~~-10~~

index 08e255884206..1c0bafeb7ea5 100644
@@ -572,16 +572,16 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
 	gb_loopback_async_operation_put(op_async);
 }
 
-static void gb_loopback_async_operation_timeout(unsigned long data)
+static void gb_loopback_async_operation_timeout(struct timer_list *t)
 {
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	struct gb_loopback_async_operation *op_async =
+		from_timer(op_async, t, timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_dev.lock, flags);
+	gb_loopback_async_operation_get(op_async);
+	spin_unlock_irqrestore(&gb_dev.lock, flags);
 
-	op_async = gb_loopback_operation_find(id);
-	if (!op_async) {
-		pr_err("operation %d not found - time out ?\n", id);
-		return;
-	}
 	schedule_work(&op_async->work);
 }

@@ -631,8 +631,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 	if (ret)
 		goto error;
 
-	setup_timer(&op_async->timer, gb_loopback_async_operation_timeout,
-			(unsigned long)operation->id);
+	timer_setup(&op_async->timer, gb_loopback_async_operation_timeout, 0);
 	op_async->timer.expires = jiffies + gb->jiffy_timeout;
 	add_timer(&op_async->timer);

drivers/staging/greybus/operation.c +3 -4

index 3023012808d9..ee4ba3f23bef 100644
@@ -294,9 +294,9 @@ static void gb_operation_work(struct work_struct *work)
 	gb_operation_put(operation);
 }
 
-static void gb_operation_timeout(unsigned long arg)
+static void gb_operation_timeout(struct timer_list *t)
 {
-	struct gb_operation *operation = (void *)arg;
+	struct gb_operation *operation = from_timer(operation, t, timer);
 
 	if (gb_operation_result_set(operation, -ETIMEDOUT)) {
 		/*

@@ -541,8 +541,7 @@ gb_operation_create_common(struct gb_connection *connection, u8 type,
 			goto err_request;
 		}
 
-		setup_timer(&operation->timer, gb_operation_timeout,
-			    (unsigned long)operation);
+		timer_setup(&operation->timer, gb_operation_timeout, 0);
 	}
 
 	operation->flags = op_flags;

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Bryan O'Donoghue

2017-10-24 15:55 UTC #1 lore raw

Quoted 28 lines · Kees Cook

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.
Cc Greg Kroah-Hartman <[email protected]>
Cc "Bryan O'Donoghue" <[email protected]>
Cc Johan Hovold <[email protected]>
Cc Alex Elder <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
v2: Added back "get" in timer code, thanks to Bryan. :)
---
  drivers/staging/greybus/loopback.c  | 19 +++++++++----------
  drivers/staging/greybus/operation.c |  7 +++----
  2 files changed, 12 insertions(+), 14 deletions(-)
drivers/staging/greybus/loopback.c +1 -1
index 08e255884206..1c0bafeb7ea5 100644
@@ -572,16 +572,16 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
  	gb_loopback_async_operation_put(op_async);
  }
 
-static void gb_loopback_async_operation_timeout(unsigned long data)
+static void gb_loopback_async_operation_timeout(struct timer_list *t)

drivers/staging/greybus/loopback.c static void gb_loopback_async_operation_work(struct work_struct *work) · line 583

Quoted 10 lines

drivers/staging/greybus/loopback.c +7 -2

  {
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	struct gb_loopback_async_operation *op_async =
+		from_timer(op_async, t, timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_dev.lock, flags);
+	gb_loopback_async_operation_get(op_async);
+	spin_unlock_irqrestore(&gb_dev.lock, flags);

Damnit I'm just wrong (I hate that).

The pointer can already have gone away by the time the timer runs - my bad...

see attached for update - with my Signed-off added.

---
bod

Quoted 1 line

From aa9fbf091122c816a47de2aece5412f7fd9225a9 Mon Sep 17 00:00:00 2001

From: Kees Cook <[email protected]>
Date: Tue, 24 Oct 2017 07:49:38 -0700
Subject: [PATCH] staging: greybus: Convert timers to use timer_setup()

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

Cc Greg Kroah-Hartman <[email protected]>

Cc "Bryan O'Donoghue" <[email protected]>

Cc Johan Hovold <[email protected]>

Cc Alex Elder <[email protected]>

Cc [email protected]

Signed-off-by Kees Cook <[email protected]>

Signed-off-by Bryan O'Donoghue <[email protected]>

---
 drivers/staging/greybus/loopback.c  | 38 ++++++++++++++++++++++++++-----------
 drivers/staging/greybus/operation.c |  7 +++----
 2 files changed, 30 insertions(+), 15 deletions(-)

drivers/staging/greybus/loopback.c +27 ~~-11~~

index 08e2558..81447e3 100644
@@ -472,6 +472,26 @@ static void gb_loopback_async_operation_put(struct gb_loopback_async_operation
 	spin_unlock_irqrestore(&gb_dev.lock, flags);
 }
 
+static struct gb_loopback_async_operation *gb_loopback_async_operation_valid(
+	struct gb_loopback_async_operation *op_async)
+{
+	struct gb_loopback_async_operation *op_async_tmp;
+	bool found = false;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_dev.lock, flags);
+	list_for_each_entry(op_async_tmp, &gb_dev.list_op_async, entry) {
+		if (op_async_tmp == op_async) {
+			gb_loopback_async_operation_get(op_async);
+			found = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&gb_dev.lock, flags);
+
+	return found ? op_async : NULL;
+}
+
 static struct gb_loopback_async_operation *
 	gb_loopback_operation_find(u16 id)
 {

@@ -572,17 +592,14 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
 	gb_loopback_async_operation_put(op_async);
 }
 
-static void gb_loopback_async_operation_timeout(unsigned long data)
+static void gb_loopback_async_operation_timeout(struct timer_list *t)
 {
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	struct gb_loopback_async_operation *op_async =
+		from_timer(op_async, t, timer);
 
-	op_async = gb_loopback_operation_find(id);
-	if (!op_async) {
-		pr_err("operation %d not found - time out ?\n", id);
-		return;
-	}
-	schedule_work(&op_async->work);
+	op_async = gb_loopback_async_operation_valid(op_async);
+	if (op_async)
+		schedule_work(&op_async->work);
 }
 
 static int gb_loopback_async_operation(struct gb_loopback *gb, int type,

@@ -631,8 +648,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 	if (ret)
 		goto error;
 
-	setup_timer(&op_async->timer, gb_loopback_async_operation_timeout,
-			(unsigned long)operation->id);
+	timer_setup(&op_async->timer, gb_loopback_async_operation_timeout, 0);
 	op_async->timer.expires = jiffies + gb->jiffy_timeout;
 	add_timer(&op_async->timer);

drivers/staging/greybus/operation.c +3 -4

index 3023012..ee4ba3f 100644
@@ -294,9 +294,9 @@ static void gb_operation_work(struct work_struct *work)
 	gb_operation_put(operation);
 }
 
-static void gb_operation_timeout(unsigned long arg)
+static void gb_operation_timeout(struct timer_list *t)
 {
-	struct gb_operation *operation = (void *)arg;
+	struct gb_operation *operation = from_timer(operation, t, timer);
 
 	if (gb_operation_result_set(operation, -ETIMEDOUT)) {
 		/*

@@ -541,8 +541,7 @@ gb_operation_create_common(struct gb_connection *connection, u8 type,
 			goto err_request;
 		}
 
-		setup_timer(&operation->timer, gb_operation_timeout,
-			    (unsigned long)operation);
+		timer_setup(&operation->timer, gb_operation_timeout, 0);
 	}
 
 	operation->flags = op_flags;

Signature · 1 line

2.7.4

Johan Hovold

maintainer

2017-10-30 11:32 UTC #2 lore raw

Quoted 42 lines · Bryan O'Donoghue

Quoted 28 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.
Cc Greg Kroah-Hartman <[email protected]>
Cc "Bryan O'Donoghue" <[email protected]>
Cc Johan Hovold <[email protected]>
Cc Alex Elder <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
v2: Added back "get" in timer code, thanks to Bryan. :)
---
  drivers/staging/greybus/loopback.c  | 19 +++++++++----------
  drivers/staging/greybus/operation.c |  7 +++----
  2 files changed, 12 insertions(+), 14 deletions(-)
drivers/staging/greybus/loopback.c +1 -1
index 08e255884206..1c0bafeb7ea5 100644
@@ -572,16 +572,16 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
  	gb_loopback_async_operation_put(op_async);
  }
 
-static void gb_loopback_async_operation_timeout(unsigned long data)
+static void gb_loopback_async_operation_timeout(struct timer_list *t)
drivers/staging/greybus/loopback.c static void gb_loopback_async_operation_work(struct work_struct *work) · line 583
Quoted 10 lines
drivers/staging/greybus/loopback.c +7 -2
  {
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	struct gb_loopback_async_operation *op_async =
+		from_timer(op_async, t, timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_dev.lock, flags);
+	gb_loopback_async_operation_get(op_async);
+	spin_unlock_irqrestore(&gb_dev.lock, flags);
Damnit I'm just wrong (I hate that).

Quoted 2 lines

The pointer can already have gone away by the time the timer runs - my bad...

Hmm. Then something is really broken in this driver, you obviously must never free the async operation which contains the timer while the timer is active.

Quoted 1 line

see attached for update - with my Signed-off added.

The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core. Otherwise, I'm afraid you're not addressing the underlying bug.

Either way, Kees, please submit the operation.c conversion separately from the loopback one, as the latter is non-trivial.

Thanks,
Johan

Bryan O'Donoghue

maintainer

2017-10-30 11:35 UTC #3 lore raw

Quoted 3 lines · Johan Hovold

The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core.

Actually I wasn't clear if you wanted to to that yourself aswell as the rest if it.

But sure I can do that conversion, it's on my list.

Johan Hovold

maintainer

2017-10-30 11:38 UTC #4 lore raw

Quoted 9 lines · Bryan O'Donoghue

Quoted 3 lines · Johan Hovold
The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core.
Actually I wasn't clear if you wanted to to that yourself aswell as the rest if it.
But sure I can do that conversion, it's on my list.

IIRC it was basically done. Just some odd locking that could now also be removed.

Thanks,
Johan

Bryan O'Donoghue

2017-10-30 11:44 UTC #5 lore raw

Quoted 9 lines · Johan Hovold

Quoted 8 lines · Bryan O'Donoghue
Quoted 3 lines · Johan Hovold
The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core.
Actually I wasn't clear if you wanted to to that yourself aswell as the rest if it.

Quoted 8 lines

Quoted 1 line
But sure I can do that conversion, it's on my list.
IIRC it was basically done. Just some odd locking that could now also be removed.
Thanks,
Johan

I think once Kees' change is applied to operation.c and we convert the async stuff to operation.c's callbacks there ought to be no use of timers, linked lists of operations.

I'll probably need at least a day to look at that, so it'll be the weekend before I can really allocate time.

---
bod

Johan Hovold

maintainer

2017-10-30 11:48 UTC #6 lore raw

Quoted 17 lines · Bryan O'Donoghue

Quoted 14 lines · Johan Hovold
Quoted 9 lines · Bryan O'Donoghue
Quoted 3 lines · Johan Hovold
The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core.
Actually I wasn't clear if you wanted to to that yourself aswell as the rest if it.
But sure I can do that conversion, it's on my list.
IIRC it was basically done. Just some odd locking that could now also be removed.

Quoted 7 lines

Quoted 3 lines
Thanks,
Johan
I think once Kees' change is applied to operation.c and we convert the async stuff to operation.c's callbacks there ought to be no use of timers, linked lists of operations.

That's correct.

Quoted 2 lines

I'll probably need at least a day to look at that, so it'll be the weekend before I can really allocate time.

Cool. I'm quite sure I just rebased your loopback conversion patch on my core timeout handling and used that to test the core implementation, so it should be straight forward.

Thanks,
Johan

Kees Cook

author

2017-10-30 21:37 UTC #7 lore raw

Quoted 26 lines · Johan Hovold

Quoted 17 lines · Bryan O'Donoghue
Quoted 14 lines · Johan Hovold
Quoted 9 lines · Bryan O'Donoghue
Quoted 3 lines · Johan Hovold
The right thing to do here is to respin your patch from last year which converts the loopback driver to use the timeout handling in greybus core.
Actually I wasn't clear if you wanted to to that yourself aswell as the rest if it.
But sure I can do that conversion, it's on my list.
IIRC it was basically done. Just some odd locking that could now also be removed.
Quoted 7 lines
Quoted 3 lines
Thanks,
Johan
I think once Kees' change is applied to operation.c and we convert the async stuff to operation.c's callbacks there ought to be no use of timers, linked lists of operations.

Quoted 8 lines

That's correct.
Quoted 2 lines
I'll probably need at least a day to look at that, so it'll be the weekend before I can really allocate time.
Cool. I'm quite sure I just rebased your loopback conversion patch on my core timeout handling and used that to test the core implementation, so it should be straight forward.

Hi,

I seem to have lost the thread of conversation a bit. What exactly remains that I should be doing here for timer conversions? (It sounded like it was already partially handled already?)

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

[email protected]

maintainer

2017-10-31 00:01 UTC #8 lore raw

Quoted 43 lines · Kees Cook

Quoted 7 lines · Johan Hovold
Quoted 6 lines · Bryan O'Donoghue
Quoted 3 lines · Johan Hovold
Quoted 2 lines · Bryan O'Donoghue
Quoted 1 line · Johan Hovold
The right thing to do here is to respin your patch from last
year which
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
converts the loopback driver to use the timeout handling in
greybus
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
core.
Actually I wasn't clear if you wanted to to that yourself aswell
as the
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
rest if it.
But sure I can do that conversion, it's on my list.
IIRC it was basically done. Just some odd locking that could now
also be
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
removed.
Thanks,
Johan
I think once Kees' change is applied to operation.c and we convert
the
Quoted 9 lines
Quoted 2 lines
async stuff to operation.c's callbacks there ought to be no use of timers, linked lists of operations.
That's correct.
Quoted 2 lines
I'll probably need at least a day to look at that, so it'll be the weekend before I can really allocate time.
Cool. I'm quite sure I just rebased your loopback conversion patch on
my
Quoted 1 line
core timeout handling and used that to test the core implementation,
so
Quoted 1 line
it should be straight forward.

Quoted 7 lines

Hi,
I seem to have lost the thread of conversation a bit. What exactly remains that I should be doing here for timer conversions? (It sounded like it was already partially handled already?)
-Kees

Trying again without top posting in html :(

Just pair the patch down to operation.c.

There's a separate change to loopback.c an old patch ARAIR that will subtract use of the timer from loopback.c so you can skip that bit.

Signature · 1 line

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Kees Cook

author

2017-10-31 00:05 UTC #9 lore raw

Quoted 50 lines · On Mon, Oct 30, 2017 at 5:01 PM, <[email protected]> wrote:

Quoted 49 lines · Kees Cook
Quoted 7 lines · Johan Hovold
Quoted 6 lines · Bryan O'Donoghue
Quoted 3 lines · Johan Hovold
Quoted 2 lines · Bryan O'Donoghue
Quoted 1 line · Johan Hovold
The right thing to do here is to respin your patch from last
year which
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
converts the loopback driver to use the timeout handling in
greybus
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
core.
Actually I wasn't clear if you wanted to to that yourself aswell
as the
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
rest if it.
But sure I can do that conversion, it's on my list.
IIRC it was basically done. Just some odd locking that could now
also be
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
removed.
Thanks,
Johan
I think once Kees' change is applied to operation.c and we convert
the
Quoted 9 lines
Quoted 2 lines
async stuff to operation.c's callbacks there ought to be no use of timers, linked lists of operations.
That's correct.
Quoted 2 lines
I'll probably need at least a day to look at that, so it'll be the weekend before I can really allocate time.
Cool. I'm quite sure I just rebased your loopback conversion patch on
my
Quoted 1 line
core timeout handling and used that to test the core implementation,
so
Quoted 1 line
it should be straight forward.
Hi,
I seem to have lost the thread of conversation a bit. What exactly remains that I should be doing here for timer conversions? (It sounded like it was already partially handled already?)

Quoted 7 lines

Quoted 1 line
-Kees
Trying again without top posting in html :(
Just pair the patch down to operation.c.
There's a separate change to loopback.c an old patch ARAIR that will subtract use of the timer from loopback.c so you can skip that bit.

Okay, cool. Since the operation.c change is trivial, I'll include it in the giant tree-wide patch that will (hopefully) land in -rc1.

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Kees Cook

author

2017-11-03 20:21 UTC #10 lore raw

Quoted 5 lines · Kees Cook

Quoted 1 line · On Mon, Oct 30, 2017 at 5:01 PM, <[email protected]> wrote:
There's a separate change to loopback.c an old patch ARAIR that will subtract use of the timer from loopback.c so you can skip that bit.
Okay, cool. Since the operation.c change is trivial, I'll include it in the giant tree-wide patch that will (hopefully) land in -rc1.

I forgot to ask: will the patch for loopback.c that removes the timers get posted in the next couple days? I just want to make sure the timer conversions don't get blocked behind this.

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Bryan O'Donoghue

2017-11-03 21:49 UTC #11 lore raw

Quoted 10 lines · Kees Cook

Quoted 5 lines · Kees Cook
Quoted 1 line · On Mon, Oct 30, 2017 at 5:01 PM, <[email protected]> wrote:
There's a separate change to loopback.c an old patch ARAIR that will subtract use of the timer from loopback.c so you can skip that bit.
Okay, cool. Since the operation.c change is trivial, I'll include it in the giant tree-wide patch that will (hopefully) land in -rc1.
I forgot to ask: will the patch for loopback.c that removes the timers get posted in the next couple days? I just want to make sure the timer conversions don't get blocked behind this.

Yep.

I should get that out tomorrow at some stage.

Kees Cook

author

2017-11-03 21:49 UTC #12 lore raw

Quoted 16 lines · Bryan O'Donoghue

Quoted 13 lines · Kees Cook
Quoted 9 lines · Kees Cook
Quoted 3 lines · On Mon, Oct 30, 2017 at 5:01 PM, <[email protected]> wrote:
There's a separate change to loopback.c an old patch ARAIR that will subtract use of the timer from loopback.c so you can skip that bit.
Okay, cool. Since the operation.c change is trivial, I'll include it in the giant tree-wide patch that will (hopefully) land in -rc1.

Quoted 8 lines

Quoted 3 lines
I forgot to ask: will the patch for loopback.c that removes the timers get posted in the next couple days? I just want to make sure the timer conversions don't get blocked behind this.
Yep.
I should get that out tomorrow at some stage.

Awesome, thanks very much!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Johan Hovold

maintainer

2017-10-30 11:36 UTC #13 lore raw

In reply to Bryan O'Donoghue

[ Resend to Bryan's nexus-software address ]

Quoted 42 lines · Bryan O'Donoghue

Quoted 28 lines · Kees Cook
In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.
Cc Greg Kroah-Hartman <[email protected]>
Cc "Bryan O'Donoghue" <[email protected]>
Cc Johan Hovold <[email protected]>
Cc Alex Elder <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
v2: Added back "get" in timer code, thanks to Bryan. :)
---
  drivers/staging/greybus/loopback.c  | 19 +++++++++----------
  drivers/staging/greybus/operation.c |  7 +++----
  2 files changed, 12 insertions(+), 14 deletions(-)
drivers/staging/greybus/loopback.c +1 -1
index 08e255884206..1c0bafeb7ea5 100644
@@ -572,16 +572,16 @@ static void gb_loopback_async_operation_work(struct work_struct *work)
  	gb_loopback_async_operation_put(op_async);
  }
 
-static void gb_loopback_async_operation_timeout(unsigned long data)
+static void gb_loopback_async_operation_timeout(struct timer_list *t)
drivers/staging/greybus/loopback.c static void gb_loopback_async_operation_work(struct work_struct *work) · line 583
Quoted 10 lines
drivers/staging/greybus/loopback.c +7 -2
  {
-	struct gb_loopback_async_operation *op_async;
-	u16 id = data;
+	struct gb_loopback_async_operation *op_async =
+		from_timer(op_async, t, timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_dev.lock, flags);
+	gb_loopback_async_operation_get(op_async);
+	spin_unlock_irqrestore(&gb_dev.lock, flags);
Damnit I'm just wrong (I hate that).

Quoted 2 lines

The pointer can already have gone away by the time the timer runs - my bad...

Hmm. Then something is really broken in this driver, you obviously must never free the async operation which contains the timer while the timer is active.

Quoted 1 line

see attached for update - with my Signed-off added.

Either way, Kees, please submit the operation.c conversion separately from the loopback one, as the latter is non-trivial.

Thanks,
Johan