Please reserve INCOMPAT flags

Please reserve INCOMPAT flags

[Andreas Dilger]

Ted,
in addition to the data-in-dirent INCOMPAT flag Rahul sent the patches
for last week, I would like to ensure that we also have the INCOMPAT
flag for large EA-in-inode flag reserved.  This patch is going into
testing at one of our large customers, and I want to make sure that
we don't accidentally get a conflicting INCOMPAT flag assignment.

#define EXT4_FEATURE_INCOMPAT_EA_INODE	0x0400
#define EXT4_FEATURE_INCOMPAT_DIRDATA   0x1000

#define EXT4_EA_INODE_FL	0x00200000 /* Inode used for large EA */

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: Please reserve INCOMPAT flags

[Andreas Dilger]

On 2009-09-06, at 03:25, Andreas Dilger wrote:
> in addition to the data-in-dirent INCOMPAT flag Rahul sent the patches
> for last week, I would like to ensure that we also have the INCOMPAT
> flag for large EA-in-inode flag reserved.  This patch is going into
> testing at one of our large customers, and I want to make sure that
> we don't accidentally get a conflicting INCOMPAT flag assignment.
>
> #define EXT4_FEATURE_INCOMPAT_EA_INODE	0x0400
> #define EXT4_FEATURE_INCOMPAT_DIRDATA   0x1000
>
> #define EXT4_EA_INODE_FL	0x00200000 /* Inode uses large EA */


Hi Ted,
I noticed Aneesh proposing to use the 0x0400 INCOMPAT flag for the  
NFSv4 ACL support, but this conflicts with the large EA feature we had  
previously discussed.  We now have a couple of customers using the  
large EA feature at this point, and I wouldn't want to break their  
filesystem as a result of an avoidable conflict.

I'll attach patches for this, which will hopefully make it easier, and  
the patch tracking tool will keep this visible.

Aneesh, maybe you can use 0x0800 for the INCOMPAT_RICHACL?

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: Please reserve INCOMPAT_EA_INODE flag

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

Attached is a patch to reserve the EA inode feature flag.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[-- Attachment #2: e2fsprogs-incompat_large_ea.diff --]
[-- Type: application/octet-stream, Size: 2292 bytes --]

Reserve the EXT4_INCOMPAT_EA_INODE feature flag for use with
large extended attributes that are stored in a separate inode.
This changes the on-disk format in several ways:

First, replace the e_value_block field with e_value_inum, so that
an xattr entry can reference an external inode.  This field is
currently unused, as all of the entries live in the same block.

struct ext2_ext_attr_entry {
 	__u8	e_name_len;	/* length of name */
 	__u8	e_name_index;	/* attribute name index */
 	__le16	e_value_offs;	/* offset in disk block of value */
>	__le32	e_value_inum;	/* inode in which the value is stored */
 	__le32	e_value_size;	/* size of attribute value */
 	__le32	e_hash;		/* hash value of name and value */
 	char	e_name[0];	/* attribute name */
}

Second, add a flag to the inode that indicates it is using a large
(external) extended attribute.  This is needed so that when unlinking
an inode the xattrs will be scanned to unlink the xattr inodes
referenced by the main inode.

Third, for inodes that have a number of xattrs that are larger than
a single block, but not large enough to justify an external inode
(less than 64kB total xattr size, due to e_value_offs limitation)
the ext2_ext_attr_header->h_blocks field can grow beyond a single
block to represent a contiguous allocation of blocks for the xattr.

Signed-off-by: Andreas Dilger <adilger@sun.com>

diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 114b001..6a104cb 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -276,6 +276,7 @@ struct ext2_dx_countlimit {
 #define EXT2_TOPDIR_FL			0x00020000 /* Top of directory hierarchies*/
 #define EXT4_HUGE_FILE_FL               0x00040000 /* Set to each huge file */
 #define EXT4_EXTENTS_FL 		0x00080000 /* Inode uses extents */
+#define EXT4_EA_INODE_FL		0x00200000 /* Inode has EA inode */
 #define EXT2_RESERVED_FL		0x80000000 /* reserved for ext2 lib */
 
 #define EXT2_FL_USER_VISIBLE		0x000BDFFF /* User visible flags */
@@ -647,6 +648,7 @@ struct ext2_super_block {
 #define EXT4_FEATURE_INCOMPAT_64BIT		0x0080
 #define EXT4_FEATURE_INCOMPAT_MMP		0x0100
 #define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
+#define EXT4_FEATURE_INCOMPAT_EA_INODE		0x0400
 
 
 #define EXT2_FEATURE_COMPAT_SUPP	0

[PATCH] Please reserve INCOMPAT_DIRDATA flag

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 165 bytes --]

Attached is a patch to reserve the data-in-dirent feature flag.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[-- Attachment #2: e2fsprogs-incompat_dirdata.diff --]
[-- Type: application/octet-stream, Size: 2294 bytes --]

Reserve the EXT4_FEATURE_INCOMPAT_DIRDATA feature flag
for adding extra file data in ext2_dir_entry_2 entries.

This changes the on-disk layout in the following way.

Firstly, the ext2_dir_entry_2 file_type field now has a mask:
#define EXT4_FT_MASK 0xf
that limits the "filetype" information to the low 4 bits of
this field.  Since these values are sequentially assigned,
this allows for up to 7 more filetypes to be assigned.  When
reading the "filetype" field, the high 4 bits should be masked
off when converting to DT_* filetypes for userspace.

The high 4 bits of "filetype" are used as a bitmask to register
up to 4 different "extended" directory entry fields.  Extended
data fields are packed without alignment into the directory entry
after the "name" field in order of increasing bitmask value, for
each field where bit is set.  In order to avoid the need to
"understand" each of the extended fields, the first byte of each
extended data field holds the size of that data field (including
the size itself), so they can be skipped if not understood.  For
fields that change the semantics of the filesystem it is expected
that a separate ROCOMPAT or INCOMPAT field is registered.

There is a single dirent data type defined currently, for Lustre:
#define EXT4_DIRENT_LUFID 		0x10
which holds a 128-bit file identifier.  It is expected that
if there are 64-bit inode values that this will be assigned
the 0x20 value.

Should a need ever arise to use all 4 of the extended dirent
data fields, it would be possible to keep the last bit (0x80)
for use as a multiplexor that stores a 1-byte aggregate data 
size, then a series of "<u8_size><u8_type><data>" records in
the last extended data record.  It is not expected that this
will actually be needed in the lifetime of ext4.

Signed-off-by: Andreas Dilger <adilger@sun.com>

diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 114b001..18f6909 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -648,6 +648,7 @@ struct ext2_super_block {
 #define EXT4_FEATURE_INCOMPAT_MMP		0x0100
 #define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
 #define EXT4_FEATURE_INCOMPAT_EA_INODE		0x0400
+#define EXT4_FEATURE_INCOMPAT_DIRDATA		0x1000
 
 
 #define EXT2_FEATURE_COMPAT_SUPP	0

Re: [PATCH] Please reserve INCOMPAT_DIRDATA flag

[tytso]

On Mon, Nov 30, 2009 at 02:16:48PM -0700, Andreas Dilger wrote:
> Attached is a patch to reserve the data-in-dirent feature flag.

I had reserved this in ext4.h in the kernel, this is just a note to
say that I've checked in this change into e2fsprogs as well.

    	       	                    -	  Ted

Re: Please reserve INCOMPAT_EA_INODE flag

[tytso]

On Mon, Nov 30, 2009 at 12:31:40PM -0700, Andreas Dilger wrote:
> Attached is a patch to reserve the EA inode feature flag.

I had reserved this in ext4.h in the kernel, this is just a note to
say that I've checked in this change into e2fsprogs as well.

    	      	      	      - Ted

Re: Please reserve INCOMPAT flags

[Aneesh Kumar K.V]

On Mon, Nov 30, 2009 at 12:15:11PM -0700, Andreas Dilger wrote:
> On 2009-09-06, at 03:25, Andreas Dilger wrote:
> >in addition to the data-in-dirent INCOMPAT flag Rahul sent the patches
> >for last week, I would like to ensure that we also have the INCOMPAT
> >flag for large EA-in-inode flag reserved.  This patch is going into
> >testing at one of our large customers, and I want to make sure that
> >we don't accidentally get a conflicting INCOMPAT flag assignment.
> >
> >#define EXT4_FEATURE_INCOMPAT_EA_INODE	0x0400
> >#define EXT4_FEATURE_INCOMPAT_DIRDATA   0x1000
> >
> >#define EXT4_EA_INODE_FL	0x00200000 /* Inode uses large EA */
> 
> 
> Hi Ted,
> I noticed Aneesh proposing to use the 0x0400 INCOMPAT flag for the
> NFSv4 ACL support, but this conflicts with the large EA feature we
> had previously discussed.  We now have a couple of customers using
> the large EA feature at this point, and I wouldn't want to break
> their filesystem as a result of an avoidable conflict.
> 
> I'll attach patches for this, which will hopefully make it easier,
> and the patch tracking tool will keep this visible.
> 
> Aneesh, maybe you can use 0x0800 for the INCOMPAT_RICHACL?
> 
> 

I have updated richacl patches to use 0x0800.

-aneesh

Re: Please reserve INCOMPAT flags

[tytso]

On Sun, Sep 06, 2009 at 11:25:46AM +0200, Andreas Dilger wrote:
> Ted,
> in addition to the data-in-dirent INCOMPAT flag Rahul sent the patches
> for last week, I would like to ensure that we also have the INCOMPAT
> flag for large EA-in-inode flag reserved.  This patch is going into
> testing at one of our large customers, and I want to make sure that
> we don't accidentally get a conflicting INCOMPAT flag assignment.
> 
> #define EXT4_FEATURE_INCOMPAT_EA_INODE	0x0400
> #define EXT4_FEATURE_INCOMPAT_DIRDATA   0x1000
> 
> #define EXT4_EA_INODE_FL	0x00200000 /* Inode used for large EA */
> 

Reserving... but stupid question, do you know if anyone is currently
using the inode flag 0x00100000?  I want to make sure that's properly
documented and reserved.

Thanks,

							- Ted

Re: Please reserve INCOMPAT flags

[Andreas Dilger]

On 2010-01-24, at 12:48, tytso@mit.edu wrote:
> On Sun, Sep 06, 2009 at 11:25:46AM +0200, Andreas Dilger wrote:
>> in addition to the data-in-dirent INCOMPAT flag Rahul sent the  
>> patches
>> for last week, I would like to ensure that we also have the INCOMPAT
>> flag for large EA-in-inode flag reserved.  This patch is going into
>> testing at one of our large customers, and I want to make sure that
>> we don't accidentally get a conflicting INCOMPAT flag assignment.
>>
>> #define EXT4_FEATURE_INCOMPAT_EA_INODE	0x0400
>> #define EXT4_FEATURE_INCOMPAT_DIRDATA   0x1000
>>
>> #define EXT4_EA_INODE_FL	0x00200000 /* Inode used for large EA */
>
> Reserving... but stupid question, do you know if anyone is currently
> using the inode flag 0x00100000?  I want to make sure that's properly
> documented and reserved.


It doesn't seem that ext2/3/4 are using the 0x00100000 value itself,  
but it seems the VFS is using this value for FS_DIRECTIO_FL.  Should  
we reserve this in the ext4 flags also, to avoid collisions?  I'm not  
sure what that flag is for, possibly to force all IO to the file to be  
uncached?

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Motion to nuke FS_DIRECTIO_FL

[tytso]

On Sun, Jan 24, 2010 at 11:18:47PM -0700, Andreas Dilger wrote:
> 
> It doesn't seem that ext2/3/4 are using the 0x00100000 value itself,
> but it seems the VFS is using this value for FS_DIRECTIO_FL.  Should
> we reserve this in the ext4 flags also, to avoid collisions?  I'm
> not sure what that flag is for, possibly to force all IO to the file
> to be uncached?

Hmm, absolutely nothing seems to use FS_DIRECTIO_FL; it looks like it
was introduced by GFS2 in commit 128e5eba in 2006 and then dropped in
commit c9f6a6bb in 2008, but we never killed the FS_DIRECTIO_FL flag
itself in include/linux/fs.h.

The summary line for c9f6a6bb is a bit amusing:

    [GFS2] Remove support for unused and pointless flag

Heh.

Sounds like we should just kill it.  Any objections?

					- Ted

Re: Motion to nuke FS_DIRECTIO_FL

[Steven Whitehouse]

Hi,

On Mon, 2010-01-25 at 03:06 -0500, tytso@mit.edu wrote:
> On Sun, Jan 24, 2010 at 11:18:47PM -0700, Andreas Dilger wrote:
> > 
> > It doesn't seem that ext2/3/4 are using the 0x00100000 value itself,
> > but it seems the VFS is using this value for FS_DIRECTIO_FL.  Should
> > we reserve this in the ext4 flags also, to avoid collisions?  I'm
> > not sure what that flag is for, possibly to force all IO to the file
> > to be uncached?
> 
> Hmm, absolutely nothing seems to use FS_DIRECTIO_FL; it looks like it
> was introduced by GFS2 in commit 128e5eba in 2006 and then dropped in
> commit c9f6a6bb in 2008, but we never killed the FS_DIRECTIO_FL flag
> itself in include/linux/fs.h.
> 
> The summary line for c9f6a6bb is a bit amusing:
> 
>     [GFS2] Remove support for unused and pointless flag
> 
> Heh.
> 
> Sounds like we should just kill it.  Any objections?
> 
> 					- Ted
No. Sounds good to me. It was never used with GFS2 and it a left-over
from GFS1 which had a flag allowing all "normal" I/O to be turned into
O_DIRECT I/O depending on an inode flag. The idea failed due to
alignment restrictions of course and nobody actually used it,

Steve.