From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tao Ma Subject: Re: [PATCH] ext4: Don't set s_buddy_cache->i_ino to EXT4_BAD_INO Date: Wed, 17 Aug 2011 23:38:51 +0800 Message-ID: <4E4BE08B.4060900@tao.ma> References: <1313569222-5079-1-git-send-email-tm@tao.ma> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: "linux-ext4@vger.kernel.org" , Yu Jian , Andreas Dilger , Theodore Ts'o To: Andreas Dilger Return-path: Received: from oproxy8-pub.bluehost.com ([69.89.22.20]:37846 "HELO oproxy8-pub.bluehost.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751764Ab1HQPi6 (ORCPT ); Wed, 17 Aug 2011 11:38:58 -0400 In-Reply-To: Sender: linux-ext4-owner@vger.kernel.org List-ID: On 08/17/2011 11:31 PM, Andreas Dilger wrote: > On 2011-08-17, at 2:20 AM, Tao Ma wrote: >> In 85fe4025, Christoph removed get_next_ino() from new_inode() and >> do s_buddy_cache->i_ino = get_next_ino(); >> And then 48e6061b gives EXT4_BAD_INO to this inode. But actually >> s_buddy_cache is used directly and we never get it from an inode number. >> So it should be safe for us to not set i_ino at all and I guess that >> is the case Christoph described in his commit log of 85fe4025 >> "For a few more filesystems we can avoid assigning any inode number given >> that they aren't user visible". > > I don't see that setting i_ino is harmful here (only done once per filesystem mount) and the comment clearly indicates that it is for precautionary reasons only. > > If this inode happens to appear in a crashdump for some reason, I'd prefer it has an inode number that can be identified easily rather than some random value left over from a previously allocated and freed inode, because inode_init_always() does not zero out i_ino. Fair enough. Thanks for the explanation. Thanks Tao > >> Cc: Yu Jian >> Cc: Andreas Dilger >> Cc: "Theodore Ts'o" >> Signed-off-by: Tao Ma >> --- >> fs/ext4/mballoc.c | 5 ----- >> 1 files changed, 0 insertions(+), 5 deletions(-) >> >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c >> index 17a5a57..982f783 100644 >> --- a/fs/ext4/mballoc.c >> +++ b/fs/ext4/mballoc.c >> @@ -2342,11 +2342,6 @@ static int ext4_mb_init_backend(struct super_block *sb) >> ext4_msg(sb, KERN_ERR, "can't get new inode"); >> goto err_freesgi; >> } >> - /* To avoid potentially colliding with an valid on-disk inode number, >> - * use EXT4_BAD_INO for the buddy cache inode number. This inode is >> - * not in the inode hash, so it should never be found by iget(), but >> - * this will avoid confusion if it ever shows up during debugging. */ >> - sbi->s_buddy_cache->i_ino = EXT4_BAD_INO; >> EXT4_I(sbi->s_buddy_cache)->i_disksize = 0; >> for (i = 0; i < ngroups; i++) { >> desc = ext4_get_group_desc(sb, i, NULL); >> -- >> 1.7.0.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html