Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932105AbcKUPHa (ORCPT ); Mon, 21 Nov 2016 10:07:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39250 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753622AbcKUPH2 (ORCPT ); Mon, 21 Nov 2016 10:07:28 -0500 Date: Mon, 21 Nov 2016 16:07:23 +0100 From: Oleg Nesterov To: Peter Zijlstra Cc: Davidlohr Bueso , mingo@kernel.org, john.stultz@linaro.org, dimitrysh@google.com, linux-kernel@vger.kernel.org, Davidlohr Bueso Subject: Re: [PATCH 3/3] locking/percpu-rwsem: Avoid unnecessary writer wakeups Message-ID: <20161121150722.GA7951@redhat.com> References: <1479495277-9075-1-git-send-email-dave@stgolabs.net> <1479495277-9075-4-git-send-email-dave@stgolabs.net> <20161121122343.GA635@redhat.com> <20161121122935.GD3092@twins.programming.kicks-ass.net> <20161121124722.GA1459@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161121124722.GA1459@redhat.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 21 Nov 2016 15:07:28 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1167 Lines: 41 On 11/21, Oleg Nesterov wrote: > > No, no, I meant that afaics both readers can see per_cpu_sum() != 0 and > thus the writer won't be woken up. Till the next down_read/up_read. > > Suppose that we have 2 CPU's, both counters == 1, both readers decrement. > its counter at the same time. > > READER_ON_CPU_0 READER_ON_CPU_1 > > --ctr_0; --ctr_1; > > if (ctr_0 + ctr_1) if (ctr_0 + ctr_1) > wakeup(); wakeup(); > > Why we can't miss a wakeup? > > This patch doesn't even add a barrier, but I think wmb() won't be enough > anyway. And in fact I am not sure this optimization makes sense... But it would be nice to avoid wake_up() when the writer sleeps in rcu_sync_enter(). Or this is the "slow mode" sem (cgroup_threadgroup_rwsem). I need to re-check, but what do you think about the change below? Oleg. --- x/kernel/locking/percpu-rwsem.c +++ x/kernel/locking/percpu-rwsem.c @@ -103,7 +103,9 @@ void __percpu_up_read(struct percpu_rw_s __this_cpu_dec(*sem->read_count); /* Prod writer to recheck readers_active */ - wake_up(&sem->writer); + smp_mb(); + if (sem->readers_block) + wake_up(&sem->writer); } EXPORT_SYMBOL_GPL(__percpu_up_read);