From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752681AbdJ3SEw (ORCPT ); Mon, 30 Oct 2017 14:04:52 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:34462 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752074AbdJ3SEu (ORCPT ); Mon, 30 Oct 2017 14:04:50 -0400 Date: Mon, 30 Oct 2017 11:04:46 -0700 From: "Paul E. McKenney" To: Kees Cook Cc: Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , linux-kernel@vger.kernel.org Subject: Re: [PATCH] rcu: Convert timers to use timer_setup() Reply-To: paulmck@linux.vnet.ibm.com References: <20171024093204.GA47418@beast> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024093204.GA47418@beast> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17103018-0044-0000-0000-000003A79C94 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007982; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000239; SDB=6.00938730; UDB=6.00473208; IPR=6.00718991; BA=6.00005663; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017793; XFM=3.00000015; UTC=2017-10-30 18:04:48 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17103018-0045-0000-0000-000007D6AD3C Message-Id: <20171030180446.GY3659@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-10-30_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1710300242 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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" > Cc: Josh Triplett > Cc: Steven Rostedt > Cc: Mathieu Desnoyers > Cc: Lai Jiangshan > Signed-off-by: Kees Cook 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 >