From owner-freebsd-fs@FreeBSD.ORG Thu Dec 4 17:13:48 2008 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8678F1065673 for ; Thu, 4 Dec 2008 17:13:48 +0000 (UTC) (envelope-from gnemmi@gmail.com) Received: from mail-qy0-f18.google.com (mail-qy0-f18.google.com [209.85.221.18]) by mx1.freebsd.org (Postfix) with ESMTP id 383F18FC17 for ; Thu, 4 Dec 2008 17:13:48 +0000 (UTC) (envelope-from gnemmi@gmail.com) Received: by qyk11 with SMTP id 11so5106133qyk.19 for ; Thu, 04 Dec 2008 09:13:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:from:to:subject:date :user-agent:cc:references:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:message-id; bh=D/IZxoDBsYC9MtrwgJrfMVshXjkMMSbPZ3AW1LzXp2A=; b=Yps3iF3Kj7ZAEgb23jfCPacFTJ4HCenNfuI5GBfQip/BYzr4Zg0YsEPQYd580Kbtej mrKn/NEkd0Ok/hoZycgV4VejJGNSHql3uO6t8TbnSmB+sQ/ASFWTVQ4SUj5KMIhPhc93 ZJKte0GLemcyAvacAkHObn5p+s/E+Zj68Y/ho= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:subject:date:user-agent:cc:references:in-reply-to :mime-version:content-type:content-transfer-encoding :content-disposition:message-id; b=xTEmDXo2mFOqMlcC7+N8qEcWZRye9gOKp5StffNkHD3qs1CiYdgSsjKH7WWbq9EehR SkaaqyLY9cQvckRi0o49JeSczM5zQ2Raz7KHHioPcbMKpVQMqrr5nMiE3rpd2nL7KBJi zNSAQZP5CAFo5Bj5Ahl4svkkMXqgzI6NPIK1M= Received: by 10.214.149.3 with SMTP id w3mr15054314qad.111.1228409234693; Thu, 04 Dec 2008 08:47:14 -0800 (PST) Received: from ?192.168.1.2? ([190.177.205.95]) by mx.google.com with ESMTPS id 6sm8871917qwk.52.2008.12.04.08.47.12 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 04 Dec 2008 08:47:14 -0800 (PST) From: Gonzalo Nemmi To: freebsd-stable@freebsd.org, josh.carroll@gmail.com Date: Thu, 4 Dec 2008 17:47:08 -0200 User-Agent: KMail/1.9.10 References: <8cb6106e0811241129o642dcf28re4ae177c8ccbaa25@mail.gmail.com> <20081125150342.GL2042@deviant.kiev.zoral.com.ua> <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com> In-Reply-To: <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200812041747.09040.gnemmi@gmail.com> Cc: freebsd-fs@freebsd.org Subject: Re: ext2 inode size patch - RE: PR kern/124621 X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Dec 2008 17:13:48 -0000 On Wednesday 03 December 2008 8:53:43 pm Josh Carroll wrote: > > Ok, I describe my concern once more. I do not object against the checking > > of the inode size. But, if inode size is changed, then some data is added > > to the inode, that could (and usually does, otherwise why extend it ?) > > change intrerpetation of the inode. Thus, we need a verification of the > > fact that simply ignoring added fields does not damage filesystem or > > cause user data corruption. Verification != testing. > > Let me take another crack at explaining why I think this patch is not > dangerous. > > The inode size is determined by reading the following member: > > __u16 s_inode_size; > > of the ext2_super_block structure. > > I took a look at the Linux 2.6.27.7 kernel source, and indeed they do > something very similar if not the same: > > #define EXT2_INODE_SIZE(s) (EXT2_SB(s)->s_inode_size) > > If you compare to what I did: > > #define EXT2_INODE_SIZE(s) ((s)->u.ext2_sb.s_inode_size) > > This is really the same thing, since EXT2_SB is defined (in the Linux > kernel src as): > > #ifdef __KERNEL__ > #include > static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb) > { > return sb->s_fs_info; > } > > And struct ext2_sb_info has the following member: > > int s_inode_size; > > Essentially, the changes I made simply query the existing information > from the filesystem, which is what the Linux kernel does as well. > > The inode size, blocks per group, etc are all defined at filesystem > creation time by mke2fs and it ensures the sanity of the relationship > between the inodes/blocks/block groups. > > The first diagram here: > > http://sunsite.nus.sg/LDP/LDP/tlk/node95.html > > Makes it clear that as long as the number of inodes per block and the > blocks per group is is sane during fs creation, looking up the inode > size as my patch does is fine, since the creation of the filesystem is > ensures a correct by construction situation. We're simply reading the > size of the inode based on the filesystem. > > I hope this is sufficient to convince some further thought about > committing this. > > For those interested in the relevant Linux kernel source, you can have > a look at line 105 of include/linux/ext2_fs.h from the linux-2.6.27.7 > kernel source. > > Thanks, > Josh Could you please point me to your patch and an explanation on how to apply it and test it? I've been following your las emails referencing it ( on @questions and @hackers or current i think it was ... ) and I'd like to give it a spin in here ... I've followed the "can't mount ext2/3" problem for a time (since I have that problem) and would like to know to what extent for patch solves the problem. Here are some of the references: mounting linux partitions Fri May 9 18:05:26 UTC 2008 http://lists.freebsd.org/pipermail/freebsd-questions/2008-May/174588.html bad file descriptor when mounting an ext2fs. Tue Jun 10 11:08:46 UTC 2008 http://lists.freebsd.org/pipermail/freebsd-questions/2008-June/176506.html mounting ext2fs partitions on FBSD7 ( third time a charm?) Fri Jul 4 23:33:53 UTC 2008 http://lists.freebsd.org/pipermail/freebsd-questions/2008-July/178219.html My case: root@inferna:~ # ls -l /dev/ad4s ad4s1% ad4s2% ad4s3% ad4s3a% ad4s3b% ad4s3c% ad4s3d% ad4s3e% ad4s3f% ad4s4% ad4s5% ad4s6% ad4s7% ad4s8% root@inferna:~ # ls -l /dev/ad4s root@inferna:~ # tune2fs -l /dev/ad4s1 | grep "Inode size" Inode size: 256 root@inferna:~ # tune2fs -l /dev/ad4s6 | grep "Inode size" Inode size: 256 root@inferna:~ # tune2fs -l /dev/ad4s7 | grep "Inode size" Inode size: 256 root@inferna:~ # tune2fs -l /dev/ad4s8 | grep "Inode size" Inode size: 256 root@inferna:~ # tune2fs -l /dev/ad4s9 | grep "Inode size" BTW: I'm willing to run any tests you need me too, even if they imply a serious risk of loosing data on the 256 inode partitions. Regards -- Blessings Gonzalo Nemmi