lkml.org 
[lkml]   [2018]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 018/115] act_ife: move tcfa_lock down to where necessary
    Date
    4.14-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 | 37 +++++++++++++------------------------
    1 file changed, 13 insertions(+), 24 deletions(-)

    --- a/net/sched/act_ife.c
    +++ b/net/sched/act_ife.c
    @@ -249,10 +249,8 @@ static int ife_validate_metatype(struct
    }

    /* 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;
    @@ -260,13 +258,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("ifemeta%u", metaid);
    rtnl_lock();
    - if (exists)
    - spin_lock_bh(&ife->tcf_lock);
    ops = find_ife_oplist(metaid);
    #endif
    }
    @@ -283,10 +277,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);
    @@ -313,12 +306,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;
    @@ -326,7 +323,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;
    }
    @@ -398,7 +395,6 @@ static void tcf_ife_cleanup(struct tc_ac
    spin_unlock_bh(&ife->tcf_lock);
    }

    -/* under ife->tcf_lock for existing action */
    static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
    bool exists)
    {
    @@ -412,11 +408,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;
    }
    @@ -481,6 +477,8 @@ static int tcf_ife_init(struct net *net,
    if (exists)
    spin_lock_bh(&ife->tcf_lock);
    ife->tcf_action = parm->action;
    + if (exists)
    + spin_unlock_bh(&ife->tcf_lock);

    if (parm->flags & IFE_ENCODE) {
    if (daddr)
    @@ -508,9 +506,6 @@ metadata_parse_err:
    tcf_idr_release(*a, bind);
    if (ret == ACT_P_CREATED)
    _tcf_ife_cleanup(*a, bind);
    -
    - if (exists)
    - spin_unlock_bh(&ife->tcf_lock);
    return err;
    }

    @@ -524,20 +519,14 @@ 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_ife_cleanup(*a, bind);
    -
    - if (exists)
    - spin_unlock_bh(&ife->tcf_lock);
    return err;
    }
    }

    - if (exists)
    - spin_unlock_bh(&ife->tcf_lock);
    -
    if (ret == ACT_P_CREATED)
    tcf_idr_insert(tn, *a);


    \
     
     \ /
      Last update: 2018-09-13 16:16    [W:4.007 / U:0.456 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site