From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752206AbcEUNt1 (ORCPT ); Sat, 21 May 2016 09:49:27 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35683 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751373AbcEUNtZ (ORCPT ); Sat, 21 May 2016 09:49:25 -0400 Subject: Re: sem_lock() vs qspinlocks To: Peter Zijlstra , Davidlohr Bueso References: <20160520053926.GC31084@linux-uzut.site> <20160520115819.GF3193@twins.programming.kicks-ass.net> <20160520140533.GA20726@insomnia> <20160520152149.GH3193@twins.programming.kicks-ass.net> <20160520160436.GQ3205@twins.programming.kicks-ass.net> <20160520210618.GK3193@twins.programming.kicks-ass.net> <20160521004839.GA28231@linux-uzut.site> <20160521073739.GB15728@worktop.ger.corp.intel.com> Cc: Linus Torvalds , Boqun Feng , Waiman Long , Ingo Molnar , ggherdovich@suse.com, Mel Gorman , Linux Kernel Mailing List , Paul McKenney , Will Deacon , 1vier1@web.de From: Manfred Spraul Message-ID: Date: Sat, 21 May 2016 15:49:20 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <20160521073739.GB15728@worktop.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2016 09:37 AM, Peter Zijlstra wrote: > On Fri, May 20, 2016 at 05:48:39PM -0700, Davidlohr Bueso wrote: >> As opposed to spin_is_locked(), spin_unlock_wait() is perhaps more tempting >> to use for locking correctness. For example, taking a look at nf_conntrack_all_lock(), >> it too likes to get smart with spin_unlock_wait() -- also for finer graining purposes. >> While not identical to sems, it goes like: >> >> nf_conntrack_all_lock(): nf_conntrack_lock(): >> spin_lock(B); spin_lock(A); >> >> if (bar) { // false >> bar = 1; ... >> } >> [loop ctrl-barrier] >> spin_unlock_wait(A); >> foo(); foo(); >> >> If the spin_unlock_wait() doesn't yet see the store that makes A visibly locked, >> we could end up with both threads in foo(), no?. (Although I'm unsure about that >> ctrl barrier and archs could fall into it. The point was to see in-tree examples >> of creative thinking with locking). > I'm tempted to put that trailing smp_rmb() in spin_unlock_wait() too; > because I suspect the netfilter code is broken without it. > > And it seems intuitive to assume that if we return from unlock_wait() we > can indeed observe the critical section we waited on. Then !spin_is_locked() and spin_unlock_wait() would be different with regards to memory barriers. Would that really help? My old plan was to document the rules, and define a generic smp_acquire__after_spin_is_unlocked. https://lkml.org/lkml/2015/3/1/153 Noone supported it, so it ended up as ipc_smp_acquire__after_spin_is_unlocked(). Should we move it to linux/spinlock.h? Who needs it? - ipc/sem.c (but please start from the version from linux-next as reference, it is far less convoluted compared to the current code) https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/ipc/sem.c - nf_conntrack - task_rq_lock() perhaps needs smp_acquire__after_ctrl_dep (I didn't figure out yet what happened to the proposed patch) https://lkml.org/lkml/2015/2/17/129 -- Manfred