linux-ext4 - Re: [PATCH v2 19/28] mke2fs: add inline

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Dec 2013 14:18:04 +0800
From:	Zheng Liu <gnehzuil.liu@...il.com>
To:	"Darrick J. Wong" <darrick.wong@...cle.com>
Cc:	linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
	Zheng Liu <wenqing.lz@...bao.com>, Tao Ma <tm@....ma>
Subject: Re: [PATCH v2 19/28] mke2fs: add inline_data support in mke2fs

On Tue, Dec 03, 2013 at 10:02:24PM -0800, Darrick J. Wong wrote:
> On Wed, Dec 04, 2013 at 01:50:52PM +0800, Zheng Liu wrote:
> > [Cc Tao to get some comments]
> > 
> > On Tue, Dec 03, 2013 at 09:26:08PM -0800, Darrick J. Wong wrote:
> > > On Wed, Dec 04, 2013 at 01:21:50PM +0800, Zheng Liu wrote:
> > > > On Tue, Dec 03, 2013 at 08:08:19PM -0800, Darrick J. Wong wrote:
> > > > > On Wed, Dec 04, 2013 at 11:27:57AM +0800, Zheng Liu wrote:
> > > > > > On Tue, Dec 03, 2013 at 02:30:26PM -0800, Darrick J. Wong wrote:
> > > > > > [...]
> > > > > > > > +		 * notify users that inline data will never be useful.
> > > > > > > > +		 */
> > > > > > > > +		if ((fs_param.s_feature_incompat &
> > > > > > > > +		     EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> > > > > > > > +		    inode_size == EXT2_GOOD_OLD_INODE_SIZE) {
> > > > > > > 
> > > > > > > Perhaps I'm missing something here, but why is it impossible to use i_blocks
> > > > > > > for inline data even if there's no space for EAs?
> > > > > > 
> > > > > > If I understand correctly, on kernel side, we determine an inode has
> > > > > > inline data according to whether we have 'system.data' xattr entry on
> > > > > > inode extended attribute space.  If an inode doesn't have enough space
> > > > > > to store an entry with 'system.data', we just think this inode doesn't
> > > > > > has inline data.  So that is why I add this sanity check.
> > > > > 
> > > > > Ok.  I was curious.  Small inode => no inline data seems like an unfortunate
> > > > > restriction to me, but oh well, it's your feature.  I don't plan to go back to
> > > > > 128 byte inodes ever. :)
> > > > > 
> > > > > Also, we could store four more bytes if we created a new e_name_index value (5?
> > > > > 9?) to represent "system.data".  Any thoughts about that?
> > > > 
> > > > Sorry, I don't get your point.  Do you want to create a new e_name_index?
> > > > Any reason lets you want to do this?
> > > 
> > > Yep, that's exactly what I propose to do, so we can cram four more bytes into
> > > the inline data.
> > 
> > Agree.  I believe it is fine.  But I am wondering if it will break the
> > file system that inline data has been enabled.
> 
> There's a fair amount of work needed for fs/ext4/inline.c.  My e2fsprogs thing
> should handle it fine, though I think the inlinedata_max_size function
> somewhere in your patchset might also be broken.
> 
> I suspect a lot depends on how widely deployed inlinedata is inside Taobao, or
> anyone else who's actually running it right now.

That is my concern because at Taobao we have already used inline data on
our own servers.  So obviously it will break our file system. :(

                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html