linux-ext4 - Re: [PATCH v2 20/28] tune2fs: 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]

Message-ID: <20131204040300.GL9535@birch.djwong.org>
Date:	Tue, 3 Dec 2013 20:03:00 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	linux-ext4@...r.kernel.org, "Theodore Ts'o" <tytso@....edu>,
	Zheng Liu <wenqing.lz@...bao.com>
Subject: Re: [PATCH v2 20/28] tune2fs: add inline_data feature in tune2fs

On Wed, Dec 04, 2013 at 11:55:36AM +0800, Zheng Liu wrote:
> On Tue, Dec 03, 2013 at 02:29:46PM -0800, Darrick J. Wong wrote:
> > On Tue, Dec 03, 2013 at 08:11:47PM +0800, Zheng Liu wrote:
> > > From: Zheng Liu <wenqing.lz@...bao.com>
> > > 
> > > Inline_data can be enabled without ext_attr.  Hence we don't check it.
> > > As doing in mke2fs we need to check inode size.
> > > 
> > > Now this feature only can be enabled because we may be out of space when
> > > disabling it.  If this feature is disabled, we need to allocate a block
> > > for every file and directory, and it might exhaust all space.
> > 
> > Could we at least try to turn off inlinedata and decline to clear the flag if
> > we ENOSPC?
> 
> If we want to turn off inline_data, that means that we need to scan all
> used inodes and try to expand it.  It isn't hard but I am wondering if
> someone really want to disable this feature.  In my opinion, I really
> want to enable inline_data feature by default when we believe it is very
> stable because it will benefit for all desktop and Android users.

I guess that really depends on our testers -- on the one hand, I do most of my
testing with scratch filesystems so on a practical level I will never need the
ability to "undo" these sorts of things.

On the other hand, there's the argument that more people will test code if they
think they can go back to the old system if the new one turns out to be a mess.

Same sort of reasoning applies to "why did you implement converting 64bit ->
32bit filesystems?" :)

--D
> 
>                                                 - Zheng
> 
> > 
> > (Yes, that's more of a TODO feature request...)
> > 
> > > Signed-off-by: Theodore Ts'o <tytso@....edu>
> > > Signed-off-by: Zheng Liu <wenqing.lz@...bao.com>
> > > ---
> > >  misc/tune2fs.8.in |    5 +++++
> > >  misc/tune2fs.c    |   16 +++++++++++++++-
> > >  2 files changed, 20 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> > > index 55c6dd9..78f965b 100644
> > > --- a/misc/tune2fs.8.in
> > > +++ b/misc/tune2fs.8.in
> > > @@ -531,6 +531,11 @@ Setting the filesystem feature is equivalent to using the
> > >  .B \-j
> > >  option.
> > >  .TP
> > > +.B inline_data
> > > +Allow data to be stored in the inode and extented attribute area.
> > 
> > "extended"
> > 
> > --D
> > 
> > > +.B Tune2fs
> > > +only supports setting this filesystem feature.
> > > +.TP
> > >  .B large_file
> > >  Filesystem can contain files that are greater than 2GB.  (Modern kernels
> > >  set this feature automatically when a file > 2GB is created.)
> > > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > > index 95c1886..1952081 100644
> > > --- a/misc/tune2fs.c
> > > +++ b/misc/tune2fs.c
> > > @@ -140,7 +140,8 @@ static __u32 ok_features[3] = {
> > >  	EXT2_FEATURE_INCOMPAT_FILETYPE |
> > >  		EXT3_FEATURE_INCOMPAT_EXTENTS |
> > >  		EXT4_FEATURE_INCOMPAT_FLEX_BG |
> > > -		EXT4_FEATURE_INCOMPAT_MMP,
> > > +		EXT4_FEATURE_INCOMPAT_MMP |
> > > +		EXT4_FEATURE_INCOMPAT_INLINE_DATA,
> > >  	/* R/O compat */
> > >  	EXT2_FEATURE_RO_COMPAT_LARGE_FILE |
> > >  		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
> > > @@ -1083,6 +1084,19 @@ mmp_error:
> > >  		disable_uninit_bg(fs,
> > >  				EXT4_FEATURE_RO_COMPAT_GDT_CSUM);
> > >  
> > > +	if (FEATURE_ON(E2P_FEATURE_INCOMPAT,
> > > +		       EXT4_FEATURE_INCOMPAT_INLINE_DATA)) {
> > > +		/*
> > > +		 * Check inode size.  If inode size is 128, tell user that
> > > +		 * inline data is useless.
> > > +		 */
> > > +		if (EXT2_INODE_SIZE(fs->super) == EXT2_GOOD_OLD_INODE_SIZE) {
> > > +			fputs(_("The inode size is too small to "
> > > +				"store inline data.\n"), stderr);
> > > +			return 1;
> > > +		}
> > > +	}
> > > +
> > >  	if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
> > >  				EXT4_FEATURE_RO_COMPAT_QUOTA)) {
> > >  		/*
> > > -- 
> > > 1.7.9.7
> > > 
> > > --
> > > 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
> --
> 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
--
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