From patchwork Tue Jun 13 20:36:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 775451 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3wnM4S0tv8z9s78 for ; Wed, 14 Jun 2017 06:36:40 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="GFxVEryV"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753891AbdFMUgi (ORCPT ); Tue, 13 Jun 2017 16:36:38 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:33761 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752695AbdFMUgh (ORCPT ); Tue, 13 Jun 2017 16:36:37 -0400 Received: by mail-io0-f194.google.com with SMTP id j200so9783091ioe.0 for ; Tue, 13 Jun 2017 13:36:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=andlEkpD+GcOZ0MLAsYHnR3VlbxZPQ9+/ZdU8EsbUMQ=; b=GFxVEryVlFM410iKktp4fSafPLjdCopXYFeHHEHj715gGzVGhyBrE05+78SSgYY1zX GYKwapNstZWFyYeZHfXDv2Ii05tnSMPhNf0eLOPcoqt0JyWd4sZwgboSEArkd93Q+2YO t/ymEXiMe/S1jHtNovSItgenGpkpJxD+ODnVwgT/luV7vmaELb1uQCP3QF1Al8ZKznam e0Hc+b5vvBtdPF6yDDPEiTVuEgxQGfSddiTq8RDir7RoA+QeMwCS3FBzPreXnK6F1lcC SOMtH6OHLx6txUYrKTL6wVz2Y7lvDICuCojO3Q+2xCfHGnkuhEOU4x65O7l/6XnSP3Pg Jfww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=andlEkpD+GcOZ0MLAsYHnR3VlbxZPQ9+/ZdU8EsbUMQ=; b=iMO5CfNj57M/MQ3GLFZUihzG6HoldVO05Nv42UMr6rOx4J7ncaKwsdLdR7pk8llHkm TvQuN28IWzWfQdbc4tMbtGxsNe3M0ubFRGfYaArpctLlU/P+vLfeIWv1HirwfTfTwDNB l7mxqdXMiG5rewrfejDePwpn+iiGhkA8YDS+IYi4xdt+rxy7i1baxNFm69cwdXbgH6DL 4ATuDiFWNphydKTxkjMBPn6agq552xHlKmyjHIjw9sjrqU39MULeytfGZWdnTyhDCveH Qn1InojogrOREXncasnsWYBNvStpUchETYrpfpEfz+PJJWfsQ5rOMv40xYcVlaKeKVGe dfRw== X-Gm-Message-State: AKS2vOyRXUr27tyCrS4KglApIoHSmRECHec4angwMDHo3lPgNf9m2I2G QVt9mHyl2aOXpENNGNI= X-Received: by 10.107.47.163 with SMTP id v35mr2162239iov.201.1497386196195; Tue, 13 Jun 2017 13:36:36 -0700 (PDT) Received: from tw-172-25-30-113.office.twttr.net ([8.25.197.25]) by smtp.gmail.com with ESMTPSA id j75sm2062621itj.4.2017.06.13.13.36.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Jun 2017 13:36:35 -0700 (PDT) From: Cong Wang To: netdev@vger.kernel.org Cc: nicholashuber@gmail.com, labbott@redhat.com, Cong Wang , Jamal Hadi Salim Subject: [Patch net] net_sched: move tcf_lock down after gen_replace_estimator() Date: Tue, 13 Jun 2017 13:36:24 -0700 Message-Id: <1497386184-14960-1-git-send-email-xiyou.wangcong@gmail.com> X-Mailer: git-send-email 2.5.5 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Laura reported a sleep-in-atomic kernel warning inside tcf_act_police_init() which calls gen_replace_estimator() with spinlock protection. It is not necessary in this case, we already have RTNL lock here so it is enough to protect concurrent writers. For the reader, i.e. tcf_act_police(), it needs to make decision based on this rate estimator, in the worst case we drop more/less packets than necessary while changing the rate in parallel, it is still acceptable. Reported-by: Laura Abbott Reported-by: Nick Huber Cc: Jamal Hadi Salim Signed-off-by: Cong Wang Acked-by: Jamal Hadi Salim --- net/sched/act_police.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/sched/act_police.c b/net/sched/act_police.c index f42008b..b062bc8 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -132,21 +132,21 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla, } } - spin_lock_bh(&police->tcf_lock); if (est) { err = gen_replace_estimator(&police->tcf_bstats, NULL, &police->tcf_rate_est, &police->tcf_lock, NULL, est); if (err) - goto failure_unlock; + goto failure; } else if (tb[TCA_POLICE_AVRATE] && (ret == ACT_P_CREATED || !gen_estimator_active(&police->tcf_rate_est))) { err = -EINVAL; - goto failure_unlock; + goto failure; } + spin_lock_bh(&police->tcf_lock); /* No failure allowed after this point */ police->tcfp_mtu = parm->mtu; if (police->tcfp_mtu == 0) { @@ -192,8 +192,6 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla, return ret; -failure_unlock: - spin_unlock_bh(&police->tcf_lock); failure: qdisc_put_rtab(P_tab); qdisc_put_rtab(R_tab);