From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756164AbbFPLsg (ORCPT ); Tue, 16 Jun 2015 07:48:36 -0400 Received: from casper.infradead.org ([85.118.1.10]:48971 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750990AbbFPLs3 (ORCPT ); Tue, 16 Jun 2015 07:48:29 -0400 Date: Tue, 16 Jun 2015 13:48:23 +0200 From: Peter Zijlstra To: Oleg Nesterov Cc: Linus Torvalds , Paul McKenney , Tejun Heo , Ingo Molnar , Linux Kernel Mailing List , der.herr@hofr.at, Davidlohr Bueso Subject: Re: [RFC][PATCH 5/5] percpu-rwsem: Optimize readers and reduce global impact Message-ID: <20150616114823.GN3644@twins.programming.kicks-ass.net> References: <20150526114356.609107918@infradead.org> <20150526120215.042527659@infradead.org> <20150529194534.GA31860@redhat.com> <20150529200932.GA2516@redhat.com> <20150530204900.GA17772@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150530204900.GA17772@redhat.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, May 30, 2015 at 10:49:00PM +0200, Oleg Nesterov wrote: > > On Fri, May 29, 2015 at 1:09 PM, Oleg Nesterov wrote: > > > > > > Doesn't it need mb() before "state = readers_slow" to ensure > > > "release" semantics? > __percpu_down_read() lacks another mb() after the "state != BLOCK" > check for the same reason, and we can use smp_load_acquire(state) > instead. I made the below modification to the patch. --- --- a/kernel/locking/percpu-rwsem.c +++ b/kernel/locking/percpu-rwsem.c @@ -51,7 +51,11 @@ void __percpu_down_read(struct percpu_rw smp_mb(); /* A matches D */ - if (likely(sem->state != readers_block)) + /* + * If !readers_block the critical section starts here, matched by the + * release in percpu_up_write(). + */ + if (likely(smp_load_acquire(sem->state) != readers_block)) return; /* @@ -154,8 +158,11 @@ void percpu_up_write(struct percpu_rw_se * One reason that we cannot just immediately flip to readers_fast is * that new readers might fail to see the results of this writer's * critical section. + * + * Therefore we force it through the slow path which guarantees an + * acquire and thereby guarantees the critical section's consistency. */ - sem->state = readers_slow; + smp_store_release(sem->state, readers_slow); /* * Release the write lock, this will allow readers back in the game.