Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754275Ab3C0UCW (ORCPT ); Wed, 27 Mar 2013 16:02:22 -0400 Received: from shelob.surriel.com ([74.92.59.67]:37585 "EHLO shelob.surriel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753475Ab3C0UCV (ORCPT ); Wed, 27 Mar 2013 16:02:21 -0400 Message-ID: <51534FF3.2010007@surriel.com> Date: Wed, 27 Mar 2013 16:00:51 -0400 From: Rik van Riel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Peter Zijlstra CC: Michel Lespinasse , Sasha Levin , torvalds@linux-foundation.org, davidlohr.bueso@hp.com, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, hhuang@redhat.com, jason.low2@hp.com, lwoodman@redhat.com, chegu_vinod@hp.com, Dave Jones , benisty.e@gmail.com, Ingo Molnar Subject: Re: [PATCH -mm -next] ipc,sem: fix lockdep false positive References: <1363809337-29718-1-git-send-email-riel@surriel.com> <5150B1C2.8090607@oracle.com> <20130325163844.042a45ba@annuminas.surriel.com> <1364303965.5053.29.camel@laptop> <1364308023.5053.40.camel@laptop> <5151BC78.3030306@surriel.com> <1364373750.5053.54.camel@laptop> In-Reply-To: <1364373750.5053.54.camel@laptop> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2150 Lines: 61 On 03/27/2013 04:42 AM, Peter Zijlstra wrote: > On Tue, 2013-03-26 at 11:19 -0400, Rik van Riel wrote: >>> Maybe something like: >>> >>> void sma_lock(struct sem_array *sma) /* global */ >>> { >>> int i; >>> >>> sma->global_locked = 1; >>> smp_wmb(); /* can we merge with the LOCK ? */ >>> spin_lock(&sma->global_lock); >>> >>> /* wait for all local locks to go away */ >>> for (i = 0; i < sma->sem_nsems; i++) >>> spin_unlock_wait(&sem->sem_base[i]->lock); >>> } >>> >>> void sma_lock_one(struct sem_array *sma, int nr) /* local */ >>> { >>> smp_rmb(); /* pairs with wmb in sma_lock() */ >>> if (unlikely(sma->global_locked)) { /* wait for global lock */ >>> while (sma->global_locked) >>> spin_unlock_wait(&sma->global_lock); >>> } >>> spin_lock(&sma->sem_base[nr]->lock); >>> } > > I since realized there's an ordering problem with ->global_locked, we > need to use spin_is_locked() or somesuch. > > Two competing sma_lock() operations will screw over the separate > variable. There may be another problem with your idea. If there are two single locks coming in for the same semaphore, the first one holds the lock, while the second one is spinning on the lock. The global lock is spinning on spin_unlock_wait, which may end up finishing after the first single lock holder unlocks, right before the second single lock holder grabs the lock. At that point, you have both a process that thinks it holds the global lock, and a process that thinks it holds a single lock. To prevent against this, the single lock probably needs to test whether the global lock is locked, after it acquires the local lock. If the global lock is locked, it needs to unlock its single lock, and then do a spin_unlock_wait on the global lock, before trying again from the start. Would that work? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/