From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754716AbbFWLJr (ORCPT ); Tue, 23 Jun 2015 07:09:47 -0400 Received: from cantor2.suse.de ([195.135.220.15]:46031 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754628AbbFWLJk (ORCPT ); Tue, 23 Jun 2015 07:09:40 -0400 Date: Tue, 23 Jun 2015 13:09:33 +0200 From: Jan Kara To: Dave Hansen Cc: jack@suse.cz, viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com, tim.c.chen@linux.intel.com, ak@linux.intel.com Subject: Re: [RFC][PATCH 1/2] fs: use RCU for free_super() vs. __sb_start_write() Message-ID: <20150623110933.GE2427@quack.suse.cz> References: <20150619223223.10B658AD@viggo.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150619223223.10B658AD@viggo.jf.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 19-06-15 15:32:23, Dave Hansen wrote: > > Currently, __sb_start_write() and freeze_super() can race with > each other. __sb_start_write() uses a smp_mb() to ensure that > freeze_super() can see its write to sb->s_writers.counter and > that it can see freeze_super()'s update to sb->s_writers.frozen. > This all seems to work fine. > > But, this smp_mb() makes __sb_start_write() the single hottest > function in the kernel if I sit in a loop and do tiny write()s to > tmpfs over and over. This is on a very small 2-core system, so > it will only get worse on larger systems. > > This _seems_ like an ideal case for RCU. __sb_start_write() is > the RCU read-side and is in a very fast, performance-sensitive > path. freeze_super() is the RCU writer and is in an extremely > rare non-performance-sensitive path. > > Instead of doing and smp_wmb() in __sb_start_write(), we do > rcu_read_lock(). This ensures that a CPU doing freeze_super() > can not proceed past its synchronize_rcu() until the grace > period has ended and the 's_writers.frozen = SB_FREEZE_WRITE' > is visible to __sb_start_write(). > > One question here: Does the work that __sb_start_write() does in > a previous grace period becomes visible to freeze_super() after > its call to synchronize_rcu()? It _seems_ like it should, but it > seems backwards to me since __sb_start_write() is the "reader" in > this case. > > This patch increases the number of writes/second that I can do > by 10.4%. > > Does anybody see any holes with this? Nice speed up and looks good to me. Just one question below. > @@ -1340,7 +1344,7 @@ int freeze_super(struct super_block *sb) > printk(KERN_ERR > "VFS:Filesystem freeze failed\n"); > sb->s_writers.frozen = SB_UNFROZEN; > - smp_wmb(); > + synchronize_rcu(); Do we really need synchronize_rcu() here? We just need to make sure write to sb->s_writers.frozen happens before we start waking processes... > wake_up(&sb->s_writers.wait_unfrozen); > deactivate_locked_super(sb); > return ret; > @@ -1387,7 +1391,7 @@ int thaw_super(struct super_block *sb) > > out: > sb->s_writers.frozen = SB_UNFROZEN; > - smp_wmb(); > + synchronize_rcu(); > wake_up(&sb->s_writers.wait_unfrozen); And here as well... Honza -- Jan Kara SUSE Labs, CR