* [PATCH] rcu: Convert timers to use timer_setup()
@ 2017-10-24 9:32 Kees Cook
2017-10-25 14:17 ` Kees Cook
2017-10-30 18:04 ` Paul E. McKenney
0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2017-10-24 9:32 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
kernel/rcu/rcutorture.c | 4 ++--
kernel/rcu/tree_plugin.h | 9 +++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index e1d3fa534ac0..b6fbbeb5a7da 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
* counter in the element should never be greater than 1, otherwise, the
* RCU implementation is broken.
*/
-static void rcu_torture_timer(unsigned long unused)
+static void rcu_torture_timer(struct timer_list *unused)
{
int idx;
unsigned long started;
@@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
set_user_nice(current, MAX_NICE);
if (irqreader && cur_ops->irq_capable)
- setup_timer_on_stack(&t, rcu_torture_timer, 0);
+ timer_setup_on_stack(&t, rcu_torture_timer, 0);
do {
if (irqreader && cur_ops->irq_capable) {
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 8a5a3f9b1250..5fb7beee76aa 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
}
/* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
-static void do_nocb_deferred_wakeup_timer(unsigned long x)
+static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
{
- do_nocb_deferred_wakeup_common((struct rcu_data *)x);
+ struct rcu_data *x = from_timer(x, t, nocb_timer);
+
+ do_nocb_deferred_wakeup_common(x);
}
/*
@@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
init_swait_queue_head(&rdp->nocb_wq);
rdp->nocb_follower_tail = &rdp->nocb_follower_head;
raw_spin_lock_init(&rdp->nocb_lock);
- setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
- (unsigned long)rdp);
+ timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
}
/*
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: Convert timers to use timer_setup()
2017-10-24 9:32 [PATCH] rcu: Convert timers to use timer_setup() Kees Cook
@ 2017-10-25 14:17 ` Kees Cook
2017-10-30 18:05 ` Paul E. McKenney
2017-10-30 18:04 ` Paul E. McKenney
1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2017-10-25 14:17 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
LKML
Sorry, I sent this and forgot that timer_setup_on_stack() is only in
-next. If this patch is okay, I can carry it in the timers tree.
Thanks!
-Kees
On Tue, Oct 24, 2017 at 11:32 AM, 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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> kernel/rcu/rcutorture.c | 4 ++--
> kernel/rcu/tree_plugin.h | 9 +++++----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index e1d3fa534ac0..b6fbbeb5a7da 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
> * counter in the element should never be greater than 1, otherwise, the
> * RCU implementation is broken.
> */
> -static void rcu_torture_timer(unsigned long unused)
> +static void rcu_torture_timer(struct timer_list *unused)
> {
> int idx;
> unsigned long started;
> @@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
> VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
> set_user_nice(current, MAX_NICE);
> if (irqreader && cur_ops->irq_capable)
> - setup_timer_on_stack(&t, rcu_torture_timer, 0);
> + timer_setup_on_stack(&t, rcu_torture_timer, 0);
>
> do {
> if (irqreader && cur_ops->irq_capable) {
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 8a5a3f9b1250..5fb7beee76aa 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
> }
>
> /* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
> -static void do_nocb_deferred_wakeup_timer(unsigned long x)
> +static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
> {
> - do_nocb_deferred_wakeup_common((struct rcu_data *)x);
> + struct rcu_data *x = from_timer(x, t, nocb_timer);
> +
> + do_nocb_deferred_wakeup_common(x);
> }
>
> /*
> @@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> init_swait_queue_head(&rdp->nocb_wq);
> rdp->nocb_follower_tail = &rdp->nocb_follower_head;
> raw_spin_lock_init(&rdp->nocb_lock);
> - setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
> - (unsigned long)rdp);
> + timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
> }
>
> /*
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: Convert timers to use timer_setup()
2017-10-24 9:32 [PATCH] rcu: Convert timers to use timer_setup() Kees Cook
2017-10-25 14:17 ` Kees Cook
@ 2017-10-30 18:04 ` Paul E. McKenney
2017-10-30 21:13 ` Kees Cook
1 sibling, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-30 18:04 UTC (permalink / raw)
To: Kees Cook
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
linux-kernel
On Tue, Oct 24, 2017 at 02:32:04AM -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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
One question below.
Thanx, Paul
> ---
> kernel/rcu/rcutorture.c | 4 ++--
> kernel/rcu/tree_plugin.h | 9 +++++----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index e1d3fa534ac0..b6fbbeb5a7da 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
> * counter in the element should never be greater than 1, otherwise, the
> * RCU implementation is broken.
> */
> -static void rcu_torture_timer(unsigned long unused)
> +static void rcu_torture_timer(struct timer_list *unused)
> {
> int idx;
> unsigned long started;
> @@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
> VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
> set_user_nice(current, MAX_NICE);
> if (irqreader && cur_ops->irq_capable)
> - setup_timer_on_stack(&t, rcu_torture_timer, 0);
> + timer_setup_on_stack(&t, rcu_torture_timer, 0);
>
> do {
> if (irqreader && cur_ops->irq_capable) {
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 8a5a3f9b1250..5fb7beee76aa 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
> }
>
> /* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
> -static void do_nocb_deferred_wakeup_timer(unsigned long x)
> +static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
> {
> - do_nocb_deferred_wakeup_common((struct rcu_data *)x);
> + struct rcu_data *x = from_timer(x, t, nocb_timer);
As long as we are creating a real typed variable for this could we
please call it "rdp" in order to follow the usual RCU conventions?
struct rcu_data *rdp = from_timer(rdp, t, nocb_timer);
> +
> + do_nocb_deferred_wakeup_common(x);
And of course here:
do_nocb_deferred_wakeup_common(rdp);
> }
>
> /*
> @@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> init_swait_queue_head(&rdp->nocb_wq);
> rdp->nocb_follower_tail = &rdp->nocb_follower_head;
> raw_spin_lock_init(&rdp->nocb_lock);
> - setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
> - (unsigned long)rdp);
> + timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
Shouldn't this instead be something like this, give or take casts?
timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, rdp);
Otherwise, I don't see how do_nocb_deferred_wakeup_common() avoids a
NULL-pointer dereference.
> }
>
> /*
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: Convert timers to use timer_setup()
2017-10-25 14:17 ` Kees Cook
@ 2017-10-30 18:05 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-30 18:05 UTC (permalink / raw)
To: Kees Cook
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
LKML
On Wed, Oct 25, 2017 at 04:17:32PM +0200, Kees Cook wrote:
> Sorry, I sent this and forgot that timer_setup_on_stack() is only in
> -next. If this patch is okay, I can carry it in the timers tree.
Given the fixes I called out, I am fine with your carrying it separately.
Thanx, Paul
> Thanks!
>
> -Kees
>
> On Tue, Oct 24, 2017 at 11:32 AM, 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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > Cc: Josh Triplett <josh@joshtriplett.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > kernel/rcu/rcutorture.c | 4 ++--
> > kernel/rcu/tree_plugin.h | 9 +++++----
> > 2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index e1d3fa534ac0..b6fbbeb5a7da 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
> > * counter in the element should never be greater than 1, otherwise, the
> > * RCU implementation is broken.
> > */
> > -static void rcu_torture_timer(unsigned long unused)
> > +static void rcu_torture_timer(struct timer_list *unused)
> > {
> > int idx;
> > unsigned long started;
> > @@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
> > VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
> > set_user_nice(current, MAX_NICE);
> > if (irqreader && cur_ops->irq_capable)
> > - setup_timer_on_stack(&t, rcu_torture_timer, 0);
> > + timer_setup_on_stack(&t, rcu_torture_timer, 0);
> >
> > do {
> > if (irqreader && cur_ops->irq_capable) {
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 8a5a3f9b1250..5fb7beee76aa 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
> > }
> >
> > /* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
> > -static void do_nocb_deferred_wakeup_timer(unsigned long x)
> > +static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
> > {
> > - do_nocb_deferred_wakeup_common((struct rcu_data *)x);
> > + struct rcu_data *x = from_timer(x, t, nocb_timer);
> > +
> > + do_nocb_deferred_wakeup_common(x);
> > }
> >
> > /*
> > @@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> > init_swait_queue_head(&rdp->nocb_wq);
> > rdp->nocb_follower_tail = &rdp->nocb_follower_head;
> > raw_spin_lock_init(&rdp->nocb_lock);
> > - setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
> > - (unsigned long)rdp);
> > + timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
> > }
> >
> > /*
> > --
> > 2.7.4
> >
> >
> > --
> > Kees Cook
> > Pixel Security
>
>
>
> --
> Kees Cook
> Pixel Security
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: Convert timers to use timer_setup()
2017-10-30 18:04 ` Paul E. McKenney
@ 2017-10-30 21:13 ` Kees Cook
2017-10-30 21:22 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2017-10-30 21:13 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
LKML
On Mon, Oct 30, 2017 at 11:04 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Tue, Oct 24, 2017 at 02:32:04AM -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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> Cc: Josh Triplett <josh@joshtriplett.org>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> One question below.
>
> Thanx, Paul
>
>> ---
>> kernel/rcu/rcutorture.c | 4 ++--
>> kernel/rcu/tree_plugin.h | 9 +++++----
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
>> index e1d3fa534ac0..b6fbbeb5a7da 100644
>> --- a/kernel/rcu/rcutorture.c
>> +++ b/kernel/rcu/rcutorture.c
>> @@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
>> * counter in the element should never be greater than 1, otherwise, the
>> * RCU implementation is broken.
>> */
>> -static void rcu_torture_timer(unsigned long unused)
>> +static void rcu_torture_timer(struct timer_list *unused)
>> {
>> int idx;
>> unsigned long started;
>> @@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
>> VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
>> set_user_nice(current, MAX_NICE);
>> if (irqreader && cur_ops->irq_capable)
>> - setup_timer_on_stack(&t, rcu_torture_timer, 0);
>> + timer_setup_on_stack(&t, rcu_torture_timer, 0);
>>
>> do {
>> if (irqreader && cur_ops->irq_capable) {
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index 8a5a3f9b1250..5fb7beee76aa 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
>> }
>>
>> /* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
>> -static void do_nocb_deferred_wakeup_timer(unsigned long x)
>> +static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
>> {
>> - do_nocb_deferred_wakeup_common((struct rcu_data *)x);
>> + struct rcu_data *x = from_timer(x, t, nocb_timer);
>
> As long as we are creating a real typed variable for this could we
> please call it "rdp" in order to follow the usual RCU conventions?
Sure thing! My scripts had tried to minimize variable name churn, but
this makes much more sense for what's happening in this function.
>
> struct rcu_data *rdp = from_timer(rdp, t, nocb_timer);
>
>> +
>> + do_nocb_deferred_wakeup_common(x);
>
> And of course here:
>
> do_nocb_deferred_wakeup_common(rdp);
>
>> }
>>
>> /*
>> @@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
>> init_swait_queue_head(&rdp->nocb_wq);
>> rdp->nocb_follower_tail = &rdp->nocb_follower_head;
>> raw_spin_lock_init(&rdp->nocb_lock);
>> - setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
>> - (unsigned long)rdp);
>> + timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
>
> Shouldn't this instead be something like this, give or take casts?
>
> timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, rdp);
Nope, the new timer API will unconditionally pass the timer pointer
(&rdp->nocb_timer) to the callback. The 3rd argument is flags.
> Otherwise, I don't see how do_nocb_deferred_wakeup_common() avoids a
> NULL-pointer dereference.
You can see the intermediate step to the API here:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=686fef928bba6be13cabe639f154af7d72b63120
> Given the fixes I called out, I am fine with your carrying it separately.
I'll adjust the variable name and carry it in the timer tree. Thanks!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: Convert timers to use timer_setup()
2017-10-30 21:13 ` Kees Cook
@ 2017-10-30 21:22 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-30 21:22 UTC (permalink / raw)
To: Kees Cook
Cc: Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
LKML
On Mon, Oct 30, 2017 at 02:13:03PM -0700, Kees Cook wrote:
> On Mon, Oct 30, 2017 at 11:04 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Tue, Oct 24, 2017 at 02:32:04AM -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: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >> Cc: Josh Triplett <josh@joshtriplett.org>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > One question below.
> >
> > Thanx, Paul
> >
> >> ---
> >> kernel/rcu/rcutorture.c | 4 ++--
> >> kernel/rcu/tree_plugin.h | 9 +++++----
> >> 2 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> >> index e1d3fa534ac0..b6fbbeb5a7da 100644
> >> --- a/kernel/rcu/rcutorture.c
> >> +++ b/kernel/rcu/rcutorture.c
> >> @@ -1078,7 +1078,7 @@ static void rcu_torture_timer_cb(struct rcu_head *rhp)
> >> * counter in the element should never be greater than 1, otherwise, the
> >> * RCU implementation is broken.
> >> */
> >> -static void rcu_torture_timer(unsigned long unused)
> >> +static void rcu_torture_timer(struct timer_list *unused)
> >> {
> >> int idx;
> >> unsigned long started;
> >> @@ -1165,7 +1165,7 @@ rcu_torture_reader(void *arg)
> >> VERBOSE_TOROUT_STRING("rcu_torture_reader task started");
> >> set_user_nice(current, MAX_NICE);
> >> if (irqreader && cur_ops->irq_capable)
> >> - setup_timer_on_stack(&t, rcu_torture_timer, 0);
> >> + timer_setup_on_stack(&t, rcu_torture_timer, 0);
> >>
> >> do {
> >> if (irqreader && cur_ops->irq_capable) {
> >> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> >> index 8a5a3f9b1250..5fb7beee76aa 100644
> >> --- a/kernel/rcu/tree_plugin.h
> >> +++ b/kernel/rcu/tree_plugin.h
> >> @@ -2265,9 +2265,11 @@ static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp)
> >> }
> >>
> >> /* Do a deferred wakeup of rcu_nocb_kthread() from a timer handler. */
> >> -static void do_nocb_deferred_wakeup_timer(unsigned long x)
> >> +static void do_nocb_deferred_wakeup_timer(struct timer_list *t)
> >> {
> >> - do_nocb_deferred_wakeup_common((struct rcu_data *)x);
> >> + struct rcu_data *x = from_timer(x, t, nocb_timer);
> >
> > As long as we are creating a real typed variable for this could we
> > please call it "rdp" in order to follow the usual RCU conventions?
>
> Sure thing! My scripts had tried to minimize variable name churn, but
> this makes much more sense for what's happening in this function.
>
> >
> > struct rcu_data *rdp = from_timer(rdp, t, nocb_timer);
> >
> >> +
> >> + do_nocb_deferred_wakeup_common(x);
> >
> > And of course here:
> >
> > do_nocb_deferred_wakeup_common(rdp);
> >
> >> }
> >>
> >> /*
> >> @@ -2331,8 +2333,7 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> >> init_swait_queue_head(&rdp->nocb_wq);
> >> rdp->nocb_follower_tail = &rdp->nocb_follower_head;
> >> raw_spin_lock_init(&rdp->nocb_lock);
> >> - setup_timer(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer,
> >> - (unsigned long)rdp);
> >> + timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, 0);
> >
> > Shouldn't this instead be something like this, give or take casts?
> >
> > timer_setup(&rdp->nocb_timer, do_nocb_deferred_wakeup_timer, rdp);
>
> Nope, the new timer API will unconditionally pass the timer pointer
> (&rdp->nocb_timer) to the callback. The 3rd argument is flags.
>
> > Otherwise, I don't see how do_nocb_deferred_wakeup_common() avoids a
> > NULL-pointer dereference.
>
> You can see the intermediate step to the API here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=timers/core&id=686fef928bba6be13cabe639f154af7d72b63120
Ah, got it, from_timer() does the needed conversion.
> > Given the fixes I called out, I am fine with your carrying it separately.
>
> I'll adjust the variable name and carry it in the timer tree. Thanks!
Sounds good!
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-30 21:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-24 9:32 [PATCH] rcu: Convert timers to use timer_setup() Kees Cook
2017-10-25 14:17 ` Kees Cook
2017-10-30 18:05 ` Paul E. McKenney
2017-10-30 18:04 ` Paul E. McKenney
2017-10-30 21:13 ` Kees Cook
2017-10-30 21:22 ` Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).