From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait Date: Mon, 21 Nov 2016 17:52:27 +0100 Message-ID: <4374986.OFD8bCgUa1@wuerfel> References: <1479708496-9828-1-git-send-email-binoy.jayan@linaro.org> <1479708496-9828-3-git-send-email-binoy.jayan@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Linus Torvalds Cc: Binoy Jayan , linux-rdma@vger.kernel.org, Mustafa Ismail , Lijun Ou , Nicholas Bellinger , Leon Romanovsky , target-devel@vger.kernel.org, Tatyana E Nikolova , Doug Ledford , Jenny Derzhavetz , Sagi Grimberg , Sean Hefty , Faisal Latif , Hal Rosenstock , linux-kernel@vger.kernel.org, "Wei Hu(Xavier)" , Mark Brown , Mark Bloch , Steve Wise , Ira Weiny List-Id: linux-rdma@vger.kernel.org On Monday, November 21, 2016 7:57:53 AM CET Linus Torvalds wrote: > Don't do this. > > Never ever do your own locking primitives. You will get the memory ordering > wrong. And even if you get it right, why do it? > > If you want to get rid of semaphores, and replace them with a mutex, that's > OK. But don't replace them with something more complex like an open coded > waiting model. I think a mutex would't work here, since fops->open() and fops->close() are not called from the same context and lockdep will complain about that. Version of the series had replaced the semaphore with a completion here, which worked correctly, but one reviewer suggested using the wait_event() instead since it's confusing to have a completion starting out in 'completed' state. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754446AbcKUQx1 (ORCPT ); Mon, 21 Nov 2016 11:53:27 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:55085 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754118AbcKUQxL (ORCPT ); Mon, 21 Nov 2016 11:53:11 -0500 From: Arnd Bergmann To: Linus Torvalds Cc: Binoy Jayan , linux-rdma@vger.kernel.org, Mustafa Ismail , Lijun Ou , Nicholas Bellinger , Leon Romanovsky , target-devel@vger.kernel.org, Tatyana E Nikolova , Doug Ledford , Jenny Derzhavetz , Sagi Grimberg , Sean Hefty , Faisal Latif , Hal Rosenstock , linux-kernel@vger.kernel.org, "Wei Hu(Xavier)" , Mark Brown , Mark Bloch , Steve Wise , Ira Weiny , Bart Van Assche , Matan Barak , Sagi Grimberg Subject: Re: [PATCH v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait Date: Mon, 21 Nov 2016 17:52:27 +0100 Message-ID: <4374986.OFD8bCgUa1@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <1479708496-9828-1-git-send-email-binoy.jayan@linaro.org> <1479708496-9828-3-git-send-email-binoy.jayan@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:STmQeJn1KewBTKV8jYIVLEh+IQ4yN4glQaxRhk+iciWY9uU5og0 ENOXZ10NWb2q121uNwO51oNtomAKaCK25ScwgPqSgFEjajXzHr+iFVew6BDbA2QUz96ROIU B0T/Jyvsk7NEniEFwzUNdbgKR6LERo7wYlHYvzN6ajjdQOpEX/V9QsXlV7yV63o2OvnsqIf 1HWzZxgLj6A0n0x9RLx2g== X-UI-Out-Filterresults: notjunk:1;V01:K0:1en81BRLXfI=:YND4h4mD/nEyui5ti3aY/s eWfcuUxC026CLF7p0/SArK2fo6Ow5x/mlQ5BcE+aYWcVyd1JwZse/bV0r+TF3+VL2HsvVR5W+ pbgW9xkLfi7V2NR5NA/fm5RiqkXPxxMaLCruJ0YzmCer4eEyU6Gri7zESmZ60RojufdQ3dVHN zUch+n6WND2K+ec2t1TxhbEkf6qwWI7wLwSi0mmUev5W3pRWenZS14zrDR41Ej39uj+/UpAx3 XJE4U+CvjjCfyhQl3hePRxSe8MAp8cGXK7q8n578w1jooVJ/j5v/vujS/qyH4Ni6xbeXQet/7 sMCrkJyOTuc8786MnD1hbYzc/NrJusFLoSJqL/JYRZOKRUTEJynTQ0Xs2pqg2cIkqIO+PRV9M mFMuxGBOre76P7Mz0uEy1LG9C5ykgvhG43lQ36OT5pkHy2qH3GMTiiVSiIET9A4EzoZbQlRxh jZR14PrBOpQDFl7iNgVvi0PbPEbp1eRZ67HGLXQs9G8y5WWvY4+qMdZIKcy5oLBKqgnpgneOO KCe5ZBaKRrxQ+cnb1p2gqb6KGMQJaAtOtUPGUkhIVluVyUdFA8rMD8vKeIUttNWWciBdmqb5V W4ZziJacQQ5hQil3BT55dimBYTKjiY/3tFLnziGEHgmpqvDa23THjTg+mAv5RqfZZPJc4NvxI jB0MB0+QKTCr+tmZLqkwpigYkF+HGq7xGAQac7ShzwGUMZ+ouJtafb21El6EnqgtnEZtRS8La +DqJ2g0AOu3OhQ/X Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday, November 21, 2016 7:57:53 AM CET Linus Torvalds wrote: > Don't do this. > > Never ever do your own locking primitives. You will get the memory ordering > wrong. And even if you get it right, why do it? > > If you want to get rid of semaphores, and replace them with a mutex, that's > OK. But don't replace them with something more complex like an open coded > waiting model. I think a mutex would't work here, since fops->open() and fops->close() are not called from the same context and lockdep will complain about that. Version of the series had replaced the semaphore with a completion here, which worked correctly, but one reviewer suggested using the wait_event() instead since it's confusing to have a completion starting out in 'completed' state. Arnd