From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934797AbcJFUDt (ORCPT ); Thu, 6 Oct 2016 16:03:49 -0400 Received: from mail-bn3nam01on0104.outbound.protection.outlook.com ([104.47.33.104]:14912 "EHLO NAM01-BN3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932462AbcJFUDp (ORCPT ); Thu, 6 Oct 2016 16:03:45 -0400 X-Greylist: delayed 1056 seconds by postgrey-1.27 at vger.kernel.org; Thu, 06 Oct 2016 16:03:43 EDT Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=jason.low2@hpe.com; MIME-Version: 1.0 In-Reply-To: <20161006054747.GB29373@linux-80c1.suse> References: <1471554672-38662-1-git-send-email-Waiman.Long@hpe.com> <1471554672-38662-2-git-send-email-Waiman.Long@hpe.com> <20161004190601.GD24086@linux-80c1.suse> <57F4EFCA.6050503@hpe.com> <57F5181F.60202@hpe.com> <20161006054747.GB29373@linux-80c1.suse> From: Jason Low Date: Thu, 6 Oct 2016 12:31:13 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH-tip v4 01/10] locking/osq: Make lock/unlock proper acquire/release barrier To: Davidlohr Bueso CC: Waiman Long , Peter Zijlstra , Ingo Molnar , Linux Kernel Mailing List , , , , , , , Dave Chinner , "Jonathan Corbet" , Scott J Norton , Douglas Hatch , Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [209.85.223.180] X-ClientProxiedBy: BN6PR06CA0018.namprd06.prod.outlook.com (10.175.123.156) To DF4PR84MB0172.NAMPRD84.PROD.OUTLOOK.COM (10.162.193.139) X-MS-Office365-Filtering-Correlation-Id: c799f1d3-51e6-430f-aacd-08d3ee1f5add X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;2:QtOEQBrTkMsKFtvbcvfXcRhOs5VZmAcqlZICgj2kpIdFQ82FAAOjp2cJr+CSPru98jBcHjLAIzHx4Ig5AoYLXFJy76710m+47FiSMUFEnurAegGAPNiJMNEANx0FhxVnUdFF/KwMBQei4ck35a6nqnX+Ak4BNkOXjM9WidVnIe8CZYEdbLHB+SLKWTwP9ASeUtQ6gf4Rdiq/RN2gUZTxVg==;3:bd42S/otMbeAI91DS6+jIo0ZhtDMC+rrQa1Of3g8VV5lnt2g5D7X7gD7+6Q7t7WdyHc8Xny+Ta3EaD/8UvewxWHvMahbz5UIqCvB/OluYhcofWV1bUdXFx8jOtWNsmLCpW6NR0zNs6CA3lQhfTI/zA== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0172; X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;25:A1uRLsWDU+ePygRzgPl2kk0lwSV8FKzHwjSDL6zf3MSNhxuZCjVAR5xzTYJACdoJNeLunZeSmSm0iatFsF/xZTjYoZA+YscBfFJCctGUQldD1325oCg/A3I9fZ2vOpL+AS2Fie6gYFKVGyi0jqGL5LRqCpyPvvk7tHgHJGaaTiWewDyWdHWZJhG4FxAGNrG6fRNy3Pf+oDBSErvnt4hNC/aK3cRs9F0Q1XhMeq8eDFFkrkWaqYSfnx1i1D6SR+XOC6eJegsQLttSAT61EvtV3LXPDXRbXzh6ydKtAPGuNx2I20UTFryEE/1hJ6lwM8gjPYWNXvUZ4uVx6LBDipRtt5AF7aqmqWSiR1pWzX2LNqpY5KsasN3ot7k2khufe399VSsc7gOyZbU6OYliu6e2aYAeUF6vQkELt0jOJVnqbHD8eWP6jfvk7e9aEth/YWhcUceWzGPtLAVMgYHcPZynkzOicB0RTpYYsYfb7d+ND/HyjSrc22Vn5arzL2cVpZMUD+6i52rbYW0oveIDeQrjhSaIzeA9lTGvIH5nDWN1pvzsjeqEFoSxOUXWG+Og2+1pDhkuAs/8okLC6Vxb0otPNtdjN5MNn4DckIImscQv/3uwuRq5rxvLwJtdl4ChjdB/POELyXhanOeviX7Vro3dsOxbzmgmx5qrM9Ry9h1Z4KeFyJNwvmiHoqk4l35N8QrpGcdidbS7tHwtbxZdqmICqSwuLlkKYiXUVsgOleBBp3M= X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;31:8I6hV+7fo8ipheUmpS2SJXHwOUllj8bHWmmL6LKUzJBo/qNkySSz4dqVhqYJxX8V3ufgkpgu2PEBXTUTh87DaSBQpB1zTe3/nan3Oa+fdME/2Dk8KN11NLqoDohvWuUqGkCzXeAIaCu4/Jv0Lq698wWGK3e7UR+xiODCVawJ4Pjmcx4GoqCdVwJ8+dLx6xtfQN6T+QXqY39sEPcjTg0lCvj07ehyYWP81VwjossjGW5cnop7UA57I7Lhjig/7lgVH77nCA5KzcOll5HWFvhCKA==;20:ek1PPgAplE7g8HvG8XEuPX8fYiN597tDCGRvUJrcJTWMh8Ab2YgeOVu9XEuSZ0Tblp1/EJpbjkMthl6ng5RaOWiUAXwSh6Uc5fRsoJagGlqEVMOdStoKiSmVcoNJW5Lb2gOibrEfLP8yXJbCW8Q8hvK3tmPOet1CbBv9Gm2tcS4zMpCHg0HNneTzgv30qixWSD30lUW4uRkQVtN5YNCM0ZE9lzUXF0j0zY9trgFbA1UwI/5OBbhbZuozsd144r9+z10pGkTFQMJWNWKikw6sFB7DLpE/X0A2qs6P2c2erbn8Yg+qufbQkrxyUum+bNcZHgFWQKKS1sMlazpqT1x2M90Uq4qL1gr9llj7zOpFk/Oz2p+PqJ4fMnvimXVKVlWoA84QwJ2PmPkrjKLXl7sHUboWui2K6M21x26H4obfyXnRDu2aAcsYmsS9lnusoBiBrfghUZq0HWvb4sp7Q40eYKHdCX57JGsHcGB6YtB2pOeCExExRKZ7XsEaIx9x5ew3 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:DF4PR84MB0172;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0172; X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;4:/k1k2krpEI3UaK6dUju7SAPEwTluxiXuADo0svSgM1SJjpKZ5ocvfSk9B+nHnKb65xhnA246Cf7ABa37UU7bNhqZ8EfwbXmQg4kE4I/NGdfVqebPi4VsmzKmgSQU0OvxGATpmOyK8GKVOAgl1ElgLUe3Rf0KHCpAEGCLoYHNUdVPb3RDhGAUrUFl/Fz7WYhR67Z0HAki0ro3HpJNPL2hUdyPi0tWE+PfgIF7fqNGMyEfvNQ/4lPgh5zygexeiqGW//YlRLmHjdje71Zsd+IRzYaY0Mu4h4X4b5R012Z6LEPj7SfZXnRx1w7G0+NLJO+pPX93Y57242aGPV8aYWmMuiHfxAmOAp7Mqsa4BV5hPUiGO+BsZYd0ES/RwpFSzQoXY6jyK7v/rYHqydTPCeLPNHjvJNoDqaDjfLP6uVZiQ9k= X-Forefront-PRVS: 00872B689F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(7916002)(51444003)(189002)(24454002)(76104003)(199003)(377454003)(9896002)(450100001)(6116002)(4326007)(3846002)(5820100001)(68736007)(23676002)(2906002)(86362001)(5660300001)(586003)(98316002)(2950100002)(61726006)(189998001)(8676002)(106356001)(110136003)(63696999)(7736002)(59536001)(19580405001)(50986999)(97736004)(76176999)(54356999)(69596002)(6862003)(92566002)(305945005)(101416001)(7846002)(42186005)(81166006)(9686002)(93886004)(93516999)(105586002)(81156014)(19580395003)(50466002)(47776003)(66066001)(55446002)(61266001)(55456009);DIR:OUT;SFP:1102;SCL:1;SRVR:DF4PR84MB0172;H:mail-io0-f180.google.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtERjRQUjg0TUIwMTcyOzIzOlppaWpLd0xGbUQ3c2dJUkgyRENHL0dLaEts?= =?utf-8?B?ZzFoUVdManJVMCtSMDVPMHA4RlpIbng2TzdHemU4M2tzRWdBajRMdVVuMStm?= =?utf-8?B?dDBtSHF6d2dqaXRpcGZBQUIrdGQvZ0JxaittbUhSMTZrYUJRSi9KZFVsU2J4?= =?utf-8?B?c2tvL0todHhyWUJ1UmRMdWVMQ3NjYmVhdTR5eHNRTVRWRmtzTnBRZlFFQ21T?= =?utf-8?B?MTBCdlZ4Z012T2IxV2tvVFBkcStOQWh6dmpuRTl1dFhWNno3alhDNUpEQ3BE?= =?utf-8?B?S1NYVXNjRDNuT0FZRjQ0YXhXRXVBZktpQ2RpNHNsbEd1aTlGSzlsczE0VWZw?= =?utf-8?B?NXlvc1dFV3FaQmc2dk1tSUxmb1o3cjZSaTV4ZXBPai9GUmVBeGZhbDVRNDhG?= =?utf-8?B?V2VOMjZPMlFoV1NjcGRUVHp1RzAwaFg5cWpFU0t3ZFpHU3J6T1B1amRhWEpS?= =?utf-8?B?WWF0SzlaVFcvYmR4T3lLakxGVG9LZzNpVktDTHVZd2QvRGpzMStEZWZ3aVBE?= =?utf-8?B?ZkdrYW5IdVFwWTJ0QldMR0V4Z2lrd2FnRVlkMk5UbUQ0OVBCUkkvSHBZUTFo?= =?utf-8?B?RitJc3FBNUNnRmQxZC9rMHN2Nlp5ZGYwRVBIaHZBNFV0VEVmTnBPRUFUVG5r?= =?utf-8?B?c0psSUhlY1NsKzZYcUR2SGN6UTAzQ3lKOHBEY3FNS0tUVzBKZVJqMGo5RE56?= =?utf-8?B?bGt6OXVzMXFZSDJGSituOUR5cGJ5Z0pFUXZKQ0t3UGFkblJhT3J2QnlWcjJw?= =?utf-8?B?Y3N1Y3JNUC9zdFFLOHk1a1NOdFJpVW9kSllnbWowUzNkOXdpc1dzTDUrWHZJ?= =?utf-8?B?NjI5dlVQckRsTWROYVk4Z1B1QzVnMW9SSExaZWFIemtGWjU3Qzc1dW55MjRU?= =?utf-8?B?UFFkQ1BoVHhIK2dDYlhrWFJYdURZazc0MUFMMHpmbWl4aG10WWZGbDlBQWtY?= =?utf-8?B?K0JtY3hudzQwZ1M1YnZramJia2VLdzQzZzFRbUpUKzEyYnhFSHorUk11WEZ0?= =?utf-8?B?cTNmWThXaDVBeGdQb1lUR0QrMGdKUEpZWktleFNyK3ZVNGhMc25uR2lxV3dH?= =?utf-8?B?K040UnhiZ1YxS1J1VXpiZjZKalByMlVUeEViblNVYjJPK0c5YjliZkw1TFE4?= =?utf-8?B?N2xKamxBeUFoN0t4ODV2Q0JuWnFFa0pmZUtKTnJuWEFzaTNGYktvRGZjb0pa?= =?utf-8?B?WnJHN1Z1RHlWTFp4ZFo1dlhxMTZGdVRjL2pSdWJJL09Wa0MxMktxYlJWQXZW?= =?utf-8?B?R21lQXNsbEk5cWxBZmdqR3hjbkxzKzVBdGw0cWpqOEZ6WmExazhlV2F3dU90?= =?utf-8?B?cjY2d1d3S2x3cUtIYnR2VzJNQUNIM0l3Tnc1d05LdFBSQmhqNU1qalBNTStR?= =?utf-8?B?bUJTNW90d014bnhzckZjelhrVW9xM2NidFE5bkUzY25jNlFjTDlVcUhOTUFI?= =?utf-8?B?RjNiMGQ4T1RMeWlLa0JtNzhiYXp5LzE1NlFtMS95Q3cxUXk1Y3dtNlB1akox?= =?utf-8?B?WmRPaGRFQUpaNExRQjRwWk9hNTZuK2VwMEN3aW53Z2hYdWlnN3lYSHVqa2VE?= =?utf-8?B?MkVRV1ZuVkxrM2I3K001Vnc3elpHa1pRUGVpV0w1Z1E2ejd2N3VuSldnWUVD?= =?utf-8?B?cEI3R0ZSWGlXQnhGU3VFRjk4RmJZMmtHMEwySDczSmZQY1NBTy93WW52VmdE?= =?utf-8?B?aGxGeGloaTBzRFo3L0xEZHROZlVQUmtSYzhFQ2Qxd1V2SzVHTjl5c3YwSGh3?= =?utf-8?B?WGFYS3M5V3JSZklIRHlEeFJUTCt6V1pBS2ZLUnRnTEYzdzErL2Q1c2JnTHNq?= =?utf-8?B?bGRKaWdFcmI5eEdVakdGR3BoTHVvbXJXVEFiOWpwT1VPY0NYSGpGaS9KNnNp?= =?utf-8?Q?+9OMvF3NefE=3D?= X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;6:Q+8yGE8y3lZ8+2cSmYdl5UPdAn1vqzzYyxLKOxJh9xuF7zhZyAzfCTfTKxwklSkknaUBgV2wTm71d043TCTkgznGkcCJZwtZ5jVLRFULzDGGILgKPjar0jyBTjHm6kS69yyWmjWtHmtJjwv1J1eG5EyOhvx5NOsvnEVCuzIHvbB0hUXI+HCIOK2pT5W39T+1gvcZUZY7BmtI4B8F7kOGc7uF6p28z7/TabSARmn8iZtboGDHV3kE+ZdDq2EFQKk8EYQdicMG7BZX0ZueLtNhB69NBEJ8Ih4R2fl55uU29k8sQX8GP2ZbnXPvH9NZIIwfiHin1KpjH4nwCebzN2X8Rayj4CKaKn6+RnOQKLeAdiY=;5:6vCEzoVsutCL1PyfXxP7E+3x40KazSstBcPyT4edlW2Hn3feryrFU3QGBycN8e5VQvgkHdm/g3zJGuSPHyX5NsN70GkCMwTWfkGHdAsLvj9PQ+MxYxxit3JHddUZk5YXgItMcxLLf0aqJ7xWD4KAFNeooF/Siu/TASG6Ei4DPuU=;24:EWW9LfQ/ZnqEuw10wDIpvgPosmKVF5GO2VrGMm5jpKLoWZOc6Y+0PwlPGjSsGCweqKGdo8c+417NkJeTn0NlZF/5+ll3GfqODMvdphQtu/k= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0172;7:HG88/J4f1337FrEqFKXNpfj3veDDk8nvT2lz2eblXcPiDWSsyxj/siZ1lQaIxVgpJ1KXqPp/ONFCQ3aWFpkkPsqG8IDwcBq8sqBad5DwAnapRUrmolJcECA5V1ChTv2vGVzn+tdawg+rsbkIj/ZCvqv7LzJ7cfc+3lk+D+sL+7Cd77RX+EqFdiGlLmlYNzEmfruLHI0tyVDARODPCG/cqAQT0qYc1EY85ZgV330bB/lTg3RU29ssFBz93i18cbZb1cBFfzgNDz+F7bjEacV3um+0lRhOJXbXEpB+DU6e8QQLXyzqqNU6z7iLl90OB/DDmtKn0OPi/UM36vAvxEM5pA== X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2016 19:31:22.5075 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0172 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 5, 2016 at 10:47 PM, Davidlohr Bueso wrote: > On Wed, 05 Oct 2016, Waiman Long wrote: > >> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c >> index 05a3785..1e6823a 100644 >> --- a/kernel/locking/osq_lock.c >> +++ b/kernel/locking/osq_lock.c >> @@ -12,6 +12,23 @@ >> */ >> static DEFINE_PER_CPU_SHARED_ALIGNED(struct optimistic_spin_node, >> osq_node); >> >> +enum mbtype { >> + acquire, >> + release, >> + relaxed, >> +}; > > > No, please. > >> + >> +static __always_inline int >> +_atomic_cmpxchg_(const enum mbtype barrier, atomic_t *v, int old, int >> new) >> +{ >> + if (barrier == acquire) >> + return atomic_cmpxchg_acquire(v, old, new); >> + else if (barrier == release) >> + return atomic_cmpxchg_release(v, old, new); >> + else >> + return atomic_cmpxchg_relaxed(v, old, new); >> +} > > > Things like the above are icky. How about something like below? I'm not > crazy about it, but there are other similar macros, ie lockref. We still > provide the osq_lock/unlock to imply acquire/release and the new _relaxed > flavor, as I agree that should be the correct naming > > While I have not touched osq_wait_next(), the following are impacted: > > - node->locked is now completely without ordering for _relaxed() (currently > its under smp_load_acquire, which does not match and the race is harmless > to begin with as we just iterate again. For the acquire flavor, it is always > formed with ctr dep + smp_rmb(). > > - If osq_lock() fails we never guarantee any ordering. > > What do you think? I think that this method looks cleaner than the version with the conditionals and enum. My first preference though would be to document that the current code doesn't provide the acquire semantics. The thinking is that if OSQ is just used internally as a queuing mechanism and isn't used as a traditional lock guarding critical sections, then it may just be misleading and just adds more complexity. If we do want a separate acquire and relaxed variants, then I think David's version using macros is a bit more in line with other locking code. Jason From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Low Subject: Re: [RFC PATCH-tip v4 01/10] locking/osq: Make lock/unlock proper acquire/release barrier Date: Thu, 6 Oct 2016 12:31:13 -0700 Message-ID: References: <1471554672-38662-1-git-send-email-Waiman.Long@hpe.com> <1471554672-38662-2-git-send-email-Waiman.Long@hpe.com> <20161004190601.GD24086@linux-80c1.suse> <57F4EFCA.6050503@hpe.com> <57F5181F.60202@hpe.com> <20161006054747.GB29373@linux-80c1.suse> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20161006054747.GB29373@linux-80c1.suse> Sender: linux-arch-owner@vger.kernel.org List-Archive: List-Post: To: Davidlohr Bueso Cc: Waiman Long , Peter Zijlstra , Ingo Molnar , Linux Kernel Mailing List , x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, Dave Chinner , Jonathan Corbet , Scott J Norton , Douglas Hatch , jason.low2@hpe.com List-ID: On Wed, Oct 5, 2016 at 10:47 PM, Davidlohr Bueso wrote: > On Wed, 05 Oct 2016, Waiman Long wrote: > >> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c >> index 05a3785..1e6823a 100644 >> --- a/kernel/locking/osq_lock.c >> +++ b/kernel/locking/osq_lock.c >> @@ -12,6 +12,23 @@ >> */ >> static DEFINE_PER_CPU_SHARED_ALIGNED(struct optimistic_spin_node, >> osq_node); >> >> +enum mbtype { >> + acquire, >> + release, >> + relaxed, >> +}; > > > No, please. > >> + >> +static __always_inline int >> +_atomic_cmpxchg_(const enum mbtype barrier, atomic_t *v, int old, int >> new) >> +{ >> + if (barrier == acquire) >> + return atomic_cmpxchg_acquire(v, old, new); >> + else if (barrier == release) >> + return atomic_cmpxchg_release(v, old, new); >> + else >> + return atomic_cmpxchg_relaxed(v, old, new); >> +} > > > Things like the above are icky. How about something like below? I'm not > crazy about it, but there are other similar macros, ie lockref. We still > provide the osq_lock/unlock to imply acquire/release and the new _relaxed > flavor, as I agree that should be the correct naming > > While I have not touched osq_wait_next(), the following are impacted: > > - node->locked is now completely without ordering for _relaxed() (currently > its under smp_load_acquire, which does not match and the race is harmless > to begin with as we just iterate again. For the acquire flavor, it is always > formed with ctr dep + smp_rmb(). > > - If osq_lock() fails we never guarantee any ordering. > > What do you think? I think that this method looks cleaner than the version with the conditionals and enum. My first preference though would be to document that the current code doesn't provide the acquire semantics. The thinking is that if OSQ is just used internally as a queuing mechanism and isn't used as a traditional lock guarding critical sections, then it may just be misleading and just adds more complexity. If we do want a separate acquire and relaxed variants, then I think David's version using macros is a bit more in line with other locking code. Jason From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Low Date: Thu, 06 Oct 2016 19:31:13 +0000 Subject: Re: [RFC PATCH-tip v4 01/10] locking/osq: Make lock/unlock proper acquire/release barrier Message-Id: List-Id: References: <1471554672-38662-1-git-send-email-Waiman.Long@hpe.com> <1471554672-38662-2-git-send-email-Waiman.Long@hpe.com> <20161004190601.GD24086@linux-80c1.suse> <57F4EFCA.6050503@hpe.com> <57F5181F.60202@hpe.com> <20161006054747.GB29373@linux-80c1.suse> In-Reply-To: <20161006054747.GB29373@linux-80c1.suse> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Davidlohr Bueso Cc: Waiman Long , Peter Zijlstra , Ingo Molnar , Linux Kernel Mailing List , x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, Dave Chinner , Jonathan Corbet , Scott J Norton , Douglas Hatch , jason.low2@hpe.com On Wed, Oct 5, 2016 at 10:47 PM, Davidlohr Bueso wrote: > On Wed, 05 Oct 2016, Waiman Long wrote: > >> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c >> index 05a3785..1e6823a 100644 >> --- a/kernel/locking/osq_lock.c >> +++ b/kernel/locking/osq_lock.c >> @@ -12,6 +12,23 @@ >> */ >> static DEFINE_PER_CPU_SHARED_ALIGNED(struct optimistic_spin_node, >> osq_node); >> >> +enum mbtype { >> + acquire, >> + release, >> + relaxed, >> +}; > > > No, please. > >> + >> +static __always_inline int >> +_atomic_cmpxchg_(const enum mbtype barrier, atomic_t *v, int old, int >> new) >> +{ >> + if (barrier = acquire) >> + return atomic_cmpxchg_acquire(v, old, new); >> + else if (barrier = release) >> + return atomic_cmpxchg_release(v, old, new); >> + else >> + return atomic_cmpxchg_relaxed(v, old, new); >> +} > > > Things like the above are icky. How about something like below? I'm not > crazy about it, but there are other similar macros, ie lockref. We still > provide the osq_lock/unlock to imply acquire/release and the new _relaxed > flavor, as I agree that should be the correct naming > > While I have not touched osq_wait_next(), the following are impacted: > > - node->locked is now completely without ordering for _relaxed() (currently > its under smp_load_acquire, which does not match and the race is harmless > to begin with as we just iterate again. For the acquire flavor, it is always > formed with ctr dep + smp_rmb(). > > - If osq_lock() fails we never guarantee any ordering. > > What do you think? I think that this method looks cleaner than the version with the conditionals and enum. My first preference though would be to document that the current code doesn't provide the acquire semantics. The thinking is that if OSQ is just used internally as a queuing mechanism and isn't used as a traditional lock guarding critical sections, then it may just be misleading and just adds more complexity. If we do want a separate acquire and relaxed variants, then I think David's version using macros is a bit more in line with other locking code. Jason