drm/i915/selftests: Convert timers to use timer_setup()

Commit Message

Kees Cook Oct. 24, 2017, 3:13 p.m. UTC

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: Jani Nikula <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
Cc: Rodrigo Vivi <[e-mail]>
Cc: David Airlie <[e-mail]>
Cc: Tvrtko Ursulin <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Kees Cook <[e-mail]>
---
 drivers/gpu/drm/i915/selftests/lib_sw_fence.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Chris Wilson Oct. 24, 2017, 4:17 p.m. UTC | #1

Quoting Kees Cook (2017-10-24 16:13:44)
> 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: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Tvrtko Ursulin <[e-mail]>
> Cc: Chris Wilson <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Signed-off-by: Kees Cook <[e-mail]>

Thank you for saving me from having to do this myself,
Reviewed-by: Chris Wilson <[e-mail]>
-Chris

Chris Wilson Oct. 25, 2017, 10:24 a.m. UTC | #2

Quoting Chris Wilson (2017-10-24 17:17:09)
> Quoting Kees Cook (2017-10-24 16:13:44)
> > 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: Jani Nikula <[e-mail]>
> > Cc: Joonas Lahtinen <[e-mail]>
> > Cc: Rodrigo Vivi <[e-mail]>
> > Cc: David Airlie <[e-mail]>
> > Cc: Tvrtko Ursulin <[e-mail]>
> > Cc: Chris Wilson <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > Signed-off-by: Kees Cook <[e-mail]>
> 
> Thank you for saving me from having to do this myself,
> Reviewed-by: Chris Wilson <[e-mail]>

I've a small batch of selftests patches queued, so added this one and
will push to drm-intel-next-queued shortly.
-Chris

Chris Wilson Oct. 25, 2017, 1:11 p.m. UTC | #3

Quoting Chris Wilson (2017-10-25 11:24:19)
> Quoting Chris Wilson (2017-10-24 17:17:09)
> > Quoting Kees Cook (2017-10-24 16:13:44)
> > > 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: Jani Nikula <[e-mail]>
> > > Cc: Joonas Lahtinen <[e-mail]>
> > > Cc: Rodrigo Vivi <[e-mail]>
> > > Cc: David Airlie <[e-mail]>
> > > Cc: Tvrtko Ursulin <[e-mail]>
> > > Cc: Chris Wilson <[e-mail]>
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > Signed-off-by: Kees Cook <[e-mail]>
> > 
> > Thank you for saving me from having to do this myself,
> > Reviewed-by: Chris Wilson <[e-mail]>
> 
> I've a small batch of selftests patches queued, so added this one and
> will push to drm-intel-next-queued shortly.

Oh dear, major faux pas. There is no timer_setup_on_stack yet.
-Chris

Kees Cook Oct. 25, 2017, 2:05 p.m. UTC | #4

On Wed, Oct 25, 2017 at 3:11 PM, Chris Wilson <[e-mail]> wrote:
> Quoting Chris Wilson (2017-10-25 11:24:19)
>> Quoting Chris Wilson (2017-10-24 17:17:09)
>> > Quoting Kees Cook (2017-10-24 16:13:44)
>> > > 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: Jani Nikula <[e-mail]>
>> > > Cc: Joonas Lahtinen <[e-mail]>
>> > > Cc: Rodrigo Vivi <[e-mail]>
>> > > Cc: David Airlie <[e-mail]>
>> > > Cc: Tvrtko Ursulin <[e-mail]>
>> > > Cc: Chris Wilson <[e-mail]>
>> > > Cc: [e-mail]
>> > > Cc: [e-mail]
>> > > Signed-off-by: Kees Cook <[e-mail]>
>> >
>> > Thank you for saving me from having to do this myself,
>> > Reviewed-by: Chris Wilson <[e-mail]>
>>
>> I've a small batch of selftests patches queued, so added this one and
>> will push to drm-intel-next-queued shortly.
>
> Oh dear, major faux pas. There is no timer_setup_on_stack yet.

Argh. Right, sorry. That's only in -next. Since this is mainly a
mechanical change, should I carry this in the timer tree, or wait
until the merge window for it to go via i915?

-Kees

Chris Wilson Oct. 25, 2017, 2:16 p.m. UTC | #5

Quoting Kees Cook (2017-10-25 15:05:13)
> On Wed, Oct 25, 2017 at 3:11 PM, Chris Wilson <[e-mail]> wrote:
> > Quoting Chris Wilson (2017-10-25 11:24:19)
> >> Quoting Chris Wilson (2017-10-24 17:17:09)
> >> > Quoting Kees Cook (2017-10-24 16:13:44)
> >> > > 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: Jani Nikula <[e-mail]>
> >> > > Cc: Joonas Lahtinen <[e-mail]>
> >> > > Cc: Rodrigo Vivi <[e-mail]>
> >> > > Cc: David Airlie <[e-mail]>
> >> > > Cc: Tvrtko Ursulin <[e-mail]>
> >> > > Cc: Chris Wilson <[e-mail]>
> >> > > Cc: [e-mail]
> >> > > Cc: [e-mail]
> >> > > Signed-off-by: Kees Cook <[e-mail]>
> >> >
> >> > Thank you for saving me from having to do this myself,
> >> > Reviewed-by: Chris Wilson <[e-mail]>
> >>
> >> I've a small batch of selftests patches queued, so added this one and
> >> will push to drm-intel-next-queued shortly.
> >
> > Oh dear, major faux pas. There is no timer_setup_on_stack yet.
> 
> Argh. Right, sorry. That's only in -next. Since this is mainly a
> mechanical change, should I carry this in the timer tree, or wait
> until the merge window for it to go via i915?

Jani has the final word, but my understanding is that there will be no
more from i915 towards the 4.15 merge. Hmm, the origin of this timer,

commit 214707fc2ce08d09982bc4fe4b7a1c1f010e82be
Author: Chris Wilson <[e-mail]>
Date:   Thu Oct 12 13:57:25 2017 +0100

    drm/i915/selftests: Wrap a timer into a i915_sw_fence

did make it into 4.15, so it would have been better to put into a
separate tree for the 4.15 merge window anyway. In hindsight, yes this
probably wants to be carried in the timer tree to be applied after i915.
(I guess there will be a few other stragglers that need to be converted
at the end of the merge window anyway.)

Jani?
-Chris

Kees Cook Oct. 25, 2017, 2:27 p.m. UTC | #6

On Wed, Oct 25, 2017 at 4:16 PM, Chris Wilson <[e-mail]> wrote:
> Quoting Kees Cook (2017-10-25 15:05:13)
>> On Wed, Oct 25, 2017 at 3:11 PM, Chris Wilson <[e-mail]> wrote:
>> > Quoting Chris Wilson (2017-10-25 11:24:19)
>> >> Quoting Chris Wilson (2017-10-24 17:17:09)
>> >> > Quoting Kees Cook (2017-10-24 16:13:44)
>> >> > > 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: Jani Nikula <[e-mail]>
>> >> > > Cc: Joonas Lahtinen <[e-mail]>
>> >> > > Cc: Rodrigo Vivi <[e-mail]>
>> >> > > Cc: David Airlie <[e-mail]>
>> >> > > Cc: Tvrtko Ursulin <[e-mail]>
>> >> > > Cc: Chris Wilson <[e-mail]>
>> >> > > Cc: [e-mail]
>> >> > > Cc: [e-mail]
>> >> > > Signed-off-by: Kees Cook <[e-mail]>
>> >> >
>> >> > Thank you for saving me from having to do this myself,
>> >> > Reviewed-by: Chris Wilson <[e-mail]>
>> >>
>> >> I've a small batch of selftests patches queued, so added this one and
>> >> will push to drm-intel-next-queued shortly.
>> >
>> > Oh dear, major faux pas. There is no timer_setup_on_stack yet.
>>
>> Argh. Right, sorry. That's only in -next. Since this is mainly a
>> mechanical change, should I carry this in the timer tree, or wait
>> until the merge window for it to go via i915?
>
> Jani has the final word, but my understanding is that there will be no
> more from i915 towards the 4.15 merge. Hmm, the origin of this timer,
>
> commit 214707fc2ce08d09982bc4fe4b7a1c1f010e82be
> Author: Chris Wilson <[e-mail]>
> Date:   Thu Oct 12 13:57:25 2017 +0100
>
>     drm/i915/selftests: Wrap a timer into a i915_sw_fence
>
> did make it into 4.15, so it would have been better to put into a
> separate tree for the 4.15 merge window anyway. In hindsight, yes this
> probably wants to be carried in the timer tree to be applied after i915.
> (I guess there will be a few other stragglers that need to be converted
> at the end of the merge window anyway.)

Yeah, it's going to be messy, but I'll manage. I'll be carrying a lot
of other stuff as well. Avoiding conflicts will be the trick. Wheee.
:)

-Kees

Jani Nikula Oct. 25, 2017, 2:32 p.m. UTC | #7

On Wed, 25 Oct 2017, Kees Cook <[e-mail]> wrote:
> On Wed, Oct 25, 2017 at 4:16 PM, Chris Wilson <[e-mail]> wrote:
>> Quoting Kees Cook (2017-10-25 15:05:13)
>>> On Wed, Oct 25, 2017 at 3:11 PM, Chris Wilson <[e-mail]> wrote:
>>> > Quoting Chris Wilson (2017-10-25 11:24:19)
>>> >> Quoting Chris Wilson (2017-10-24 17:17:09)
>>> >> > Quoting Kees Cook (2017-10-24 16:13:44)
>>> >> > > 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: Jani Nikula <[e-mail]>
>>> >> > > Cc: Joonas Lahtinen <[e-mail]>
>>> >> > > Cc: Rodrigo Vivi <[e-mail]>
>>> >> > > Cc: David Airlie <[e-mail]>
>>> >> > > Cc: Tvrtko Ursulin <[e-mail]>
>>> >> > > Cc: Chris Wilson <[e-mail]>
>>> >> > > Cc: [e-mail]
>>> >> > > Cc: [e-mail]
>>> >> > > Signed-off-by: Kees Cook <[e-mail]>
>>> >> >
>>> >> > Thank you for saving me from having to do this myself,
>>> >> > Reviewed-by: Chris Wilson <[e-mail]>
>>> >>
>>> >> I've a small batch of selftests patches queued, so added this one and
>>> >> will push to drm-intel-next-queued shortly.
>>> >
>>> > Oh dear, major faux pas. There is no timer_setup_on_stack yet.
>>>
>>> Argh. Right, sorry. That's only in -next. Since this is mainly a
>>> mechanical change, should I carry this in the timer tree, or wait
>>> until the merge window for it to go via i915?
>>
>> Jani has the final word, but my understanding is that there will be no
>> more from i915 towards the 4.15 merge. Hmm, the origin of this timer,
>>
>> commit 214707fc2ce08d09982bc4fe4b7a1c1f010e82be
>> Author: Chris Wilson <[e-mail]>
>> Date:   Thu Oct 12 13:57:25 2017 +0100
>>
>>     drm/i915/selftests: Wrap a timer into a i915_sw_fence
>>
>> did make it into 4.15, so it would have been better to put into a
>> separate tree for the 4.15 merge window anyway. In hindsight, yes this
>> probably wants to be carried in the timer tree to be applied after i915.
>> (I guess there will be a few other stragglers that need to be converted
>> at the end of the merge window anyway.)
>
> Yeah, it's going to be messy, but I'll manage. I'll be carrying a lot
> of other stuff as well. Avoiding conflicts will be the trick. Wheee.
> :)

Acked-by: Jani Nikula <[e-mail]>

for merging via timer tree. Otherwise we'll need to wait for the changes
to hit Linus' tree, then get backmerges to our tree, and it's v4.16
before you know it. ;)

Patch

diff --git a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
index 3790fdf44a1a..b26f07b55d86 100644
--- a/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/lib_sw_fence.c
@@ -49,9 +49,9 @@  void onstack_fence_fini(struct i915_sw_fence *fence)
 	i915_sw_fence_fini(fence);
 }
 
-static void timed_fence_wake(unsigned long data)
+static void timed_fence_wake(struct timer_list *t)
 {
-	struct timed_fence *tf = (struct timed_fence *)data;
+	struct timed_fence *tf = from_timer(tf, t, timer);
 
 	i915_sw_fence_commit(&tf->fence);
 }
@@ -60,7 +60,7 @@  void timed_fence_init(struct timed_fence *tf, unsigned long expires)
 {
 	onstack_fence_init(&tf->fence);
 
-	setup_timer_on_stack(&tf->timer, timed_fence_wake, (unsigned long)tf);
+	timer_setup_on_stack(&tf->timer, timed_fence_wake, 0);
 
 	if (time_after(expires, jiffies))
 		mod_timer(&tf->timer, expires);