Date: Thu, 4 Dec 2008 17:47:08 -0200 From: Gonzalo Nemmi <gnemmi@gmail.com> To: freebsd-stable@freebsd.org, josh.carroll@gmail.com Cc: freebsd-fs@freebsd.org Subject: Re: ext2 inode size patch - RE: PR kern/124621 Message-ID: <200812041747.09040.gnemmi@gmail.com> In-Reply-To: <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com> References: <8cb6106e0811241129o642dcf28re4ae177c8ccbaa25@mail.gmail.com> <20081125150342.GL2042@deviant.kiev.zoral.com.ua> <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <linux/ext2_fs_sb.h>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200812041747.09040.gnemmi>