From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752722AbbGNNjP (ORCPT ); Tue, 14 Jul 2015 09:39:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60003 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752237AbbGNNjO (ORCPT ); Tue, 14 Jul 2015 09:39:14 -0400 Date: Tue, 14 Jul 2015 15:37:31 +0200 From: Oleg Nesterov To: Jan Kara Cc: Al Viro , Linus Torvalds , Paul McKenney , Peter Zijlstra , Daniel Wagner , Davidlohr Bueso , Ingo Molnar , Tejun Heo , linux-kernel@vger.kernel.org, Dave Hansen Subject: Re: [PATCH RFC 0/4] change sb_writers to use percpu_rw_semaphore Message-ID: <20150714133731.GA24837@redhat.com> References: <20150713212536.GA13855@redhat.com> <20150714104810.GB24369@quack.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150714104810.GB24369@quack.suse.cz> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/14, Jan Kara wrote: > > Hello, > > On Mon 13-07-15 23:25:36, Oleg Nesterov wrote: > > Al, Jan, could you comment? I mean the intent, the patches are > > obviously not for inclusion yet. > > Thanks for the patches! Hum, what do people have with freeze protection > these days? Noone cared about it for years and sudddently two patch sets > within a month :) Anyway, have you seen the patch set from Dave Hansen? > > It is here: https://lkml.org/lkml/2015/6/24/682 > He modifies the freezing primitives in a different way. AFAICS the > resulting performance of the fast path should be about the same. At first glance, 2-3 do something similar, yes... > So unless > I'm missing something and there is a significant performance advantage to > Dave's patches I'm all for using a generic primitive you suggest. I think percpu_rw_semaphore looks a bit better. And even a bit faster. And it will not block __sb_start_write() entirely while freeze_super() sleeps in synchronize_rcu(). freeze_super() should be faster too after rcu_sync changes, but this is not that important. But again, to me the main advantage is that we can use the generic primitives and remove this nontrivial code in fs/super.c. > Can you perhaps work with Dave on some common resolution? Dave, what do you think? Will you agree with percpu_rw_semaphore ? > > - __sb_start_write() will be a little bit faster, but this > > is minor. > > Actually Dave could measure the gain achieved by removing the barrier. It > would be good to verify that your patches achieve a similar gain. The fast path in percpu_down_read() is really fast, it does a plain LOAD plus __this_cpu_add() under preempt_disable(). I doubt this can be improved. The actual code is: static bool update_fast_ctr(struct percpu_rw_semaphore *brw, unsigned int val) { bool success = false; preempt_disable(); if (likely(!atomic_read(&brw->write_ctr))) { __this_cpu_add(*brw->fast_read_ctr, val); success = true; } preempt_enable(); return success; } void percpu_down_read(struct percpu_rw_semaphore *brw) { might_sleep(); if (likely(update_fast_ctr(brw, +1))) { rwsem_acquire_read(&brw->rw_sem.dep_map, 0, 0, _RET_IP_); return; } down_read(&brw->rw_sem); atomic_inc(&brw->slow_read_ctr); __up_read(&brw->rw_sem); } > > - Fix get_super_thawed(), it will spin if MS_RDONLY... > > > > It is not clear to me what exactly should we do, but this > > doesn't look hard. Perhaps it can just return if MS_RDONLY. > > What's the exact problem here? Note that freeze_super() does not do sb_wait_write() (which blocks __sb_start_write) if MS_RDONLY. This means that after 1/4 get_super_thawed() will spin until SB_UNFROZEN in this case, sb_start_write() won't block. But please forget, this is not the problem. I mean, afaics this is easy to fix, but the initial version should just keep ->wait_unfrozen specially for get_super_thawed(), then we can remove it in a separate patch. Thanks! Oleg.