On Sat, Oct 12, 2013 at 01:23:15AM -0700, Darrick J. Wong wrote:
> On Sat, Oct 12, 2013 at 04:16:38PM +0800, Zheng Liu wrote:
> > On Fri, Oct 11, 2013 at 05:39:45PM -0700, Darrick J. Wong wrote:
> > > On Fri, Aug 02, 2013 at 05:49:42PM +0800, Zheng Liu wrote:
> > > > From: Zheng Liu <wenqing.lz@xxxxxxxxxx>
> > > >
> > > > Inline_data feature can be set when ext_attr feature is enabled. 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.
> > > >
> > > > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> > > > Signed-off-by: Zheng Liu <wenqing.lz@xxxxxxxxxx>
> > > > ---
> > > > misc/tune2fs.8.in | 5 +++++
> > > > misc/tune2fs.c | 17 ++++++++++++++++-
> > > > 2 files changed, 21 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
> > > > index 55c6dd9..b768fff 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 inode.
> > > > +.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 7d6520e..f22a736 100644
> > > > --- a/misc/tune2fs.c
> > > > +++ b/misc/tune2fs.c
> > > > @@ -137,7 +137,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|
> > > > @@ -995,6 +996,20 @@ mmp_error:
> > > > disable_uninit_bg(fs,
> > > > EXT4_FEATURE_RO_COMPAT_GDT_CSUM);
> > > >
> > > > + if (FEATURE_ON(E2P_FEATURE_INCOMPAT,
> > > > + EXT4_FEATURE_INCOMPAT_INLINE_DATA)) {
> > > > + /* Inline_data feature cannot be enabled if ext_attr is
> > > > + * disabled.
> > > > + */
> > >
> > > Multiline comment issue.
> > >
> > > > + if (!(fs->super->s_feature_compat &
> > > > + EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> > >
> > > EXT2_HAS_COMPAT_FEATURE()?
> > >
> > > > + fputs(_("The inline_data feature cannot "
> > > > + "be set if ext_attr feature is disabled.\n"),
> > > > + stderr);
> > > > + return 1;
> > > > + }
> > > > + }
> > > > +
> > >
> > > Maybe I'm missing something, but I don't see anything prohibiting the user from
> > > disabling inline_data? Or from doing the conversion?
> >
> > Yes, now in tune2fs we can only enable inline_data feature. If we want
> > to disable inline_data, that means that we need to allocate a block for
> > every inode with inline data, and we could exhaust all disk space.
> > Another implementation might be just clear inline_data flag from
> > super_block and doesn't allocate a block for these inodes. The former
> > obviously is not a good choice. But the latter might be better. I am
> > really not sure for this. What do you think?
>
> I don't think it's a good idea to turn off inline_data once it's been enabled,
> just like we don't let people turn off extents...
>
> *Oh*, duh, I didn't notice that INLINE_DATA is *NOT* in the clear_ok_features
> list. Never mind this comment, I'm just making noise.
>
> (The complaints about the comment issue and not using EXT2_HAS_COMPAT_FEATURE
> remain.)
Got it. :)
- Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
