From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boaz Harrosh Subject: Re: [RFC] vfs/inode: For none-block-based filesystems default to sb->s_bdi Date: Mon, 04 Oct 2010 18:02:13 -0400 Message-ID: <4CAA4EE5.2070308@panasas.com> References: <4CAA4B1D.1010904@panasas.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Trond Myklebust , Benny Halevy To: Jan Kara , Christoph Hellwig , Al Viro , linux-fsdevel Return-path: Received: from daytona.panasas.com ([67.152.220.89]:11203 "EHLO daytona.int.panasas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752565Ab0JDWCY (ORCPT ); Mon, 4 Oct 2010 18:02:24 -0400 In-Reply-To: <4CAA4B1D.1010904@panasas.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 10/04/2010 05:46 PM, Boaz Harrosh wrote: > > In alloc_inode (inode_init_always) in the case that sb->s_bdev > is NULL. Should we not use sb->s_bdi as the default > mapping->backing_dev_info? > > This fixes my none-block-based filesystem recent WARN_ON > at fs/fs-writeback.c:87 inode_to_bdi() > > If not done here I'll need to do this in 5 different cases > in FS code. (OK the code could enjoy some re-factoring). > > It does look logical the question is how many FSs will now > get broken? > > Signed-off-by: Boaz Harrosh > --- > fs/inode.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index 8646433..200314f 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -181,6 +181,8 @@ int inode_init_always(struct super_block *sb, struct inode *inode) > > bdi = sb->s_bdev->bd_inode->i_mapping->backing_dev_info; > mapping->backing_dev_info = bdi; > + } else { > + mapping->backing_dev_info = sb->s_bdi; > } > inode->i_private = NULL; > inode->i_mapping = mapping; Sorry I've just seen Jan's patch: From: Jan Kara Date: Mon, 27 Sep 2010 23:56:48 +0200 Subject: [PATCH] bdi: Initialize inode->i_mapping.backing_dev_info to sb->s_bdi Currently, we initialize inode->i_mapping.backing_dev_info to the bdi of device sb->s_bdev points to. However there is quite a big number of filesystems that do not set sb->s_bdev (because they do not have one) but do set sb->s_bdi. These filesystems would generally benefit from setting inode->i_mapping.backing_dev_info to their s_bdi because otherwise their inodes would point to default_backing_dev_info and thus dirty inode tracking would happen there. So change inode initialization code to use sb->s_bdi if it is available. Signed-off-by: Jan Kara --- fs/inode.c | 22 ++++++++++++++-------- 1 files changed, 14 insertions(+), 8 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 8646433..e415be4 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -172,15 +172,21 @@ int inode_init_always(struct super_block *sb, struct inode *inode) mapping->writeback_index = 0; /* - * If the block_device provides a backing_dev_info for client - * inodes then use that. Otherwise the inode share the bdev's - * backing_dev_info. + * If the filesystem provides a backing_dev_info for client inodes + * then use that. Otherwise inodes share default_backing_dev_info. */ - if (sb->s_bdev) { - struct backing_dev_info *bdi; - - bdi = sb->s_bdev->bd_inode->i_mapping->backing_dev_info; - mapping->backing_dev_info = bdi; + if (sb->s_bdi && sb->s_bdi != &noop_backing_dev_info) { + /* + * Catch cases where filesystem might be bitten by using s_bdi + * instead of sb->s_bdev. Can be removed in 2.6.38. + */ + if (sb->s_bdev) { + struct backing_dev_info *bdi = + sb->s_bdev->bd_inode->i_mapping->backing_dev_info; + WARN(bdi != sb->s_bdi, "s_bdev bdi %s != s_bdi %s\n", + bdi->name, sb->s_bdi->name); + } + mapping->backing_dev_info = sb->s_bdi; } inode->i_private = NULL; inode->i_mapping = mapping; That works for me as well. Was it decided how to solve this? Other wise I'll need to patch exofs, ASAP for this -rc Thanks Boaz