| Message ID | 20171024084637.GA23944@beast |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: tipc: Convert timers to use timer_setup() | expand |
NAK. It doesn't sound like a good idea to send this to net. Especially since one of these timers has already been refactored in net-next. ///jon > -----Original Message----- > From: Kees Cook [mailto:keescook@chromium.org] > Sent: Tuesday, October 24, 2017 10:47 > To: David S. Miller <davem@davemloft.net> > Cc: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue > <ying.xue@windriver.com>; netdev@vger.kernel.org; tipc- > discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org > Subject: [PATCH] net: tipc: Convert timers to use timer_setup() > > 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: Jon Maloy <jon.maloy@ericsson.com> > Cc: Ying Xue <ying.xue@windriver.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: netdev@vger.kernel.org > Cc: tipc-discussion@lists.sourceforge.net > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > net/tipc/discover.c | 6 +++--- > net/tipc/monitor.c | 6 +++--- > net/tipc/node.c | 8 ++++---- > net/tipc/socket.c | 10 +++++----- > net/tipc/subscr.c | 6 +++--- > 5 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/net/tipc/discover.c b/net/tipc/discover.c index > 02462d67d191..92e4828c6b09 100644 > --- a/net/tipc/discover.c > +++ b/net/tipc/discover.c > @@ -224,9 +224,9 @@ void tipc_disc_remove_dest(struct tipc_link_req > *req) > * > * Called whenever a link setup request timer associated with a bearer > expires. > */ > -static void disc_timeout(unsigned long data) > +static void disc_timeout(struct timer_list *t) > { > - struct tipc_link_req *req = (struct tipc_link_req *)data; > + struct tipc_link_req *req = from_timer(req, t, timer); > struct sk_buff *skb; > int max_delay; > > @@ -292,7 +292,7 @@ int tipc_disc_create(struct net *net, struct > tipc_bearer *b, > req->num_nodes = 0; > req->timer_intv = TIPC_LINK_REQ_INIT; > spin_lock_init(&req->lock); > - setup_timer(&req->timer, disc_timeout, (unsigned long)req); > + timer_setup(&req->timer, disc_timeout, 0); > mod_timer(&req->timer, jiffies + req->timer_intv); > b->link_req = req; > *skb = skb_clone(req->buf, GFP_ATOMIC); diff --git > a/net/tipc/monitor.c b/net/tipc/monitor.c index > 9e109bb1a207..b9c32557d73c 100644 > --- a/net/tipc/monitor.c > +++ b/net/tipc/monitor.c > @@ -578,9 +578,9 @@ void tipc_mon_get_state(struct net *net, u32 addr, > read_unlock_bh(&mon->lock); > } > > -static void mon_timeout(unsigned long m) > +static void mon_timeout(struct timer_list *t) > { > - struct tipc_monitor *mon = (void *)m; > + struct tipc_monitor *mon = from_timer(mon, t, timer); > struct tipc_peer *self; > int best_member_cnt = dom_size(mon->peer_cnt) - 1; > > @@ -623,7 +623,7 @@ int tipc_mon_create(struct net *net, int bearer_id) > self->is_up = true; > self->is_head = true; > INIT_LIST_HEAD(&self->list); > - setup_timer(&mon->timer, mon_timeout, (unsigned long)mon); > + timer_setup(&mon->timer, mon_timeout, 0); > mon->timer_intv = msecs_to_jiffies(MON_TIMEOUT + (tn->random > & 0xffff)); > mod_timer(&mon->timer, jiffies + mon->timer_intv); > return 0; > diff --git a/net/tipc/node.c b/net/tipc/node.c index > 89f8ac73bf65..009a81631280 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -153,7 +153,7 @@ static void tipc_node_link_down(struct tipc_node *n, > int bearer_id, > bool delete); > static void node_lost_contact(struct tipc_node *n, struct sk_buff_head > *inputq); static void tipc_node_delete(struct tipc_node *node); -static void > tipc_node_timeout(unsigned long data); > +static void tipc_node_timeout(struct timer_list *t); > static void tipc_node_fsm_evt(struct tipc_node *n, int evt); static struct > tipc_node *tipc_node_find(struct net *net, u32 addr); static void > tipc_node_put(struct tipc_node *node); @@ -361,7 +361,7 @@ struct > tipc_node *tipc_node_create(struct net *net, u32 addr, u16 capabilities) > goto exit; > } > tipc_node_get(n); > - setup_timer(&n->timer, tipc_node_timeout, (unsigned long)n); > + timer_setup(&n->timer, tipc_node_timeout, 0); > n->keepalive_intv = U32_MAX; > hlist_add_head_rcu(&n->hash, &tn- > >node_htable[tipc_hashfn(addr)]); > list_for_each_entry_rcu(temp_node, &tn->node_list, list) { @@ - > 500,9 +500,9 @@ void tipc_node_remove_conn(struct net *net, u32 dnode, > u32 port) > > /* tipc_node_timeout - handle expiration of node timer > */ > -static void tipc_node_timeout(unsigned long data) > +static void tipc_node_timeout(struct timer_list *t) > { > - struct tipc_node *n = (struct tipc_node *)data; > + struct tipc_node *n = from_timer(n, t, timer); > struct tipc_link_entry *le; > struct sk_buff_head xmitq; > int bearer_id; > diff --git a/net/tipc/socket.c b/net/tipc/socket.c index > b3b72d8e9543..6387839f643d 100644 > --- a/net/tipc/socket.c > +++ b/net/tipc/socket.c > @@ -125,7 +125,7 @@ static void tipc_sock_destruct(struct sock *sk); static > int tipc_release(struct socket *sock); static int tipc_accept(struct socket > *sock, struct socket *new_sock, int flags, > bool kern); > -static void tipc_sk_timeout(unsigned long data); > +static void tipc_sk_timeout(struct timer_list *t); > static int tipc_sk_publish(struct tipc_sock *tsk, uint scope, > struct tipc_name_seq const *seq); static int > tipc_sk_withdraw(struct tipc_sock *tsk, uint scope, @@ -464,7 +464,7 @@ > static int tipc_sk_create(struct net *net, struct socket *sock, > NAMED_H_SIZE, 0); > > msg_set_origport(msg, tsk->portid); > - setup_timer(&sk->sk_timer, tipc_sk_timeout, (unsigned long)tsk); > + timer_setup(&sk->sk_timer, tipc_sk_timeout, 0); > sk->sk_shutdown = 0; > sk->sk_backlog_rcv = tipc_sk_backlog_rcv; > sk->sk_rcvbuf = sysctl_tipc_rmem[1]; > @@ -2530,14 +2530,14 @@ static int tipc_shutdown(struct socket *sock, int > how) > return res; > } > > -static void tipc_sk_timeout(unsigned long data) > +static void tipc_sk_timeout(struct timer_list *t) > { > - struct tipc_sock *tsk = (struct tipc_sock *)data; > + struct sock *sk = from_timer(sk, t, sk_timer); > + struct tipc_sock *tsk = tipc_sk(sk); > u32 peer_port = tsk_peer_port(tsk); > u32 peer_node = tsk_peer_node(tsk); > u32 own_node = tsk_own_node(tsk); > u32 own_port = tsk->portid; > - struct sock *sk = &tsk->sk; > struct net *net = sock_net(sk); > struct sk_buff *skb = NULL; > > diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index > be3d9e3183dc..251065dfd8df 100644 > --- a/net/tipc/subscr.c > +++ b/net/tipc/subscr.c > @@ -133,9 +133,9 @@ void tipc_subscrp_report_overlap(struct > tipc_subscription *sub, u32 found_lower, > node); > } > > -static void tipc_subscrp_timeout(unsigned long data) > +static void tipc_subscrp_timeout(struct timer_list *t) > { > - struct tipc_subscription *sub = (struct tipc_subscription *)data; > + struct tipc_subscription *sub = from_timer(sub, t, timer); > struct tipc_subscriber *subscriber = sub->subscriber; > > spin_lock_bh(&subscriber->lock); > @@ -303,7 +303,7 @@ static void tipc_subscrp_subscribe(struct net *net, > struct tipc_subscr *s, > tipc_subscrb_get(subscriber); > spin_unlock_bh(&subscriber->lock); > > - setup_timer(&sub->timer, tipc_subscrp_timeout, (unsigned > long)sub); > + timer_setup(&sub->timer, tipc_subscrp_timeout, 0); > timeout = htohl(sub->evt.s.timeout, swap); > > if (timeout != TIPC_WAIT_FOREVER) > -- > 2.7.4 > > > -- > Kees Cook > Pixel Security
On Tue, Oct 24, 2017 at 11:44 AM, Jon Maloy <jon.maloy@ericsson.com> wrote:
> NAK. It doesn't sound like a good idea to send this to net. Especially since one of these timers has already been refactored in net-next.
Hi! I'm not sure what you mean about the one timer issue. I don't see
any use of timer_setup() in net/tipc (and no recent conversions to the
older setup_timer() API). What's the preferred path for landing this
API conversion in net/tipc/?
And, just to note, these changes are almost entirely mechanical. The
only "special" case is in tipc_sk_timeout() where the argument needs
to be slightly adjusted to fetch the tsk from the sk again.
Thanks!
-Kees
> -----Original Message----- > From: keescook@google.com [mailto:keescook@google.com] On Behalf Of > Kees Cook > Sent: Friday, October 27, 2017 06:58 > To: Jon Maloy <jon.maloy@ericsson.com> > Cc: David S. Miller <davem@davemloft.net>; Ying Xue > <ying.xue@windriver.com>; netdev@vger.kernel.org; tipc- > discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] net: tipc: Convert timers to use timer_setup() > > On Tue, Oct 24, 2017 at 11:44 AM, Jon Maloy <jon.maloy@ericsson.com> > wrote: > > NAK. It doesn't sound like a good idea to send this to net. Especially since > one of these timers has already been refactored in net-next. > > Hi! I'm not sure what you mean about the one timer issue. I don't see any > use of timer_setup() in net/tipc (and no recent conversions to the older > setup_timer() API). What's the preferred path for landing this API conversion > in net/tipc/? > > And, just to note, these changes are almost entirely mechanical. The only > "special" case is in tipc_sk_timeout() where the argument needs to be > slightly adjusted to fetch the tsk from the sk again. I am referring to commit 0d5fcebf3c37 ("tipc: refactor tipc_sk_timeout() function") which is in "net-next" but not yet in "net". I guess David M is the one who decides whether you deliver to net or net-next, but in the former case you will have to wait until David merges in the above commit, and then add your change again. BR ///jon > > Thanks! > > -Kees > > -- > Kees Cook > Pixel Security
On Mon, Oct 30, 2017 at 2:57 AM, Jon Maloy <jon.maloy@ericsson.com> wrote: > > >> -----Original Message----- >> From: keescook@google.com [mailto:keescook@google.com] On Behalf Of >> Kees Cook >> Sent: Friday, October 27, 2017 06:58 >> To: Jon Maloy <jon.maloy@ericsson.com> >> Cc: David S. Miller <davem@davemloft.net>; Ying Xue >> <ying.xue@windriver.com>; netdev@vger.kernel.org; tipc- >> discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH] net: tipc: Convert timers to use timer_setup() >> >> On Tue, Oct 24, 2017 at 11:44 AM, Jon Maloy <jon.maloy@ericsson.com> >> wrote: >> > NAK. It doesn't sound like a good idea to send this to net. Especially since >> one of these timers has already been refactored in net-next. >> >> Hi! I'm not sure what you mean about the one timer issue. I don't see any >> use of timer_setup() in net/tipc (and no recent conversions to the older >> setup_timer() API). What's the preferred path for landing this API conversion >> in net/tipc/? >> >> And, just to note, these changes are almost entirely mechanical. The only >> "special" case is in tipc_sk_timeout() where the argument needs to be >> slightly adjusted to fetch the tsk from the sk again. > > I am referring to commit 0d5fcebf3c37 ("tipc: refactor tipc_sk_timeout() function") which is in "net-next" but not yet in "net". > I guess David M is the one who decides whether you deliver to net or net-next, but in the former case you will have to wait until David merges in the above commit, and then add your change again. Ah, thanks, I see the change now. These patches passed in the night, it seems. I've sent a v2 now for net-next. Thanks! -Kees
diff --git a/net/tipc/discover.c b/net/tipc/discover.c index 02462d67d191..92e4828c6b09 100644 --- a/net/tipc/discover.c +++ b/net/tipc/discover.c @@ -224,9 +224,9 @@ void tipc_disc_remove_dest(struct tipc_link_req *req) * * Called whenever a link setup request timer associated with a bearer expires. */ -static void disc_timeout(unsigned long data) +static void disc_timeout(struct timer_list *t) { - struct tipc_link_req *req = (struct tipc_link_req *)data; + struct tipc_link_req *req = from_timer(req, t, timer); struct sk_buff *skb; int max_delay; @@ -292,7 +292,7 @@ int tipc_disc_create(struct net *net, struct tipc_bearer *b, req->num_nodes = 0; req->timer_intv = TIPC_LINK_REQ_INIT; spin_lock_init(&req->lock); - setup_timer(&req->timer, disc_timeout, (unsigned long)req); + timer_setup(&req->timer, disc_timeout, 0); mod_timer(&req->timer, jiffies + req->timer_intv); b->link_req = req; *skb = skb_clone(req->buf, GFP_ATOMIC); diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c index 9e109bb1a207..b9c32557d73c 100644 --- a/net/tipc/monitor.c +++ b/net/tipc/monitor.c @@ -578,9 +578,9 @@ void tipc_mon_get_state(struct net *net, u32 addr, read_unlock_bh(&mon->lock); } -static void mon_timeout(unsigned long m) +static void mon_timeout(struct timer_list *t) { - struct tipc_monitor *mon = (void *)m; + struct tipc_monitor *mon = from_timer(mon, t, timer); struct tipc_peer *self; int best_member_cnt = dom_size(mon->peer_cnt) - 1; @@ -623,7 +623,7 @@ int tipc_mon_create(struct net *net, int bearer_id) self->is_up = true; self->is_head = true; INIT_LIST_HEAD(&self->list); - setup_timer(&mon->timer, mon_timeout, (unsigned long)mon); + timer_setup(&mon->timer, mon_timeout, 0); mon->timer_intv = msecs_to_jiffies(MON_TIMEOUT + (tn->random & 0xffff)); mod_timer(&mon->timer, jiffies + mon->timer_intv); return 0; diff --git a/net/tipc/node.c b/net/tipc/node.c index 89f8ac73bf65..009a81631280 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -153,7 +153,7 @@ static void tipc_node_link_down(struct tipc_node *n, int bearer_id, bool delete); static void node_lost_contact(struct tipc_node *n, struct sk_buff_head *inputq); static void tipc_node_delete(struct tipc_node *node); -static void tipc_node_timeout(unsigned long data); +static void tipc_node_timeout(struct timer_list *t); static void tipc_node_fsm_evt(struct tipc_node *n, int evt); static struct tipc_node *tipc_node_find(struct net *net, u32 addr); static void tipc_node_put(struct tipc_node *node); @@ -361,7 +361,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u16 capabilities) goto exit; } tipc_node_get(n); - setup_timer(&n->timer, tipc_node_timeout, (unsigned long)n); + timer_setup(&n->timer, tipc_node_timeout, 0); n->keepalive_intv = U32_MAX; hlist_add_head_rcu(&n->hash, &tn->node_htable[tipc_hashfn(addr)]); list_for_each_entry_rcu(temp_node, &tn->node_list, list) { @@ -500,9 +500,9 @@ void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port) /* tipc_node_timeout - handle expiration of node timer */ -static void tipc_node_timeout(unsigned long data) +static void tipc_node_timeout(struct timer_list *t) { - struct tipc_node *n = (struct tipc_node *)data; + struct tipc_node *n = from_timer(n, t, timer); struct tipc_link_entry *le; struct sk_buff_head xmitq; int bearer_id; diff --git a/net/tipc/socket.c b/net/tipc/socket.c index b3b72d8e9543..6387839f643d 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -125,7 +125,7 @@ static void tipc_sock_destruct(struct sock *sk); static int tipc_release(struct socket *sock); static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags, bool kern); -static void tipc_sk_timeout(unsigned long data); +static void tipc_sk_timeout(struct timer_list *t); static int tipc_sk_publish(struct tipc_sock *tsk, uint scope, struct tipc_name_seq const *seq); static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope, @@ -464,7 +464,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock, NAMED_H_SIZE, 0); msg_set_origport(msg, tsk->portid); - setup_timer(&sk->sk_timer, tipc_sk_timeout, (unsigned long)tsk); + timer_setup(&sk->sk_timer, tipc_sk_timeout, 0); sk->sk_shutdown = 0; sk->sk_backlog_rcv = tipc_sk_backlog_rcv; sk->sk_rcvbuf = sysctl_tipc_rmem[1]; @@ -2530,14 +2530,14 @@ static int tipc_shutdown(struct socket *sock, int how) return res; } -static void tipc_sk_timeout(unsigned long data) +static void tipc_sk_timeout(struct timer_list *t) { - struct tipc_sock *tsk = (struct tipc_sock *)data; + struct sock *sk = from_timer(sk, t, sk_timer); + struct tipc_sock *tsk = tipc_sk(sk); u32 peer_port = tsk_peer_port(tsk); u32 peer_node = tsk_peer_node(tsk); u32 own_node = tsk_own_node(tsk); u32 own_port = tsk->portid; - struct sock *sk = &tsk->sk; struct net *net = sock_net(sk); struct sk_buff *skb = NULL; diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index be3d9e3183dc..251065dfd8df 100644 --- a/net/tipc/subscr.c +++ b/net/tipc/subscr.c @@ -133,9 +133,9 @@ void tipc_subscrp_report_overlap(struct tipc_subscription *sub, u32 found_lower, node); } -static void tipc_subscrp_timeout(unsigned long data) +static void tipc_subscrp_timeout(struct timer_list *t) { - struct tipc_subscription *sub = (struct tipc_subscription *)data; + struct tipc_subscription *sub = from_timer(sub, t, timer); struct tipc_subscriber *subscriber = sub->subscriber; spin_lock_bh(&subscriber->lock); @@ -303,7 +303,7 @@ static void tipc_subscrp_subscribe(struct net *net, struct tipc_subscr *s, tipc_subscrb_get(subscriber); spin_unlock_bh(&subscriber->lock); - setup_timer(&sub->timer, tipc_subscrp_timeout, (unsigned long)sub); + timer_setup(&sub->timer, tipc_subscrp_timeout, 0); timeout = htohl(sub->evt.s.timeout, swap); if (timeout != TIPC_WAIT_FOREVER)
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: Jon Maloy <jon.maloy@ericsson.com> Cc: Ying Xue <ying.xue@windriver.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: netdev@vger.kernel.org Cc: tipc-discussion@lists.sourceforge.net Signed-off-by: Kees Cook <keescook@chromium.org> --- net/tipc/discover.c | 6 +++--- net/tipc/monitor.c | 6 +++--- net/tipc/node.c | 8 ++++---- net/tipc/socket.c | 10 +++++----- net/tipc/subscr.c | 6 +++--- 5 files changed, 18 insertions(+), 18 deletions(-)