From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 02/11] writeback: switch to per-bdi threads for flushing data Date: Wed, 20 May 2009 14:11:12 +0200 Message-ID: <20090520121111.GF3760@duck.suse.cz> References: <1242649192-16263-1-git-send-email-jens.axboe@oracle.com> <1242649192-16263-3-git-send-email-jens.axboe@oracle.com> <20090520111850.GB3760@duck.suse.cz> <20090520113234.GT11363@kernel.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, chris.mason@oracle.com, david@fromorbit.com, hch@infradead.org, akpm@linux-foundation.org, yanmin_zhang@linux.intel.com To: Jens Axboe Return-path: Received: from cantor.suse.de ([195.135.220.2]:47665 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755518AbZETMLM (ORCPT ); Wed, 20 May 2009 08:11:12 -0400 Content-Disposition: inline In-Reply-To: <20090520113234.GT11363@kernel.dk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed 20-05-09 13:32:34, Jens Axboe wrote: > On Wed, May 20 2009, Jan Kara wrote: > > Hi Jens, > > > > a few comments here. Mainly, I still don't think the sys_sync() is > > working right - see comments below. > > Thanks! I took the liberty of killing some of the code in between, to > make it easier to see. > > > > +void bdi_writeback_all(struct super_block *sb, long nr_pages) > > > +{ > > > + struct backing_dev_info *bdi; > > > + > > > + rcu_read_lock(); > > > + > > > +restart: > > > + list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) { > > Isn't the RCU list here a bit overengineering? AFAICS we use the list > > only here and if I'm grepping right, generic_sync_sb_inodes() is currently > > only used for data integrity sync (after your patches) from fs-writeback.c > > and by UBIFS to do equivalent of writeback_inodes(). So simple spinlock > > guarding the list should be just fine. Or am I missing something? > > Sure, we could. But it's really not that much of a difference, > implementation wise. Yeah. It's just that when I see RCU, I'm a bit cautious what's going on. When I see spinlock, everything is simple and clear ;). And I'm in favor of using the simplest synchronization primitive that does it's work good enough ;). Honza -- Jan Kara SUSE Labs, CR