* [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls [not found] ` <5317A59D.4@users.sourceforge.net> @ 2014-11-18 19:47 ` SF Markus Elfring 2014-11-19 13:40 ` Pablo Neira Ayuso 2014-11-19 22:26 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls Julian Anastasov 2015-11-14 19:05 ` [PATCH] net-ipv6: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring 1 sibling, 2 replies; 10+ messages in thread From: SF Markus Elfring @ 2014-11-18 19:47 UTC (permalink / raw) To: David S. Miller, Jozsef Kadlecsik, Julian Anastasov, Pablo Neira Ayuso, Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam Cc: LKML, kernel-janitors, Coccinelle From: Markus Elfring <elfring@users.sourceforge.net> Date: Tue, 18 Nov 2014 20:37:05 +0100 The functions free_percpu() and module_put() test whether their argument is NULL and then return immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- net/netfilter/ipvs/ip_vs_ctl.c | 3 +-- net/netfilter/ipvs/ip_vs_pe.c | 3 +-- net/netfilter/ipvs/ip_vs_sched.c | 3 +-- net/netfilter/ipvs/ip_vs_sync.c | 3 +-- net/netfilter/nf_tables_api.c | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index fd3f444..7c5e40a 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc) static void ip_vs_service_free(struct ip_vs_service *svc) { - if (svc->stats.cpustats) - free_percpu(svc->stats.cpustats); + free_percpu(svc->stats.cpustats); kfree(svc); } diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c index 1a82b29..0df17ca 100644 --- a/net/netfilter/ipvs/ip_vs_pe.c +++ b/net/netfilter/ipvs/ip_vs_pe.c @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name) rcu_read_unlock(); return pe; } - if (pe->module) - module_put(pe->module); + module_put(pe->module); } rcu_read_unlock(); diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c index 4dbcda6..199760c 100644 --- a/net/netfilter/ipvs/ip_vs_sched.c +++ b/net/netfilter/ipvs/ip_vs_sched.c @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name) mutex_unlock(&ip_vs_sched_mutex); return sched; } - if (sched->module) - module_put(sched->module); + module_put(sched->module); } mutex_unlock(&ip_vs_sched_mutex); diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c index eadffb2..cafe28d 100644 --- a/net/netfilter/ipvs/ip_vs_sync.c +++ b/net/netfilter/ipvs/ip_vs_sync.c @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc, p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC); if (!p->pe_data) { - if (p->pe->module) - module_put(p->pe->module); + module_put(p->pe->module); return -ENOMEM; } p->pe_data_len = pe_data_len; diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index deeb95f..b115f54 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb) break; case NFT_MSG_NEWCHAIN: if (nft_trans_chain_update(trans)) { - if (nft_trans_chain_stats(trans)) - free_percpu(nft_trans_chain_stats(trans)); + free_percpu(nft_trans_chain_stats(trans)); nft_trans_destroy(trans); } else { -- 2.1.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls 2014-11-18 19:47 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls SF Markus Elfring @ 2014-11-19 13:40 ` Pablo Neira Ayuso 2015-07-02 15:10 ` [PATCH] net-ipvs: Delete an unnecessary check before the function call "module_put" SF Markus Elfring 2014-11-19 22:26 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls Julian Anastasov 1 sibling, 1 reply; 10+ messages in thread From: Pablo Neira Ayuso @ 2014-11-19 13:40 UTC (permalink / raw) To: SF Markus Elfring Cc: David S. Miller, Jozsef Kadlecsik, Julian Anastasov, Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle On Tue, Nov 18, 2014 at 08:47:31PM +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 18 Nov 2014 20:37:05 +0100 > > The functions free_percpu() and module_put() test whether their argument > is NULL and then return immediately. Thus the test around the call is > not needed. @IPVS folks: Since this involves a nf_tables specific chunk, ack your chunks and I'll put this into nf-next to speed up things. Thanks. > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > net/netfilter/ipvs/ip_vs_ctl.c | 3 +-- > net/netfilter/ipvs/ip_vs_pe.c | 3 +-- > net/netfilter/ipvs/ip_vs_sched.c | 3 +-- > net/netfilter/ipvs/ip_vs_sync.c | 3 +-- > net/netfilter/nf_tables_api.c | 3 +-- > 5 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c > index fd3f444..7c5e40a 100644 > --- a/net/netfilter/ipvs/ip_vs_ctl.c > +++ b/net/netfilter/ipvs/ip_vs_ctl.c > @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc) > > static void ip_vs_service_free(struct ip_vs_service *svc) > { > - if (svc->stats.cpustats) > - free_percpu(svc->stats.cpustats); > + free_percpu(svc->stats.cpustats); > kfree(svc); > } > > diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c > index 1a82b29..0df17ca 100644 > --- a/net/netfilter/ipvs/ip_vs_pe.c > +++ b/net/netfilter/ipvs/ip_vs_pe.c > @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name) > rcu_read_unlock(); > return pe; > } > - if (pe->module) > - module_put(pe->module); > + module_put(pe->module); > } > rcu_read_unlock(); > > diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c > index 4dbcda6..199760c 100644 > --- a/net/netfilter/ipvs/ip_vs_sched.c > +++ b/net/netfilter/ipvs/ip_vs_sched.c > @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name) > mutex_unlock(&ip_vs_sched_mutex); > return sched; > } > - if (sched->module) > - module_put(sched->module); > + module_put(sched->module); > } > > mutex_unlock(&ip_vs_sched_mutex); > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c > index eadffb2..cafe28d 100644 > --- a/net/netfilter/ipvs/ip_vs_sync.c > +++ b/net/netfilter/ipvs/ip_vs_sync.c > @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc, > > p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC); > if (!p->pe_data) { > - if (p->pe->module) > - module_put(p->pe->module); > + module_put(p->pe->module); > return -ENOMEM; > } > p->pe_data_len = pe_data_len; > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index deeb95f..b115f54 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb) > break; > case NFT_MSG_NEWCHAIN: > if (nft_trans_chain_update(trans)) { > - if (nft_trans_chain_stats(trans)) > - free_percpu(nft_trans_chain_stats(trans)); > + free_percpu(nft_trans_chain_stats(trans)); > > nft_trans_destroy(trans); > } else { > -- > 2.1.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] net-ipvs: Delete an unnecessary check before the function call "module_put" 2014-11-19 13:40 ` Pablo Neira Ayuso @ 2015-07-02 15:10 ` SF Markus Elfring 2015-07-09 1:41 ` Simon Horman 0 siblings, 1 reply; 10+ messages in thread From: SF Markus Elfring @ 2015-07-02 15:10 UTC (permalink / raw) To: David S. Miller, Jozsef Kadlecsik, Julian Anastasov, Pablo Neira Ayuso, Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall From: Markus Elfring <elfring@users.sourceforge.net> Date: Thu, 2 Jul 2015 17:00:14 +0200 The module_put() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- net/netfilter/ipvs/ip_vs_sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c index 199760c..e50221b 100644 --- a/net/netfilter/ipvs/ip_vs_sched.c +++ b/net/netfilter/ipvs/ip_vs_sched.c @@ -137,7 +137,7 @@ struct ip_vs_scheduler *ip_vs_scheduler_get(const char *sched_name) void ip_vs_scheduler_put(struct ip_vs_scheduler *scheduler) { - if (scheduler && scheduler->module) + if (scheduler) module_put(scheduler->module); } -- 2.4.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] net-ipvs: Delete an unnecessary check before the function call "module_put" 2015-07-02 15:10 ` [PATCH] net-ipvs: Delete an unnecessary check before the function call "module_put" SF Markus Elfring @ 2015-07-09 1:41 ` Simon Horman 0 siblings, 0 replies; 10+ messages in thread From: Simon Horman @ 2015-07-09 1:41 UTC (permalink / raw) To: SF Markus Elfring Cc: David S. Miller, Jozsef Kadlecsik, Julian Anastasov, Pablo Neira Ayuso, Patrick McHardy, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam, LKML, kernel-janitors@vger.kernel.org, Julia Lawall On Thu, Jul 02, 2015 at 05:10:41PM +0200, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Thu, 2 Jul 2015 17:00:14 +0200 > > The module_put() function tests whether its argument is NULL and then > returns immediately. Thus the test around the call is not needed. > > This issue was detected by using the Coccinelle software. Thanks, applied to ipvs-next for v4.3. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls 2014-11-18 19:47 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls SF Markus Elfring 2014-11-19 13:40 ` Pablo Neira Ayuso @ 2014-11-19 22:26 ` Julian Anastasov 2014-11-20 1:13 ` Simon Horman 1 sibling, 1 reply; 10+ messages in thread From: Julian Anastasov @ 2014-11-19 22:26 UTC (permalink / raw) To: SF Markus Elfring Cc: David S. Miller, Jozsef Kadlecsik, Pablo Neira Ayuso, Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle Hello, On Tue, 18 Nov 2014, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 18 Nov 2014 20:37:05 +0100 > > The functions free_percpu() and module_put() test whether their argument > is NULL and then return immediately. Thus the test around the call is > not needed. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> Pablo, the IPVS parts look ok to me, Acked-by: Julian Anastasov <ja@ssi.bg> > --- > net/netfilter/ipvs/ip_vs_ctl.c | 3 +-- > net/netfilter/ipvs/ip_vs_pe.c | 3 +-- > net/netfilter/ipvs/ip_vs_sched.c | 3 +-- > net/netfilter/ipvs/ip_vs_sync.c | 3 +-- > net/netfilter/nf_tables_api.c | 3 +-- > 5 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c > index fd3f444..7c5e40a 100644 > --- a/net/netfilter/ipvs/ip_vs_ctl.c > +++ b/net/netfilter/ipvs/ip_vs_ctl.c > @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc) > > static void ip_vs_service_free(struct ip_vs_service *svc) > { > - if (svc->stats.cpustats) > - free_percpu(svc->stats.cpustats); > + free_percpu(svc->stats.cpustats); > kfree(svc); > } > > diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c > index 1a82b29..0df17ca 100644 > --- a/net/netfilter/ipvs/ip_vs_pe.c > +++ b/net/netfilter/ipvs/ip_vs_pe.c > @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name) > rcu_read_unlock(); > return pe; > } > - if (pe->module) > - module_put(pe->module); > + module_put(pe->module); > } > rcu_read_unlock(); > > diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c > index 4dbcda6..199760c 100644 > --- a/net/netfilter/ipvs/ip_vs_sched.c > +++ b/net/netfilter/ipvs/ip_vs_sched.c > @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name) > mutex_unlock(&ip_vs_sched_mutex); > return sched; > } > - if (sched->module) > - module_put(sched->module); > + module_put(sched->module); > } > > mutex_unlock(&ip_vs_sched_mutex); > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c > index eadffb2..cafe28d 100644 > --- a/net/netfilter/ipvs/ip_vs_sync.c > +++ b/net/netfilter/ipvs/ip_vs_sync.c > @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc, > > p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC); > if (!p->pe_data) { > - if (p->pe->module) > - module_put(p->pe->module); > + module_put(p->pe->module); > return -ENOMEM; > } > p->pe_data_len = pe_data_len; > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index deeb95f..b115f54 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb) > break; > case NFT_MSG_NEWCHAIN: > if (nft_trans_chain_update(trans)) { > - if (nft_trans_chain_stats(trans)) > - free_percpu(nft_trans_chain_stats(trans)); > + free_percpu(nft_trans_chain_stats(trans)); > > nft_trans_destroy(trans); > } else { > -- > 2.1.3 Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls 2014-11-19 22:26 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls Julian Anastasov @ 2014-11-20 1:13 ` Simon Horman 2014-11-20 12:16 ` Pablo Neira Ayuso 0 siblings, 1 reply; 10+ messages in thread From: Simon Horman @ 2014-11-20 1:13 UTC (permalink / raw) To: Julian Anastasov Cc: SF Markus Elfring, David S. Miller, Jozsef Kadlecsik, Pablo Neira Ayuso, Patrick McHardy, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle On Thu, Nov 20, 2014 at 12:26:56AM +0200, Julian Anastasov wrote: > > Hello, > > On Tue, 18 Nov 2014, SF Markus Elfring wrote: > > > From: Markus Elfring <elfring@users.sourceforge.net> > > Date: Tue, 18 Nov 2014 20:37:05 +0100 > > > > The functions free_percpu() and module_put() test whether their argument > > is NULL and then return immediately. Thus the test around the call is > > not needed. > > > > This issue was detected by using the Coccinelle software. > > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > > Pablo, the IPVS parts look ok to me, > > Acked-by: Julian Anastasov <ja@ssi.bg> Acked-by: Simon Horman <horms@verge.net.au> > > > --- > > net/netfilter/ipvs/ip_vs_ctl.c | 3 +-- > > net/netfilter/ipvs/ip_vs_pe.c | 3 +-- > > net/netfilter/ipvs/ip_vs_sched.c | 3 +-- > > net/netfilter/ipvs/ip_vs_sync.c | 3 +-- > > net/netfilter/nf_tables_api.c | 3 +-- > > 5 files changed, 5 insertions(+), 10 deletions(-) > > > > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c > > index fd3f444..7c5e40a 100644 > > --- a/net/netfilter/ipvs/ip_vs_ctl.c > > +++ b/net/netfilter/ipvs/ip_vs_ctl.c > > @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc) > > > > static void ip_vs_service_free(struct ip_vs_service *svc) > > { > > - if (svc->stats.cpustats) > > - free_percpu(svc->stats.cpustats); > > + free_percpu(svc->stats.cpustats); > > kfree(svc); > > } > > > > diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c > > index 1a82b29..0df17ca 100644 > > --- a/net/netfilter/ipvs/ip_vs_pe.c > > +++ b/net/netfilter/ipvs/ip_vs_pe.c > > @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name) > > rcu_read_unlock(); > > return pe; > > } > > - if (pe->module) > > - module_put(pe->module); > > + module_put(pe->module); > > } > > rcu_read_unlock(); > > > > diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c > > index 4dbcda6..199760c 100644 > > --- a/net/netfilter/ipvs/ip_vs_sched.c > > +++ b/net/netfilter/ipvs/ip_vs_sched.c > > @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name) > > mutex_unlock(&ip_vs_sched_mutex); > > return sched; > > } > > - if (sched->module) > > - module_put(sched->module); > > + module_put(sched->module); > > } > > > > mutex_unlock(&ip_vs_sched_mutex); > > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c > > index eadffb2..cafe28d 100644 > > --- a/net/netfilter/ipvs/ip_vs_sync.c > > +++ b/net/netfilter/ipvs/ip_vs_sync.c > > @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc, > > > > p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC); > > if (!p->pe_data) { > > - if (p->pe->module) > > - module_put(p->pe->module); > > + module_put(p->pe->module); > > return -ENOMEM; > > } > > p->pe_data_len = pe_data_len; > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index deeb95f..b115f54 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > > @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb) > > break; > > case NFT_MSG_NEWCHAIN: > > if (nft_trans_chain_update(trans)) { > > - if (nft_trans_chain_stats(trans)) > > - free_percpu(nft_trans_chain_stats(trans)); > > + free_percpu(nft_trans_chain_stats(trans)); > > > > nft_trans_destroy(trans); > > } else { > > -- > > 2.1.3 > > Regards > > -- > Julian Anastasov <ja@ssi.bg> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls 2014-11-20 1:13 ` Simon Horman @ 2014-11-20 12:16 ` Pablo Neira Ayuso 0 siblings, 0 replies; 10+ messages in thread From: Pablo Neira Ayuso @ 2014-11-20 12:16 UTC (permalink / raw) To: Simon Horman Cc: Julian Anastasov, SF Markus Elfring, David S. Miller, Jozsef Kadlecsik, Patrick McHardy, Wensong Zhang, netdev, lvs-devel, netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle On Thu, Nov 20, 2014 at 10:13:59AM +0900, Simon Horman wrote: > On Thu, Nov 20, 2014 at 12:26:56AM +0200, Julian Anastasov wrote: > > > > Hello, > > > > On Tue, 18 Nov 2014, SF Markus Elfring wrote: > > > > > From: Markus Elfring <elfring@users.sourceforge.net> > > > Date: Tue, 18 Nov 2014 20:37:05 +0100 > > > > > > The functions free_percpu() and module_put() test whether their argument > > > is NULL and then return immediately. Thus the test around the call is > > > not needed. > > > > > > This issue was detected by using the Coccinelle software. > > > > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > > > > Pablo, the IPVS parts look ok to me, > > > > Acked-by: Julian Anastasov <ja@ssi.bg> > > Acked-by: Simon Horman <horms@verge.net.au> Applied, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] net-ipv6: Delete unnecessary checks before the function call "kfree_skb" [not found] ` <5317A59D.4@users.sourceforge.net> 2014-11-18 19:47 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls SF Markus Elfring @ 2015-11-14 19:05 ` SF Markus Elfring 2015-11-15 3:32 ` Eric Dumazet 1 sibling, 1 reply; 10+ messages in thread From: SF Markus Elfring @ 2015-11-14 19:05 UTC (permalink / raw) To: Alexey Kuznetsov, David S. Miller, Hideaki Yoshfuji, James Morris, Jozsef Kadlecsik, Pablo Neira Ayuso, Patrick McHardy, netdev, netfilter-devel, coreteam Cc: LKML, kernel-janitors, Julia Lawall From: Markus Elfring <elfring@users.sourceforge.net> Date: Sat, 14 Nov 2015 19:55:00 +0100 The kfree_skb() function tests whether its argument is NULL and then returns immediately. Thus the test around the calls is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- net/ipv6/af_inet6.c | 7 ++----- net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c index 44bb66b..4cd9259 100644 --- a/net/ipv6/af_inet6.c +++ b/net/ipv6/af_inet6.c @@ -416,12 +416,9 @@ void inet6_destroy_sock(struct sock *sk) /* Release rx options */ skb = xchg(&np->pktoptions, NULL); - if (skb) - kfree_skb(skb); - + kfree_skb(skb); skb = xchg(&np->rxpmtu, NULL); - if (skb) - kfree_skb(skb); + kfree_skb(skb); /* Free flowlabels */ fl6_free_socklist(sk); diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c index d5efeb8..dbc013b 100644 --- a/net/ipv6/netfilter/nf_conntrack_reasm.c +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c @@ -172,8 +172,7 @@ static unsigned int nf_hashfn(const struct inet_frag_queue *q) static void nf_skb_free(struct sk_buff *skb) { - if (NFCT_FRAG6_CB(skb)->orig) - kfree_skb(NFCT_FRAG6_CB(skb)->orig); + kfree_skb(NFCT_FRAG6_CB(skb)->orig); } static void nf_ct_frag6_expire(unsigned long data) -- 2.6.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] net-ipv6: Delete unnecessary checks before the function call "kfree_skb" 2015-11-14 19:05 ` [PATCH] net-ipv6: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring @ 2015-11-15 3:32 ` Eric Dumazet 2015-11-15 6:17 ` SF Markus Elfring 0 siblings, 1 reply; 10+ messages in thread From: Eric Dumazet @ 2015-11-15 3:32 UTC (permalink / raw) To: SF Markus Elfring Cc: Alexey Kuznetsov, David S. Miller, Hideaki Yoshfuji, James Morris, Jozsef Kadlecsik, Pablo Neira Ayuso, Patrick McHardy, netdev, netfilter-devel, coreteam, LKML, kernel-janitors, Julia Lawall On Sat, 2015-11-14 at 20:05 +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 14 Nov 2015 19:55:00 +0100 > > The kfree_skb() function tests whether its argument is NULL and then > returns immediately. Thus the test around the calls is not needed. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > net/ipv6/af_inet6.c | 7 ++----- > net/ipv6/netfilter/nf_conntrack_reasm.c | 3 +-- > 2 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c > index 44bb66b..4cd9259 100644 > --- a/net/ipv6/af_inet6.c > +++ b/net/ipv6/af_inet6.c > @@ -416,12 +416,9 @@ void inet6_destroy_sock(struct sock *sk) > /* Release rx options */ > > skb = xchg(&np->pktoptions, NULL); > - if (skb) > - kfree_skb(skb); > - > + kfree_skb(skb); > skb = xchg(&np->rxpmtu, NULL); > - if (skb) > - kfree_skb(skb); > + kfree_skb(skb); > There is no 'issue' here, or not this one. In most cases, these pointers are NULL, so the test can be predicted by the processor. While if the test is done in kfree_skb(), the branch predictor of the cpu wont be able to predict things. By feeding too many NULL pointers to kfree_skb(), we slow down it. Branch misses and hits were considered important years ago... But seeing this inet6_destroy_sock() is (ab)using xchg() three times, I am not sure author cared that much about performance. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: net-ipv6: Delete unnecessary checks before the function call "kfree_skb" 2015-11-15 3:32 ` Eric Dumazet @ 2015-11-15 6:17 ` SF Markus Elfring 0 siblings, 0 replies; 10+ messages in thread From: SF Markus Elfring @ 2015-11-15 6:17 UTC (permalink / raw) To: Eric Dumazet Cc: Alexey Kuznetsov, David S. Miller, Hideaki Yoshfuji, James Morris, Jozsef Kadlecsik, Pablo Neira Ayuso, Patrick McHardy, netdev, netfilter-devel, coreteam, LKML, kernel-janitors, Julia Lawall > While if the test is done in kfree_skb(), the branch predictor of the > cpu wont be able to predict things. > > By feeding too many NULL pointers to kfree_skb(), we slow down it. Would it make sense to annotate checks before such function calls as "UNLIKELY"? Regards, Markus ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-11-15 6:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5307CAA2.8060406@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402212321410.2043@localhost6.localdomain6>
[not found] ` <530A086E.8010901@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402231635510.1985@localhost6.localdomain6>
[not found] ` <530A72AA.3000601@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402240658210.2090@localhost6.localdomain6>
[not found] ` <530B5FB6.6010207@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402241710370.2074@hadrien>
[not found] ` <530C5E18.1020800@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402251014170.2080@hadrien>
[not found] ` <530CD2C4.4050903@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402251840450.7035@hadrien>
[not found] ` <530CF8FF.8080600@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402252117150.2047@localhost6.localdomain6>
[not found] ` <530DD06F.4090703@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402262129250.2221@localhost6.localdomain6>
[not found] ` <5317A59D.4@users.so urceforge.net>
[not found] ` <5317A59D.4@users.sourceforge.net>
2014-11-18 19:47 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls SF Markus Elfring
2014-11-19 13:40 ` Pablo Neira Ayuso
2015-07-02 15:10 ` [PATCH] net-ipvs: Delete an unnecessary check before the function call "module_put" SF Markus Elfring
2015-07-09 1:41 ` Simon Horman
2014-11-19 22:26 ` [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls Julian Anastasov
2014-11-20 1:13 ` Simon Horman
2014-11-20 12:16 ` Pablo Neira Ayuso
2015-11-14 19:05 ` [PATCH] net-ipv6: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring
2015-11-15 3:32 ` Eric Dumazet
2015-11-15 6:17 ` SF Markus Elfring
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).