[PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/powerpc/kernel/watchdog.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 15e209a37c2d..50797528b5e1 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -263,9 +263,8 @@ static void wd_timer_reset(unsigned int cpu, struct timer_list *t)
 	add_timer_on(t, cpu);
 }
 
-static void wd_timer_fn(unsigned long data)
+static void wd_timer_fn(struct timer_list *t)
 {
-	struct timer_list *t = this_cpu_ptr(&wd_timer);
 	int cpu = smp_processor_id();
 
 	watchdog_timer_interrupt(cpu);
@@ -292,7 +291,7 @@ static void start_watchdog_timer_on(unsigned int cpu)
 
 	per_cpu(wd_timer_tb, cpu) = get_tb();
 
-	setup_pinned_timer(t, wd_timer_fn, 0);
+	timer_setup(t, wd_timer_fn, TIMER_PINNED);
 	wd_timer_reset(cpu, t);
 }
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Nicholas Piggin]

On Mon, 16 Oct 2017 16:47:10 -0700
Kees Cook <keescook@chromium.org> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Looks fine to me. Is this intended to be merged via the powerpc tree
in the next merge window?

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/kernel/watchdog.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
> index 15e209a37c2d..50797528b5e1 100644
> --- a/arch/powerpc/kernel/watchdog.c
> +++ b/arch/powerpc/kernel/watchdog.c
> @@ -263,9 +263,8 @@ static void wd_timer_reset(unsigned int cpu, struct timer_list *t)
>  	add_timer_on(t, cpu);
>  }
>  
> -static void wd_timer_fn(unsigned long data)
> +static void wd_timer_fn(struct timer_list *t)
>  {
> -	struct timer_list *t = this_cpu_ptr(&wd_timer);
>  	int cpu = smp_processor_id();
>  
>  	watchdog_timer_interrupt(cpu);
> @@ -292,7 +291,7 @@ static void start_watchdog_timer_on(unsigned int cpu)
>  
>  	per_cpu(wd_timer_tb, cpu) = get_tb();
>  
> -	setup_pinned_timer(t, wd_timer_fn, 0);
> +	timer_setup(t, wd_timer_fn, TIMER_PINNED);
>  	wd_timer_reset(cpu, t);
>  }
>  

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Michael Ellerman]

Nicholas Piggin <npiggin@gmail.com> writes:

> On Mon, 16 Oct 2017 16:47:10 -0700
> Kees Cook <keescook@chromium.org> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Looks fine to me. Is this intended to be merged via the powerpc tree
> in the next merge window?

It relies on the new timer_setup(), which is in one of tglx's trees (I
think). So I expect it to go via that tree.

cheers

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Kees Cook]

On Tue, Oct 17, 2017 at 5:29 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> On Mon, 16 Oct 2017 16:47:10 -0700
>> Kees Cook <keescook@chromium.org> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> Looks fine to me. Is this intended to be merged via the powerpc tree
>> in the next merge window?
>
> It relies on the new timer_setup(), which is in one of tglx's trees (I
> think). So I expect it to go via that tree.

It's in -rc3, but the timer tree can carry it if you want. Which do you prefer?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Michael Ellerman]

Kees Cook <keescook@chromium.org> writes:

> On Tue, Oct 17, 2017 at 5:29 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>>
>>> On Mon, 16 Oct 2017 16:47:10 -0700
>>> Kees Cook <keescook@chromium.org> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Cc: Paul Mackerras <paulus@samba.org>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>
>>> Looks fine to me. Is this intended to be merged via the powerpc tree
>>> in the next merge window?
>>
>> It relies on the new timer_setup(), which is in one of tglx's trees (I
>> think). So I expect it to go via that tree.
>
> It's in -rc3, but the timer tree can carry it if you want. Which do
> you prefer?

Oh sorry, I assumed it was in only in linux-next.

I'll take this. Thanks.

cheers

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Michael Ellerman]

Michael Ellerman <mpe@ellerman.id.au> writes:
> Kees Cook <keescook@chromium.org> writes:
>> On Tue, Oct 17, 2017 at 5:29 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Nicholas Piggin <npiggin@gmail.com> writes:
>>>> On Mon, 16 Oct 2017 16:47:10 -0700
>>>> Kees Cook <keescook@chromium.org> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>>> Cc: Paul Mackerras <paulus@samba.org>
>>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>
>>>> Looks fine to me. Is this intended to be merged via the powerpc tree
>>>> in the next merge window?
>>>
>>> It relies on the new timer_setup(), which is in one of tglx's trees (I
>>> think). So I expect it to go via that tree.
>>
>> It's in -rc3, but the timer tree can carry it if you want. Which do
>> you prefer?
>
> Oh sorry, I assumed it was in only in linux-next.
>
> I'll take this. Thanks.

Ugh, I'm an <expletive deleted>.

My next is based on rc2, so I can't take this.

Can you please pick it up.

cheers

Re: [PATCH] powerpc/watchdog: Convert timers to use timer_setup()

[Michael Ellerman]

Kees Cook <keescook@chromium.org> writes:

> 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: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/powerpc/kernel/watchdog.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
> index 15e209a37c2d..50797528b5e1 100644
> --- a/arch/powerpc/kernel/watchdog.c
> +++ b/arch/powerpc/kernel/watchdog.c
> @@ -263,9 +263,8 @@ static void wd_timer_reset(unsigned int cpu, struct timer_list *t)
>  	add_timer_on(t, cpu);
>  }
>  
> -static void wd_timer_fn(unsigned long data)
> +static void wd_timer_fn(struct timer_list *t)

At first glance this looks like it will break, changing the signature,
but the new timer_setup() force-casts to the old signature (for now),
and we can fit a pointer in an unsigned long so it's OK.

>  {
> -	struct timer_list *t = this_cpu_ptr(&wd_timer);
>  	int cpu = smp_processor_id();
>  
>  	watchdog_timer_interrupt(cpu);
> @@ -292,7 +291,7 @@ static void start_watchdog_timer_on(unsigned int cpu)
>  
>  	per_cpu(wd_timer_tb, cpu) = get_tb();
>  
> -	setup_pinned_timer(t, wd_timer_fn, 0);
> +	timer_setup(t, wd_timer_fn, TIMER_PINNED);
>  	wd_timer_reset(cpu, t);
>  }
>  

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers