From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56152C43381 for ; Mon, 18 Feb 2019 10:07:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1890821479 for ; Mon, 18 Feb 2019 10:07:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="Wprt3L8f" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729570AbfBRKHs (ORCPT ); Mon, 18 Feb 2019 05:07:48 -0500 Received: from mail-eopbgr00067.outbound.protection.outlook.com ([40.107.0.67]:38640 "EHLO EUR02-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729460AbfBRKHs (ORCPT ); Mon, 18 Feb 2019 05:07:48 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mmYZcMdH1X053ro0b9I7SLPOcHXP+Gizf7YkJUqpMTw=; b=Wprt3L8fNFmgOc817en1kO9rxt5j3kNolecKGCGrQuNFwJ8UiGCZx6uxRN1tgZSdq3NGT3yGlOnTFk/ocuK7SLC88cGmWW4XpkPxtPgoLmNF42mDztID9xjKAgliaxZJ377pDkpfbkVAN2/8R6RdCj4zDitPAZAEyPf6rlIXqHw= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3018.eurprd05.prod.outlook.com (10.175.31.13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1622.18; Mon, 18 Feb 2019 10:07:05 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::4041:bb68:2af3:eab8]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::4041:bb68:2af3:eab8%5]) with mapi id 15.20.1622.018; Mon, 18 Feb 2019 10:07:05 +0000 From: Vlad Buslov To: Cong Wang CC: Linux Kernel Network Developers , Jamal Hadi Salim , Jiri Pirko , David Miller , Alexei Starovoitov , Daniel Borkmann Subject: Re: [PATCH net-next v4 05/17] net: sched: traverse chains in block with tcf_get_next_chain() Thread-Topic: [PATCH net-next v4 05/17] net: sched: traverse chains in block with tcf_get_next_chain() Thread-Index: AQHUwefCnpHBoUvaMEWLwFR2bAA8fKXhdauAgAPp5QA= Date: Mon, 18 Feb 2019 10:07:04 +0000 Message-ID: References: <20190211085548.7190-1-vladbu@mellanox.com> <20190211085548.7190-6-vladbu@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LO2P265CA0318.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a4::18) To HE1PR0502MB3641.eurprd05.prod.outlook.com (2603:10a6:7:85::11) authentication-results: spf=none (sender IP is ) smtp.mailfrom=vladbu@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [37.142.13.130] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 86d5c206-a996-4674-0fe2-08d69588d547 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(5600110)(711020)(4605104)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020);SRVR:HE1PR0502MB3018; x-ms-traffictypediagnostic: HE1PR0502MB3018: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3018;23:kmOR164TnVTrKX8U9S9t4i/b5eVc3aBwnQIo9?= =?iso-8859-1?Q?+pJTo+7dELluv01bDWMxI2JsyJTiksL2P+JSlxY1QG99Za0uy41hmffpa6?= =?iso-8859-1?Q?YnDejqwoQ18xEbc99OCbwSGcn+d7QQHsPZuVTf1La/xuB/aYhw6qEpuSrN?= =?iso-8859-1?Q?GL2L/Cm78ErtOqtQWe1bn6zcSIzdbr7Zj2OfrBf1mQ9wxxaBsbPkio62bX?= =?iso-8859-1?Q?ja/c+08uQxj4v+Wy94eZ1ReeZUBJdjhstND4gPUAt/tPfHn04tg3M/TqRl?= =?iso-8859-1?Q?7IgrFiVBLIuLTLjOLdkFt23C0shCFylF7ub0BN+UdUU65b8owf7l0nJDqu?= =?iso-8859-1?Q?oyDT6WcJZ1F26ywNnvSXwyaGMuFPl82aTyXIARsQv8aspAjpDqBwbTB5LJ?= =?iso-8859-1?Q?VahoFPhn87aDgIUQiWgH7a74LRiAETOfEvq+4A+HGSUxmnUWm0TUfnvwS7?= =?iso-8859-1?Q?HHKtk1kkuZdfpVruP/N9lK6spgCroqDC6//8BS0ZGXLzQC3tXEtn07dhKI?= =?iso-8859-1?Q?Qg0bnuP2T2dsodFHydr/I6pGDxU5HPMMki0Tsuw86XCB9xtFqYVTYamRB6?= =?iso-8859-1?Q?WVLPljopK7h0nLFbFxUmOQQoct0wUHQM2hwv67Ye0AOBjo5mhxnsMTG/pp?= =?iso-8859-1?Q?DeuAndymXNpeOgtFSmhDGvCm4H+U9aQpT87xx5AKpDMcIPmd/3LHba2neV?= =?iso-8859-1?Q?Mhmwzq2Gy9cB7r4MMet90oQNYew9a5FjCDT83GK1o/r90yoG8rYw1AWqXC?= =?iso-8859-1?Q?rv3FlY+g9dC4BoQKAQgAlOATB5dvkTzELYoXTIVf+pQAGrLlZ5J9YsnrEe?= =?iso-8859-1?Q?B4nUNWl7A8E6JjRS4nQVGWW1MxllclDskRdhv8AS6fzKcIjrOdagZkr8p7?= =?iso-8859-1?Q?n1c/MYlO7oBDB0Xd+jXtUNSvLKe+Y+FMH86yPoZiz3CpBRprqqy3mx0ge4?= =?iso-8859-1?Q?oo7jXWpxDhOOJY+X+Eek2jH2Y83bvI3k57uzrxxAKs3THlMrskdBVO7Zif?= =?iso-8859-1?Q?32AAg693RE0VAeMKKNf7fqlDRaHv7H5tXaC2MjfDLhSYV7/4iRIrw9jMKU?= =?iso-8859-1?Q?JbeNAH8Syk2drUNLZBTmZJrRDhHSBmAl6IIchGD54iKgB75TGOo6YeNIv9?= =?iso-8859-1?Q?Sxbpq7nF575kvLNjT4IV5SqwqdTF3UbFXkgZHXDNrL/HKWYl7k98n8lYVK?= =?iso-8859-1?Q?lK/lB3HsRV3LIFIpbsNzSQK4ufoyu0Kwfu4fsvOLpPl4JVDDPS2T1siu4R?= =?iso-8859-1?Q?1JZoqj/SfQq1YLqhU5BEdT4ThydbXoILZ/GU87ih0rxlRe5aYsU3f6oDDV?= =?iso-8859-1?Q?q9/A=3D?= x-microsoft-antispam-prvs: x-forefront-prvs: 09525C61DB x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(376002)(136003)(39860400002)(366004)(396003)(346002)(189003)(199004)(316002)(97736004)(5660300002)(52116002)(68736007)(305945005)(102836004)(186003)(26005)(8936002)(54906003)(11346002)(25786009)(446003)(4326008)(76176011)(66066001)(2616005)(14454004)(6246003)(229853002)(36756003)(106356001)(53936002)(6916009)(71200400001)(71190400001)(105586002)(6436002)(14444005)(2906002)(486006)(6116002)(99286004)(3846002)(6486002)(476003)(386003)(6512007)(6506007)(8676002)(81156014)(7736002)(86362001)(53546011)(81166006)(478600001)(256004);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3018;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: awNolq3hz0SsGpuLwK6Ps3NeQTrDp6m2tckea/M+fePybxA39cJ8KOhnEmULQr5jkzwiSWavM3/Io4Ygbe+df1tG88CauOzswFtn5Qt7x5TkrJiudM6wdmN4r4xAXZ+kEIMlHw70E7Vw5YLl6SH1vFaMqtQ4J8Mb7TdPlvHY20ZzT3A6s9i3Llt0zda18wmCEqjyEldB66/pH+opoKDiDcYaViYqyW+7n7KdpOQYQO7mXMmM+x5pks81GGtVw22h1SZAxdYxW/CfKeSfBvh+SUiw4Kuf2Ek3ZUofOwiKoZJcVlFAj0p9iATVaxxSNsWBnXL511rccJEVo6br2dd8+nT/7S6C8/wg/nd1zazFw+q/Yw+Z0+RCKFsBodOrZnEicTbG7JXBEDrSjiYaDqiXQjIa6D/vgLq3HOnvbKzchB0= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 86d5c206-a996-4674-0fe2-08d69588d547 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Feb 2019 10:07:03.6071 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3018 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Cong, Thanks for reviewing! On Fri 15 Feb 2019 at 22:21, Cong Wang wrote: > (Sorry for joining this late.) > > On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov wrote: >> @@ -2432,7 +2474,11 @@ static int tc_dump_chain(struct sk_buff *skb, str= uct netlink_callback *cb) >> index_start =3D cb->args[0]; >> index =3D 0; >> >> - list_for_each_entry(chain, &block->chain_list, list) { >> + for (chain =3D __tcf_get_next_chain(block, NULL); >> + chain; >> + chain_prev =3D chain, >> + chain =3D __tcf_get_next_chain(block, chain), >> + tcf_chain_put(chain_prev)) { > > Why do you want to take the block->lock in each iteration > of the loop rather than taking once for the whole loop? This loop calls classifier ops callback in tc_chain_fill_node(). I don't call any classifier ops callbacks while holding block or chain lock in this change because the goal is to achieve fine-grained locking for data structures used by filter update path. Locking per-block or per-chain is much coarser than taking reference counters to parent structures and allowing classifiers to implement their own locking. In this case call to ops->tmplt_dump() is probably quite fast and its execution time doesn't depend on number of filters on the classifier, so releasing block->lock on each iteration doesn't provide much benefit, if at all. However, it is easier for me to reason about locking correctness in this refactoring by following a simple rule that no locks (besides rtnl mutex) can be held when calling classifier ops callbacks.