drm/i915: Convert timers to use timer_setup()

PATCH In review Kees Cook 3 participants 4 messages Oct 5, 2017 – Oct 6, 2017

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

Kees Cook

author

2017-10-05 00:54 UTC #0 lore raw

PATCH 1/1 drm/i915: 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 Daniel Vetter <[email protected]>

Cc Jani Nikula <[email protected]>

Cc David Airlie <[email protected]>

Cc Chris Wilson <[email protected]>

Cc Joonas Lahtinen <[email protected]>

Cc Tvrtko Ursulin <[email protected]>

Cc Oscar Mateo <[email protected]>

Cc [email protected]

Cc Thomas Gleixner <[email protected]>

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

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/gpu/drm/i915/selftests/mock_engine.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/gpu/drm/i915/selftests/mock_engine.c +3 -5

index fc0fd7498689..331c2b09869e 100644
@@ -32,9 +32,9 @@ static struct mock_request *first_request(struct mock_engine *engine)
 					link);
 }
 
-static void hw_delay_complete(unsigned long data)
+static void hw_delay_complete(struct timer_list *t)
 {
-	struct mock_engine *engine = (typeof(engine))data;
+	struct mock_engine *engine = from_timer(engine, t, hw_delay);
 	struct mock_request *request;
 
 	spin_lock(&engine->hw_lock);

@@ -161,9 +161,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
 
 	/* fake hw queue */
 	spin_lock_init(&engine->hw_lock);
-	setup_timer(&engine->hw_delay,
-		    hw_delay_complete,
-		    (unsigned long)engine);
+	timer_setup(&engine->hw_delay, hw_delay_complete, 0);
 	INIT_LIST_HEAD(&engine->hw_queue);
 
 	return &engine->base;

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Joonas Lahtinen

maintainer Reviewed-by on v1 1/1

2017-10-05 13:45 UTC #1 lore raw

Quoted 15 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 Daniel Vetter <[email protected]>
Cc Jani Nikula <[email protected]>
Cc David Airlie <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Tvrtko Ursulin <[email protected]>
Cc Oscar Mateo <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Thomas Gleixner <[email protected]>
Signed-off-by Kees Cook <[email protected]>

<SNIP>

drivers/gpu/drm/i915/selftests/mock_engine.c static struct mock_request *first_request(struct mock_engine *engine) · line 37

Quoted 9 lines

drivers/gpu/drm/i915/selftests/mock_engine.c +2 -2

@@ -32,9 +32,9 @@ static struct mock_request *first_request(struct mock_engine *engine)
 					link);
 }
 
-static void hw_delay_complete(unsigned long data)
+static void hw_delay_complete(struct timer_list *t)
 {
-	struct mock_engine *engine = (typeof(engine))data;
+	struct mock_engine *engine = from_timer(engine, t, hw_delay);

The order is bit strange to me, it's not same as with container_of, but I guess GCC will complain for getting it wrong. It's also slightly different doing the typeof for you, so I guess it makes sense, so:

Reviewed-by Joonas Lahtinen <[email protected]>

Do you expect for us to merge or are you looking to merge all timer changes from single tree?

Regards, Joonas

Signature · 7 lines

Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Kees Cook

author

2017-10-05 17:35 UTC #2 lore raw

Quoted 23 lines · Joonas Lahtinen

Quoted 15 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 Daniel Vetter <[email protected]>
Cc Jani Nikula <[email protected]>
Cc David Airlie <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Tvrtko Ursulin <[email protected]>
Cc Oscar Mateo <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Thomas Gleixner <[email protected]>
Signed-off-by Kees Cook <[email protected]>
<SNIP>
Quoted 4 lines
(quoted diff) +0 -0
@@ -32,9 +32,9 @@ static struct mock_request *first_request(struct mock_engine *engine)
                                      link);
 }
 

Quoted 9 lines

drivers/gpu/drm/i915/selftests/mock_engine.c static struct mock_request *first_request(struct mock_engine *engine) · line 37
Quoted 5 lines
drivers/gpu/drm/i915/selftests/mock_engine.c +2 -2
-static void hw_delay_complete(unsigned long data)
+static void hw_delay_complete(struct timer_list *t)
 {
-     struct mock_engine *engine = (typeof(engine))data;
+     struct mock_engine *engine = from_timer(engine, t, hw_delay);
The order is bit strange to me, it's not same as with container_of, but I guess GCC will complain for getting it wrong. It's also slightly different doing the typeof for you, so I guess it makes sense, so:

Yeah, this seemed to be the least bad of several options. Other things ended up being either very long, named unlike anything else already in the kernel, etc.

Quoted 1 line

Reviewed-by Joonas Lahtinen <[email protected]>

Thanks!

Quoted 2 lines

Do you expect for us to merge or are you looking to merge all timer changes from single tree?

If you have -rc3 in your tree already, please take this into your tree. If you prefer the timer tree to carry it, that can happen too. tglx suggested to me that it was better for maintainers to carry the changes.

-Kees

Signature · 6 lines

Kees Cook
Pixel Security
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

maintainer

2017-10-06 08:34 UTC #3 lore raw

Quoted 43 lines · Kees Cook

Quoted 23 lines · Joonas Lahtinen
Quoted 15 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 Daniel Vetter <[email protected]>
Cc Jani Nikula <[email protected]>
Cc David Airlie <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Tvrtko Ursulin <[email protected]>
Cc Oscar Mateo <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Thomas Gleixner <[email protected]>
Signed-off-by Kees Cook <[email protected]>
<SNIP>
Quoted 4 lines
(quoted diff) +0 -0
@@ -32,9 +32,9 @@ static struct mock_request *first_request(struct mock_engine *engine)
                                      link);
 }
 
Quoted 9 lines
drivers/gpu/drm/i915/selftests/mock_engine.c static struct mock_request *first_request(struct mock_engine *engine) · line 37
Quoted 5 lines
drivers/gpu/drm/i915/selftests/mock_engine.c +2 -2
-static void hw_delay_complete(unsigned long data)
+static void hw_delay_complete(struct timer_list *t)
 {
-     struct mock_engine *engine = (typeof(engine))data;
+     struct mock_engine *engine = from_timer(engine, t, hw_delay);
The order is bit strange to me, it's not same as with container_of, but I guess GCC will complain for getting it wrong. It's also slightly different doing the typeof for you, so I guess it makes sense, so:
Yeah, this seemed to be the least bad of several options. Other things ended up being either very long, named unlike anything else already in the kernel, etc.
Quoted 1 line
Reviewed-by Joonas Lahtinen <[email protected]>
Thanks!

Quoted 7 lines

Quoted 2 lines
Do you expect for us to merge or are you looking to merge all timer changes from single tree?
If you have -rc3 in your tree already, please take this into your tree. If you prefer the timer tree to carry it, that can happen too. tglx suggested to me that it was better for maintainers to carry the changes.

We'll pick this when we have -rc3.

Thanks,
Jani.

Signature · 5 lines

Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel