4.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Cong Wang xiyou.wangcong@gmail.com
[ Upstream commit 4e407ff5cd67ec76eeeea1deec227b7982dc7f66 ]
The only time we need to take tcfa_lock is when adding a new metainfo to an existing ife->metalist. We don't need to take tcfa_lock so early and so broadly in tcf_ife_init().
This means we can always take ife_mod_lock first, avoid the reverse locking ordering warning as reported by Vlad.
Reported-by: Vlad Buslov vladbu@mellanox.com Tested-by: Vlad Buslov vladbu@mellanox.com Cc: Vlad Buslov vladbu@mellanox.com Cc: Jamal Hadi Salim jhs@mojatatu.com Signed-off-by: Cong Wang xiyou.wangcong@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/sched/act_ife.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-)
--- a/net/sched/act_ife.c +++ b/net/sched/act_ife.c @@ -265,10 +265,8 @@ static const char *ife_meta_id2name(u32 #endif
/* called when adding new meta information - * under ife->tcf_lock for existing action */ -static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid, - void *val, int len, bool exists) +static int load_metaops_and_vet(u32 metaid, void *val, int len) { struct tcf_meta_ops *ops = find_ife_oplist(metaid); int ret = 0; @@ -276,13 +274,9 @@ static int load_metaops_and_vet(struct t if (!ops) { ret = -ENOENT; #ifdef CONFIG_MODULES - if (exists) - spin_unlock_bh(&ife->tcf_lock); rtnl_unlock(); request_module("ife-meta-%s", ife_meta_id2name(metaid)); rtnl_lock(); - if (exists) - spin_lock_bh(&ife->tcf_lock); ops = find_ife_oplist(metaid); #endif } @@ -299,10 +293,9 @@ static int load_metaops_and_vet(struct t }
/* called when adding new meta information - * under ife->tcf_lock for existing action */ static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval, - int len, bool atomic) + int len, bool atomic, bool exists) { struct tcf_meta_info *mi = NULL; struct tcf_meta_ops *ops = find_ife_oplist(metaid); @@ -329,12 +322,16 @@ static int add_metainfo(struct tcf_ife_i } }
+ if (exists) + spin_lock_bh(&ife->tcf_lock); list_add_tail(&mi->metalist, &ife->metalist); + if (exists) + spin_unlock_bh(&ife->tcf_lock);
return ret; }
-static int use_all_metadata(struct tcf_ife_info *ife) +static int use_all_metadata(struct tcf_ife_info *ife, bool exists) { struct tcf_meta_ops *o; int rc = 0; @@ -342,7 +339,7 @@ static int use_all_metadata(struct tcf_i
read_lock(&ife_mod_lock); list_for_each_entry(o, &ifeoplist, list) { - rc = add_metainfo(ife, o->metaid, NULL, 0, true); + rc = add_metainfo(ife, o->metaid, NULL, 0, true, exists); if (rc == 0) installed += 1; } @@ -419,7 +416,6 @@ static void tcf_ife_cleanup(struct tc_ac kfree_rcu(p, rcu); }
-/* under ife->tcf_lock for existing action */ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb, bool exists) { @@ -433,11 +429,11 @@ static int populate_metalist(struct tcf_ val = nla_data(tb[i]); len = nla_len(tb[i]);
- rc = load_metaops_and_vet(ife, i, val, len, exists); + rc = load_metaops_and_vet(i, val, len); if (rc != 0) return rc;
- rc = add_metainfo(ife, i, val, len, exists); + rc = add_metainfo(ife, i, val, len, false, exists); if (rc) return rc; } @@ -531,8 +527,6 @@ static int tcf_ife_init(struct net *net, p->eth_type = ife_type; }
- if (exists) - spin_lock_bh(&ife->tcf_lock);
if (ret == ACT_P_CREATED) INIT_LIST_HEAD(&ife->metalist); @@ -544,9 +538,6 @@ static int tcf_ife_init(struct net *net, metadata_parse_err: if (ret == ACT_P_CREATED) tcf_idr_release(*a, bind); - - if (exists) - spin_unlock_bh(&ife->tcf_lock); kfree(p); return err; } @@ -561,18 +552,17 @@ metadata_parse_err: * as we can. You better have at least one else we are * going to bail out */ - err = use_all_metadata(ife); + err = use_all_metadata(ife, exists); if (err) { if (ret == ACT_P_CREATED) tcf_idr_release(*a, bind); - - if (exists) - spin_unlock_bh(&ife->tcf_lock); kfree(p); return err; } }
+ if (exists) + spin_lock_bh(&ife->tcf_lock); ife->tcf_action = parm->action; if (exists) spin_unlock_bh(&ife->tcf_lock);