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, URIBL_BLOCKED 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 37FF0C43381 for ; Tue, 26 Feb 2019 14:57:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EDA15217F5 for ; Tue, 26 Feb 2019 14:57:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="Xgy78GTt" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727998AbfBZO5g (ORCPT ); Tue, 26 Feb 2019 09:57:36 -0500 Received: from mail-eopbgr10080.outbound.protection.outlook.com ([40.107.1.80]:10201 "EHLO EUR02-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727506AbfBZO5g (ORCPT ); Tue, 26 Feb 2019 09:57:36 -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=j0e7BljjV8NA9sD/PzuesUgqqM8xJm3wiMMpJhSfQYs=; b=Xgy78GTtrGfpkLubJDRiOkpW46lwhzSYRFejtK6UcY5U/ETaiDKg9wGtBpcD9p3mpW8sVPaRnmeLRtEebH+NvENqv8q1LUmjCM38B/BbC07XzDuUpp1yudSIWYyoizIgEVHyJI24rV1OggLb7yNJQ4QZe8ohVZITUrvq8eCqQ1k= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3627.eurprd05.prod.outlook.com (10.167.126.161) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.18; Tue, 26 Feb 2019 14:57:28 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749%5]) with mapi id 15.20.1643.019; Tue, 26 Feb 2019 14:57:28 +0000 From: Vlad Buslov To: Cong Wang CC: Linux Kernel Network Developers , Jamal Hadi Salim , Jiri Pirko , David Miller Subject: Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference Thread-Topic: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference Thread-Index: AQHUxDmfnNJQQFjgm0S2EuLlSiVKMKXl8iaAgADz/YCAAmnzgIABQWoAgAGwuACABH4tAIAAbR8AgAERLoA= Date: Tue, 26 Feb 2019 14:57:28 +0000 Message-ID: References: <20190214074712.17846-1-vladbu@mellanox.com> <20190214074712.17846-2-vladbu@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LO2P265CA0293.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a5::17) 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: c23973aa-b0b6-4afa-99ad-08d69bfab998 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);SRVR:HE1PR0502MB3627; x-ms-traffictypediagnostic: HE1PR0502MB3627: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3627;23:2IHq5yqemCGmCrR2sacf6+J6y7nJMfF+MVQ77?= =?iso-8859-1?Q?QeAjg5GI9JzUsMot7ekPy/rt/fa6xx3xemwvO49IkTGVH8Q6atH6coselP?= =?iso-8859-1?Q?a85ii1xlJbRJnTLXPQ7XpFyKdg7mxSCd7mnc7gkCNCcDcpn8QVjERqT6UE?= =?iso-8859-1?Q?ePTZFQEnt6G/mnhHvL6l7FUY/Mny1/wMeN1qT5zAVeMWUQAS0c1MRzQjqx?= =?iso-8859-1?Q?4YZtApzwX1W1Pa1QRMPLf1PNTqXVyGoLl6f1oI7ZhKDl0ko3p4ISWFrvvW?= =?iso-8859-1?Q?h9Z4MnQi4/WyExM/mEisw9EdTXTmLYQ21r8SAa/eW2IvE/bhpESV0nDaGn?= =?iso-8859-1?Q?TJlNByrX++B1bWeaKBipPPvb20Ed8oGjKoPforJIPoioqBNEgn0GgNtlLU?= =?iso-8859-1?Q?o8SxquYr+M7z/QzzwvoX/5xu24L9b0PFTk2JuKqmyjiSfpxxhDPmoRCHwD?= =?iso-8859-1?Q?cYISG+kL6tINXJJs1HFVuYo5U+yIOW5Nh5EFKlU19dnU8lZbOdpbKYOOFr?= =?iso-8859-1?Q?1ilKA7TFh8bP5RTeQXkaSxAQbWRot86Uz+kfGvDm0tB4KVvWuXiw4bB/3+?= =?iso-8859-1?Q?PHVKGfbkGpqpGRq2I4x81rSEjYQdrl/irwqW5W+mgpeusRxhB2Ymjmn6XQ?= =?iso-8859-1?Q?1Qxj4T4qCZMTM+LSe3eBvlDHCvTX5WtAXvZk/w3C4n3Pv/63jLsbb+gUgE?= =?iso-8859-1?Q?tALQwRZhKAkOqlJd3b7hqG0IuTZTSst98csILm2j03t+DhJEQLLGEdsXmF?= =?iso-8859-1?Q?+DqqwPTA60QCj72/3d+fQGQxY2gDkaKPGQVJNqxhQTM4+HpZ5Er6HlJFBT?= =?iso-8859-1?Q?92+o6z/fk3i1sgIsipFbmCdn8m6HyJ8++50wD5who9vvqgG+BnM47GHvS3?= =?iso-8859-1?Q?AHOJ0oNDmt5+s8vQH1IEso/AEmDQ4kVBmtuC54Elaw9kvnuINDDVEgNsyJ?= =?iso-8859-1?Q?1Ny6SZK1as6C8jGAmC4x+HtQluHkmClrdckHaPlu4aYKU/fW1YWOMs5dDK?= =?iso-8859-1?Q?jQ+8yh7MYIXU40ziHLBKefeCeADCtl1RJjDZhaFez+RrL1dAn1DoZCrJ2o?= =?iso-8859-1?Q?eakH6DgPL5UzjHN2ue+eXa20yPwBTHyRhNl8j2JFWMqOACUtt74wV6xYkr?= =?iso-8859-1?Q?U/z19eb5B9ukJj2iIsOMD7AnaqxTKarkDb+TxbGNofr1xOjTmFSJHhb6JZ?= =?iso-8859-1?Q?Sn4TIVS9IjL/XlIaWYkdE3T/5iXzBCdSFbyabw6vnGSYi5S3DiLJc8P5I/?= =?iso-8859-1?Q?x1yqYydHmsWmoqLRwyZGQ1ErabxcTHnqZINAbp8XvHE+MrTuZ0wBbHtjDi?= =?iso-8859-1?Q?nbwFqKMNPY5lZuSrYkXMgS+aB?= x-microsoft-antispam-prvs: x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(346002)(136003)(376002)(366004)(396003)(39860400002)(199004)(189003)(102836004)(6506007)(53936002)(26005)(25786009)(53546011)(71190400001)(71200400001)(81166006)(81156014)(8936002)(66066001)(3846002)(6116002)(36756003)(8676002)(186003)(386003)(106356001)(7736002)(305945005)(229853002)(2616005)(476003)(4326008)(105586002)(486006)(6436002)(99286004)(6486002)(6916009)(11346002)(446003)(256004)(97736004)(6512007)(14444005)(6246003)(52116002)(76176011)(478600001)(93886005)(68736007)(316002)(2906002)(54906003)(86362001)(14454004)(5660300002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3627;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX: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: Kn3/Yts3zqInbmPFYABvfjRz/Pb9t3ORWFWEb6sdgXjfxCLMJGGwVgyx48COftK+WAuyCxytE6dXBeGMddVRYp7AC3VBzMnYufl71HyrHMxR7wYj+s9Lcdv7jpTMfaABV+jjs93+z8MM6AgDpBVSwwiNhM5qC8G15+Xjm2KezaQi3IRRcNAFoJew2j5A1E1obstBrmjsjpbQepOa4y8lqmPKYE4/khZOTqAKr/oSVX2SD8b42tQPr2ncz6dXmPECXWpSpVuAf4AHj21uwe1+4+4H7KfRRvRGl5a4Y8DEHAu4cufuI2fhTkDkDKdf/H99g0jw+er4Cv8oEjelgU1weX76OJpuZpPAwDr2bRZ2XCwDLmvwhXwoKPafC0PN8jXaR1YzBB61Ps5VPoSd0yVIU6Jjd51uw8yRWQUlfr7WK6Y= 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: c23973aa-b0b6-4afa-99ad-08d69bfab998 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2019 14:57:26.9162 (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: HE1PR0502MB3627 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon 25 Feb 2019 at 22:39, Cong Wang wrote: > On Mon, Feb 25, 2019 at 8:11 AM Vlad Buslov wrote: >> >> >> On Fri 22 Feb 2019 at 19:32, Cong Wang wrote: >> > >> > So if it is no longer RCU any more, why do you still use >> > rcu_dereference_protected()? That is, why not just deref it as a raw >> > pointer? > > > Any answer for this question? I decided that since there is neither possibility of concurrent pointer assignment nor deallocation of object that it points to, most performant solution would be using rcu_dereference_protected() which is the only RCU dereference helper that doesn't use READ_ONCE. I now understand that this is confusing (and most likely doesn't provide any noticeable performance improvement anyway!) and will change this patch to use rcu_dereference_raw() as you suggest. > > >> > >> > And, I don't think I can buy your argument here. The RCU infrastructur= e >> > should not be changed even after your patches, the fast path is still >> > protocted by RCU read lock, while the slow path now is protected by >> > some smaller-scope locks. What makes cls_flower so unique that >> > it doesn't even need RCU here? tp->root is not reassigned but it is st= ill >> > freed via RCU infra, that is in fl_destroy_sleepable(). >> > >> > Thanks. >> >> My cls API patch set introduced reference counting for tcf_proto >> structure. With that change tp->ops->destroy() (which calls fl_destroy() >> and fl_destroy_sleepable(), in case of flower classifier) is only called >> after last reference to tp is released. All slow path users of tp->ops >> must obtain reference to tp, so concurrent call to fl_destroy() is not >> possible. Before this change tcf_proto structure didn't have reference >> counting support and required users to obtain rtnl mutex before calling >> its ops callbacks. This was verified in flower by using rtnl_dereference >> to obtain tp->root. > > Yes, but fast path doesn't hold a refnct of tp, does it? If not, you stil= l > rely on RCU for sync with readers. If yes, then probably RCU can be > gone. > > Now you are in a middle of the two, that is taking RCU read lock on > fast path without a refcnt, meanwhile still uses rcu_dereference on > slow paths without any lock. > > For me, you at least don't use the RCU API correctly here. > > Thanks. Yes, fast path still relies on RCU. What I meant is that slow path (cls API) now only calls tp ops after obtaining reference to tp, so there is no need to protect it from concurrent tp->ops->destroy() by means of rtnl or any other lock. I understand that using rcu_dereference_protected() is confusing in this case and will refactor this patch appropriately.