| Message ID | 20170130163713.17524-5-aschultz@tpip.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Andreas, On Mon, Jan 30, 2017 at 05:37:11PM +0100, Andreas Schultz wrote: > Consolidate duplicate code into helper. Makes complete sense. However: > +static void pdp_context_delete(struct pdp_ctx *pctx); > + why introduce the forward-declaration and then define the function further down in the file? I think in general, it is preferred to put helper functions on top, before their first use, so forward declarations can be avoided? Particularly if the helper function doesn't call any other functions that are not yet declared at this point. Please note the above might just be my personal taste, not sure if that's a general habit in the kernel networking code these days. So with or without the re-ordering: Acked-by: Harald Welte <laforge@gnumonks.org>
----- On Feb 6, 2017, at 2:58 PM, laforge laforge@gnumonks.org wrote: > Hi Andreas, > > On Mon, Jan 30, 2017 at 05:37:11PM +0100, Andreas Schultz wrote: >> Consolidate duplicate code into helper. > > Makes complete sense. > > However: > >> +static void pdp_context_delete(struct pdp_ctx *pctx); >> + > > why introduce the forward-declaration and then define the function > further down in the file? I think in general, it is preferred to put > helper functions on top, before their first use, so forward declarations > can be avoided? Particularly if the helper function doesn't call any > other functions that are not yet declared at this point. I wanted to keep functions that work on the data structure close together. I this case the function the initialized the pdp context and the delete function for it. Andreas > > Please note the above might just be my personal taste, not sure if > that's a general habit in the kernel networking code these days. > > So with or without the re-ordering: > > Acked-by: Harald Welte <laforge@gnumonks.org> > > -- > - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ > ============================================================================ > "Privacy in residential applications is a desirable marketing option." > (ETSI EN 300 175-7 Ch. A6)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 6b7a3c2..68c6c9b 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -84,6 +84,8 @@ struct gtp_net { static u32 gtp_h_initval; +static void pdp_context_delete(struct pdp_ctx *pctx); + static inline u32 gtp0_hashfn(u64 tid) { u32 *tid32 = (u32 *) &tid; @@ -774,13 +776,10 @@ static void gtp_hashtable_free(struct gtp_dev *gtp) struct pdp_ctx *pctx; int i; - for (i = 0; i < gtp->hash_size; i++) { - hlist_for_each_entry_rcu(pctx, >p->tid_hash[i], hlist_tid) { - hlist_del_rcu(&pctx->hlist_tid); - hlist_del_rcu(&pctx->hlist_addr); - kfree_rcu(pctx, rcu_head); - } - } + for (i = 0; i < gtp->hash_size; i++) + hlist_for_each_entry_rcu(pctx, >p->tid_hash[i], hlist_tid) + pdp_context_delete(pctx); + synchronize_rcu(); kfree(gtp->addr_hash); kfree(gtp->tid_hash); @@ -985,6 +984,13 @@ static int ipv4_pdp_add(struct gtp_dev *gtp, struct genl_info *info) return 0; } +static void pdp_context_delete(struct pdp_ctx *pctx) +{ + hlist_del_rcu(&pctx->hlist_tid); + hlist_del_rcu(&pctx->hlist_addr); + kfree(pctx); +} + static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info) { struct gtp_dev *gtp; @@ -1093,9 +1099,7 @@ static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info) pr_debug("GTPv1-U: deleting tunnel id = %x/%x (pdp %p)\n", pctx->u.v1.i_tei, pctx->u.v1.o_tei, pctx); - hlist_del_rcu(&pctx->hlist_tid); - hlist_del_rcu(&pctx->hlist_addr); - kfree_rcu(pctx, rcu_head); + pdp_context_delete(pctx); out_unlock: rcu_read_unlock();
Consolidate duplicate code into helper. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- drivers/net/gtp.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)