All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-05  0:54 Kees Cook
  2017-10-05 13:45   ` Joonas Lahtinen
  0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2017-10-05  0:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Daniel Vetter, Jani Nikula, David Airlie, Chris Wilson,
	Joonas Lahtinen, Tvrtko Ursulin, Oscar Mateo, intel-gfx,
	dri-devel, Thomas Gleixner

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 <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index fc0fd7498689..331c2b09869e 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -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;
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
  2017-10-05  0:54 [PATCH] drm/i915: Convert timers to use timer_setup() Kees Cook
@ 2017-10-05 13:45   ` Joonas Lahtinen
  0 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-10-05 13:45 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: Daniel Vetter, Jani Nikula, David Airlie, Chris Wilson,
	Tvrtko Ursulin, Oscar Mateo, intel-gfx, dri-devel,
	Thomas Gleixner

On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
> 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 <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>

<SNIP>

> @@ -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 <joonas.lahtinen@linux.intel.com>

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-05 13:45   ` Joonas Lahtinen
  0 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-10-05 13:45 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: David Airlie, intel-gfx, dri-devel, Daniel Vetter, Thomas Gleixner

On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
> 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 <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>

<SNIP>

> @@ -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 <joonas.lahtinen@linux.intel.com>

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
  2017-10-05 13:45   ` Joonas Lahtinen
@ 2017-10-05 17:35     ` Kees Cook
  -1 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2017-10-05 17:35 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: LKML, Daniel Vetter, Jani Nikula, David Airlie, Chris Wilson,
	Tvrtko Ursulin, Oscar Mateo, intel-gfx,
	Maling list - DRI developers, Thomas Gleixner

On Thu, Oct 5, 2017 at 6:45 AM, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
> On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
>> 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 <daniel.vetter@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> <SNIP>
>
>> @@ -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:

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.

> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks!

> 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

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-05 17:35     ` Kees Cook
  0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2017-10-05 17:35 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: David Airlie, intel-gfx, LKML, Maling list - DRI developers,
	Daniel Vetter, Thomas Gleixner

On Thu, Oct 5, 2017 at 6:45 AM, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
> On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
>> 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 <daniel.vetter@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> <SNIP>
>
>> @@ -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:

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.

> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks!

> 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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
  2017-10-05 17:35     ` Kees Cook
@ 2017-10-06  8:34       ` Jani Nikula
  -1 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-10-06  8:34 UTC (permalink / raw)
  To: Kees Cook, Joonas Lahtinen
  Cc: LKML, Daniel Vetter, David Airlie, Chris Wilson, Tvrtko Ursulin,
	Oscar Mateo, intel-gfx, Maling list - DRI developers,
	Thomas Gleixner

On Thu, 05 Oct 2017, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Oct 5, 2017 at 6:45 AM, Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com> wrote:
>> On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
>>> 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 <daniel.vetter@intel.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Cc: David Airlie <airlied@linux.ie>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> <SNIP>
>>
>>> @@ -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:
>
> 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.
>
>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Thanks!
>
>> 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.



-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-06  8:34       ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-10-06  8:34 UTC (permalink / raw)
  To: Kees Cook, Joonas Lahtinen
  Cc: Oscar Mateo, Tvrtko Ursulin, intel-gfx, LKML,
	Maling list - DRI developers, Daniel Vetter, Thomas Gleixner

On Thu, 05 Oct 2017, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Oct 5, 2017 at 6:45 AM, Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com> wrote:
>> On Wed, 2017-10-04 at 17:54 -0700, Kees Cook wrote:
>>> 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 <daniel.vetter@intel.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Cc: David Airlie <airlied@linux.ie>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> <SNIP>
>>
>>> @@ -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:
>
> 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.
>
>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Thanks!
>
>> 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.



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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
  2017-10-16 22:55 ` Kees Cook
@ 2017-10-17  7:01   ` Joonas Lahtinen
  -1 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-10-17  7:01 UTC (permalink / raw)
  To: Kees Cook, Daniel Vetter
  Cc: Jani Nikula, David Airlie, Chris Wilson, Tvrtko Ursulin,
	Oscar Mateo, intel-gfx, dri-devel, linux-kernel

On Mon, 2017-10-16 at 15:55 -0700, Kees Cook wrote:
> 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 <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> # for mock_engine

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

For some reason our CI didn't pick this up even though it was correctly
sent to the mailing list, so I will re-send with my refreshed R-b for
our CI and then we can merge this.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-17  7:01   ` Joonas Lahtinen
  0 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-10-17  7:01 UTC (permalink / raw)
  To: Kees Cook, Daniel Vetter; +Cc: David Airlie, intel-gfx, linux-kernel, dri-devel

On Mon, 2017-10-16 at 15:55 -0700, Kees Cook wrote:
> 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 <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> # for mock_engine

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

For some reason our CI didn't pick this up even though it was correctly
sent to the mailing list, so I will re-send with my refreshed R-b for
our CI and then we can merge this.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-16 22:55 ` Kees Cook
  0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2017-10-16 22:55 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Jani Nikula, David Airlie, Chris Wilson, Joonas Lahtinen,
	Tvrtko Ursulin, Oscar Mateo, intel-gfx, dri-devel, linux-kernel

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 <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> # for mock_engine
---
This patch includes additional timers since the last time it was sent.
---
 drivers/gpu/drm/i915/i915_sw_fence.c         |  8 +++-----
 drivers/gpu/drm/i915/intel_breadcrumbs.c     | 18 ++++++++----------
 drivers/gpu/drm/i915/selftests/mock_engine.c |  8 +++-----
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index ca33cc08cb07..e8ca67a129d2 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -369,9 +369,9 @@ struct i915_sw_dma_fence_cb {
 	struct irq_work work;
 };
 
-static void timer_i915_sw_fence_wake(unsigned long data)
+static void timer_i915_sw_fence_wake(struct timer_list *t)
 {
-	struct i915_sw_dma_fence_cb *cb = (struct i915_sw_dma_fence_cb *)data;
+	struct i915_sw_dma_fence_cb *cb = from_timer(cb, t, timer);
 	struct i915_sw_fence *fence;
 
 	fence = xchg(&cb->fence, NULL);
@@ -434,9 +434,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
 	i915_sw_fence_await(fence);
 
 	cb->dma = NULL;
-	__setup_timer(&cb->timer,
-		      timer_i915_sw_fence_wake, (unsigned long)cb,
-		      TIMER_IRQSAFE);
+	timer_setup(&cb->timer, timer_i915_sw_fence_wake, TIMER_IRQSAFE);
 	init_irq_work(&cb->work, irq_i915_sw_fence_work);
 	if (timeout) {
 		cb->dma = dma_fence_get(dma);
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 29c62d481cef..48e1ba01ccf8 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -74,9 +74,10 @@ static noinline void missed_breadcrumb(struct intel_engine_cs *engine)
 	set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
 }
 
-static void intel_breadcrumbs_hangcheck(unsigned long data)
+static void intel_breadcrumbs_hangcheck(struct timer_list *t)
 {
-	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+	struct intel_engine_cs *engine = from_timer(engine, t,
+						    breadcrumbs.hangcheck);
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 
 	if (!b->irq_armed)
@@ -108,9 +109,10 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
 	}
 }
 
-static void intel_breadcrumbs_fake_irq(unsigned long data)
+static void intel_breadcrumbs_fake_irq(struct timer_list *t)
 {
-	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+	struct intel_engine_cs *engine = from_timer(engine, t,
+						    breadcrumbs.fake_irq);
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 
 	/* The timer persists in case we cannot enable interrupts,
@@ -787,12 +789,8 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
 	spin_lock_init(&b->rb_lock);
 	spin_lock_init(&b->irq_lock);
 
-	setup_timer(&b->fake_irq,
-		    intel_breadcrumbs_fake_irq,
-		    (unsigned long)engine);
-	setup_timer(&b->hangcheck,
-		    intel_breadcrumbs_hangcheck,
-		    (unsigned long)engine);
+	timer_setup(&b->fake_irq, intel_breadcrumbs_fake_irq, 0);
+	timer_setup(&b->hangcheck, intel_breadcrumbs_hangcheck, 0);
 
 	/* Spawn a thread to provide a common bottom-half for all signals.
 	 * As this is an asynchronous interface we cannot steal the current
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index fc0fd7498689..331c2b09869e 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -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;
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] drm/i915: Convert timers to use timer_setup()
@ 2017-10-16 22:55 ` Kees Cook
  0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2017-10-16 22:55 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: David Airlie, intel-gfx, linux-kernel, dri-devel

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 <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> # for mock_engine
---
This patch includes additional timers since the last time it was sent.
---
 drivers/gpu/drm/i915/i915_sw_fence.c         |  8 +++-----
 drivers/gpu/drm/i915/intel_breadcrumbs.c     | 18 ++++++++----------
 drivers/gpu/drm/i915/selftests/mock_engine.c |  8 +++-----
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index ca33cc08cb07..e8ca67a129d2 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -369,9 +369,9 @@ struct i915_sw_dma_fence_cb {
 	struct irq_work work;
 };
 
-static void timer_i915_sw_fence_wake(unsigned long data)
+static void timer_i915_sw_fence_wake(struct timer_list *t)
 {
-	struct i915_sw_dma_fence_cb *cb = (struct i915_sw_dma_fence_cb *)data;
+	struct i915_sw_dma_fence_cb *cb = from_timer(cb, t, timer);
 	struct i915_sw_fence *fence;
 
 	fence = xchg(&cb->fence, NULL);
@@ -434,9 +434,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
 	i915_sw_fence_await(fence);
 
 	cb->dma = NULL;
-	__setup_timer(&cb->timer,
-		      timer_i915_sw_fence_wake, (unsigned long)cb,
-		      TIMER_IRQSAFE);
+	timer_setup(&cb->timer, timer_i915_sw_fence_wake, TIMER_IRQSAFE);
 	init_irq_work(&cb->work, irq_i915_sw_fence_work);
 	if (timeout) {
 		cb->dma = dma_fence_get(dma);
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 29c62d481cef..48e1ba01ccf8 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -74,9 +74,10 @@ static noinline void missed_breadcrumb(struct intel_engine_cs *engine)
 	set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
 }
 
-static void intel_breadcrumbs_hangcheck(unsigned long data)
+static void intel_breadcrumbs_hangcheck(struct timer_list *t)
 {
-	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+	struct intel_engine_cs *engine = from_timer(engine, t,
+						    breadcrumbs.hangcheck);
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 
 	if (!b->irq_armed)
@@ -108,9 +109,10 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
 	}
 }
 
-static void intel_breadcrumbs_fake_irq(unsigned long data)
+static void intel_breadcrumbs_fake_irq(struct timer_list *t)
 {
-	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+	struct intel_engine_cs *engine = from_timer(engine, t,
+						    breadcrumbs.fake_irq);
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
 
 	/* The timer persists in case we cannot enable interrupts,
@@ -787,12 +789,8 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
 	spin_lock_init(&b->rb_lock);
 	spin_lock_init(&b->irq_lock);
 
-	setup_timer(&b->fake_irq,
-		    intel_breadcrumbs_fake_irq,
-		    (unsigned long)engine);
-	setup_timer(&b->hangcheck,
-		    intel_breadcrumbs_hangcheck,
-		    (unsigned long)engine);
+	timer_setup(&b->fake_irq, intel_breadcrumbs_fake_irq, 0);
+	timer_setup(&b->hangcheck, intel_breadcrumbs_hangcheck, 0);
 
 	/* Spawn a thread to provide a common bottom-half for all signals.
 	 * As this is an asynchronous interface we cannot steal the current
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index fc0fd7498689..331c2b09869e 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -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;
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-17  7:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05  0:54 [PATCH] drm/i915: Convert timers to use timer_setup() Kees Cook
2017-10-05 13:45 ` Joonas Lahtinen
2017-10-05 13:45   ` Joonas Lahtinen
2017-10-05 17:35   ` Kees Cook
2017-10-05 17:35     ` Kees Cook
2017-10-06  8:34     ` Jani Nikula
2017-10-06  8:34       ` Jani Nikula
2017-10-16 22:55 Kees Cook
2017-10-16 22:55 ` Kees Cook
2017-10-17  7:01 ` Joonas Lahtinen
2017-10-17  7:01   ` Joonas Lahtinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.