From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Gunthorpe Subject: Re: [PATCH] net/mlx4_en: ensure rx_desc updating reaches HW before prod db updating Date: Sat, 13 Jan 2018 12:15:58 -0700 Message-ID: <20180113191558.GC32353@ziepe.ca> References: <1515728542-3060-1-git-send-email-jianchao.w.wang@oracle.com> <20180112163247.GB15974@ziepe.ca> <1515775567.131759.42.camel@gmail.com> <85116e56-52b1-944d-6ee2-916ccfc3a7a6@mellanox.com> <1515788191.131759.48.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Saeed Mahameed Cc: Eric Dumazet , Jianchao Wang , tariqt-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org, junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-rdma@vger.kernel.org On Fri, Jan 12, 2018 at 01:01:56PM -0800, Saeed Mahameed wrote: > Simply putting a memory barrier on the top or the bottom of a functions, > means nothing unless you are looking at the whole picture, of all the > callers of that function to understand why is it there. When I review code I want to see the memory barrier placed *directly* before the write which allows the DMA. So yes, this is my preference: > update_doorbell() { > dma_wmb(); > ring->db = prod; > } Conceptually what is happening here is very similar to what smp_store_release() does for SMP cases. In most cases wmb should always be strongly connected with a following write. smp_store_release() is called 'release' because the write it incorporates allows the other CPU to 'see' what is being protected. Similarly here, the write to the db allows the device to 'see' the new ring data. And this is bad idea: > fill buffers(); > dma_wmb(); > update_doorbell(); > I simply like the 2nd one since with one look you can understand > what this dma_wmb is protecting. What do you think the wmb is protecting in the above? It isn't the fill. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753170AbeAMTQS (ORCPT + 1 other); Sat, 13 Jan 2018 14:16:18 -0500 Received: from mail-he1eur01on0086.outbound.protection.outlook.com ([104.47.0.86]:64865 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750933AbeAMTQO (ORCPT ); Sat, 13 Jan 2018 14:16:14 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=jgg@mellanox.com; Date: Sat, 13 Jan 2018 12:15:58 -0700 From: Jason Gunthorpe To: Saeed Mahameed Cc: Eric Dumazet , Jianchao Wang , tariqt@mellanox.com, junxiao.bi@oracle.com, netdev@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] net/mlx4_en: ensure rx_desc updating reaches HW before prod db updating Message-ID: <20180113191558.GC32353@ziepe.ca> References: <1515728542-3060-1-git-send-email-jianchao.w.wang@oracle.com> <20180112163247.GB15974@ziepe.ca> <1515775567.131759.42.camel@gmail.com> <85116e56-52b1-944d-6ee2-916ccfc3a7a6@mellanox.com> <1515788191.131759.48.camel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [70.74.179.152] X-ClientProxiedBy: BN6PR03CA0005.namprd03.prod.outlook.com (2603:10b6:404:23::15) To HE1PR0501MB2858.eurprd05.prod.outlook.com (2603:10a6:3:c1::23) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: e26c8bc2-c4ea-415b-54cd-08d55aba1ab2 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(2017052603307)(7153060)(7193020);SRVR:HE1PR0501MB2858; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0501MB2858;3:bMasoiu0ynD/Mcd0IQifNEEphht6BRKRGBJCkcsBsG9Us/21zg1vkJgZMkZEqX5tRHD1wsh9P5Wq6IZDL/cKzdY0uXuWmKbVJtWMIpOuq7kL4YXRra+oqYBCACkEtUYQJp5KdaO+2cyN4L2iBIR1UzI+a2LQx/mQuVvOslMBsoysOwh4FrY26RbbpdsZ/+8t+8p9s1JZTmrBytpo6UIhN14VmVp0ypV4Cu/s8rKvzjgHYi4ECl8vTm7L4hoPGQcB;25:KG01msJ7eX8/Hbi0ocwrMvz+tafq2tkjSn/Aa1O1r3hgl8bybF4pZMfI3F2BGVykHRwl0KACzkLAWZ5bNrb6HZMn58YYZvY6qU5rrQYLdYPO5MBBaiX2EZxL/n942MLbFkqK/j/kuwp4UmmS/qx23HkNUtaEO7Kt62rDnD+Uq28T0EbG/NrksoxuCDBtAan6mx43NiXHRjVYEgSWfczqDKgk+Y2G09VBMZS2kdHJKLDJZfPEv7ocNnZlmqxPRjKmVHZHtf7tvkxgEp2USSdO+aaUwfn2CQiQvai75n993hLY3QePcqKPa5/TbtTBtk5wJNq9/jwe8XoD1+0r1cbeTQ==;31:9tAGONKaMnotvhUHy0Y87zqbGM1QeAwo/ORK0gaxm7HD+o/cpA1JxkQanHWjlzu1hgndWDhJs92SCVuTkPnSyhxJFkEqC/4x0ta3Of/dUMiCfNaLR2psoLnCkqoFIw1//TxrVH5aSeS7z/513Eb12i9d6J5IRmB3nOBetHZO3gNwd+OPmXjo6V+cU7RpK/XfctVPV68M/wJ88C7XJxjW2wQdvDJTnWDEz86ACgBHgi4= X-MS-TrafficTypeDiagnostic: HE1PR0501MB2858: X-Microsoft-Exchange-Diagnostics: 1;HE1PR0501MB2858;20:YMaTKRrstcHbSoCbIqQSTPct4R409aO6ebn5mUsVsLNLzVMHseN3XktEMBmRA9AKo8e782vfAep5smARAS5Sctfp3Jrj0pTisKcbYXDhv7mMbBz01SAUouCwGlgDrjJa4RN7OUDs70h31I/EYzKl+C1t9lvhprZAd/AwYWQWW1iGMO7os80xQKepyuq4KTQ/mj2OzDvvtqcmigou9xe92h8cR+BlvpxfLbBH62QIbRTPKfB2LSpyWJZGTMbvuNnNFMa9W0SInKbQGgQnfjEasO2CiUpnsUZQueEFRkppHD0+woPDUB9UoxfpiM98afeNhRl8mm8YzcNWibk20Xo7DxeE+auGz0cfa+Xn2HT/yy/iZpLT/CngAZBz5LVFU4UpZjBTTULzXUuT2FXUWhiG3uXWvAslXpuygFV8ISG7BkRfoW/Boh9hTcmB5QwBhjZdOxq5S0r0h+FBd1zojWYeEHJWa+wsDkf1S3w0u779Da0WeV2xChGSL9NVXgCIbyym;4:SPSa4nWvVAxUtbzN/AAAhu7vJE47/WR5jlev6jXdFGyC0dKCpceNtv6dSZMcbKLM8kf5z1j+PhXKucSNskjYilnjZIn+IpBBBCZZZ6fAJiUbxwKySiF+sC8Eywm4OXg9Z/c0pw9e9IYOkuJ8Cjx42/BNNfrmLotwlXXcKV5lioXLbFFOId+solbVqmMYMaKlEtlSevZ8UAb1AIZOOHsamw/IzEpmpOrX1cLPSx2kxfFZnGdZq62FPkt1Zar8aeryHpWF1SwLEw9ssupOhMyk6g== X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040470)(2401047)(8121501046)(5005006)(10201501046)(3231023)(944501158)(3002001)(93006095)(93001095)(6055026)(6041268)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123564045)(20161123558120)(6072148)(201708071742011);SRVR:HE1PR0501MB2858;BCL:0;PCL:0;RULEID:(100000803101)(100110400095);SRVR:HE1PR0501MB2858; X-Forefront-PRVS: 05514B7026 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(346002)(39380400002)(366004)(396003)(39860400002)(376002)(76104003)(24454002)(189003)(199004)(478600001)(6246003)(9686003)(39060400002)(6862004)(86362001)(57986006)(53936002)(83506002)(33656002)(69596002)(5660300001)(76176011)(386003)(52116002)(59450400001)(33896004)(16586007)(58126008)(83796002)(93886005)(106356001)(105586002)(316002)(54906003)(2950100002)(8676002)(6636002)(36756003)(122856001)(50466002)(81156014)(8936002)(305945005)(23726003)(68736007)(47776003)(7736002)(66066001)(46656002)(81166006)(2906002)(9746002)(97736004)(1076002)(4326008)(3846002)(6116002)(9786002)(229853002)(18370500001)(24400500001)(42262002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0501MB2858;H:mlx.ziepe.ca;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;HE1PR0501MB2858;23:iLAu1Lsp7B3mf0YqNYgrJ2L9Xa+px17Yb8EVmdR?= =?us-ascii?Q?EVWe7rnflSDTLOxVZBOE2HbyJJ2FrkfsDwclFHVbue+5RkXWfDmAgJB291v0?= =?us-ascii?Q?OKkri10mErBp5OJ2MKqIgUYy48XGSbeZV+KstFbLLdz7/sv5QhDvXKFzv2Ep?= =?us-ascii?Q?sz5tm7W0clHPW5BP6IwY+Jp4MGkAuIEhm1QstaPy2OyE476JNAG5nhlCWqXg?= =?us-ascii?Q?J7HbLBZfElQljPF/3QlHGiHV7jsvudkQWG+eScGHeO7JRWpCHyVWtQnLEMZI?= =?us-ascii?Q?Yjw/w7BYdbjYb/grVV9ZRsZsh8IZmtN7BhX+8g0jqAfHJfYu++syMJzIiyxA?= =?us-ascii?Q?I5z409sWwyNvu8BFxYyjV0zxY/7I/QGUngLp9zkEj8uizphOxjchuLt2XZIh?= =?us-ascii?Q?kmWG9IW3VitlGQ759/DJ882/BxAlpYCjtCjbtVwCZZ0ZuBkYizbXeiafK9CH?= =?us-ascii?Q?bZ4tN7Y1iciEOl2JSBJcWlSoVSP78rHvh+zl9r0u7PWhmKln+lE7YkuqmIKB?= =?us-ascii?Q?PnxVB8r2PXavClktjlndca6tv1vP14hLShNyReEbC81W6YrEV6ksyqc3a6U9?= =?us-ascii?Q?0eGBhaqC0H4l3jvOhVGg0db3vrqd0+q7s1XdNNkyjziddKcx1Bg58BPpMASk?= =?us-ascii?Q?eyQZzFlMejRIyu98SlvCfeNEiGkEpW3JaX175FJ8qhSvYr+Q5DlAYEt8J3Kd?= =?us-ascii?Q?BfZwg4a0XFsY2doaYe94CcIAKp9npqDHLtvFoC737EhGWNVEsWAJKhcHb1ch?= =?us-ascii?Q?YsfjWDCtqZcLac/IqCN+Q4YN6OP4QnL5aUlzYqEXdXrozTyNRJ+oBCvAjmVp?= =?us-ascii?Q?CWSRp9JECSr6hW7ZTqtvDg4klUFtbMlfJfoQlJPzjzHNCjZRDYASrEiKzPgB?= =?us-ascii?Q?3SezK34qp+tcko2uN4aHQqCG+bYhrJoY9epjre0ygrJ/Uw9ACg77VE4nvBNM?= =?us-ascii?Q?i9cZv/icqkRFnLSyaXnVofT4wr2aaMKHPb0cRjrvCoqwB+/L4oBV9w5++eOa?= =?us-ascii?Q?/d+Dtp5ew38polTm5s7v7qq5KXEVxVu9DPzwag55E8V+P8hshyNYtedYCxV/?= =?us-ascii?Q?MQSYsBIvMCBUvlwam7au3/qNvbCHNSCBUXgFvACeVD2ua7SvG08D1bIBRzMk?= =?us-ascii?Q?/cmD0U8ucvT/mymZUStCDg1+d5UdhUR8XGQCsYm9dO5nt+J7BSOiywOmygo9?= =?us-ascii?Q?G/pwmi425GIE+CER+sUXQuwI9c0VTm8TFkgvVQC6YSrivmNiWrvdxhpqdPiB?= =?us-ascii?Q?jAcgVm8lFMpqKx6DKWXQ1AETovxSVZPFcYJRDgVMUWp+453ZYtIm2gUtkCgO?= =?us-ascii?Q?idB+tmiYa/23Es5iyd5m7mGMhwX3u1FsmWX034IPg7nQWSUIdcBXt967ZRQB?= =?us-ascii?Q?wDXcGMs+nIxDSshF5511MysnmHoMbOrO0CYCqh2FhaPbxeC4Z+/OSb2sxxAZ?= =?us-ascii?Q?E3Lddswp2l2OElMLd85gw6KMzUp1Q3z3jPklOwuXYAvKAMBRs5q7R?= X-Microsoft-Exchange-Diagnostics: 1;HE1PR0501MB2858;6:Yiuek/FJ8DsrCgYOFqeSNqzg3eSqDryPaZzCk6vzdmDzYF2b57FjbAalVTCvTzanX6sHg0o8aSh1B+1AULQXeq3rEVg6cR2vlUh3fAVDIs6uTTMaMzh+Dtf4Ey7LJDgX3HgxXTyHZV1SFmMgefaQCvCPcu1D2pMtjGABOyiCAi2xCkY72tl7X6Ws61SOQfgilWVaqFQGG+yq5lknK3f3Mpisnt8o+2wNXTfCsxAEatGTPodAcBqYM5+1TpBzzSXlzeJi7kVj0WVNYoE7WHlQpZqoE21C3WfnonMjZ9/+wy7eEuJghP/7qaaOz6VxT2liX/p6GIb8LVaURqSO2mUUigdxQf82uixjSYEJ16S4FZ4=;5:ABbPMnFWaFvsDOQRpjRKOKb10NcdWwJ5v6IJdTEFxDKTIUAXQLEbRYiMs/bAmnmfgP638rYnOEZhp7rY/QHqLfu5IBWkqTUKaN7knjtoHSHwJ9eMZR2aAeq99o4X20i24MYZ1nHophJAzWQdyJkt/l9BXGjg/Xc/rl4Ojl3ZQ/A=;24:+/Q9GtD1BDFI3sTkeuEpm76scir7hvjw5h25B2FozPVsINDlC1aC3Xf3Z163l7uwhdXFrZP6X78uD6JWoyfnR3Nx1YGaAE8Anv/Xo0rqF78=;7:f4UHO14or3lt1i1YLMVptgC3q0/tbbCJjadk7NkYJQsIrFRh2b8YUV2JcST/T41cRyQnfVdrfOrc0CrIxO4wMWEFvgP1nfzEXjKEagjtTo9ISFMZk7Y08SpGVx1205FNtHQds+fgZDp+dv84Xsd/i5/i7wcj0CK0f03H4TfIZAfStnUTy8N0VdEBOG4kpFZY6DjpsDr1xiimkGdQhTzgb81E9DCCMUqAyuJQxXbP1VMzwdHuGfM0iEO8hfltZq9E SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2018 19:16:09.9229 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: e26c8bc2-c4ea-415b-54cd-08d55aba1ab2 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0501MB2858 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Fri, Jan 12, 2018 at 01:01:56PM -0800, Saeed Mahameed wrote: > Simply putting a memory barrier on the top or the bottom of a functions, > means nothing unless you are looking at the whole picture, of all the > callers of that function to understand why is it there. When I review code I want to see the memory barrier placed *directly* before the write which allows the DMA. So yes, this is my preference: > update_doorbell() { > dma_wmb(); > ring->db = prod; > } Conceptually what is happening here is very similar to what smp_store_release() does for SMP cases. In most cases wmb should always be strongly connected with a following write. smp_store_release() is called 'release' because the write it incorporates allows the other CPU to 'see' what is being protected. Similarly here, the write to the db allows the device to 'see' the new ring data. And this is bad idea: > fill buffers(); > dma_wmb(); > update_doorbell(); > I simply like the 2nd one since with one look you can understand > what this dma_wmb is protecting. What do you think the wmb is protecting in the above? It isn't the fill. Jason