fast EA for ext3

hi!

this patch implements possibility to store EA into inode body which may be larger 128 bytes in 2.5 kernel. I decided to touch current code as small as possible. format of inode with EA is following:

<old inode>     /* 128 bytes */
i_extra_isize   /* 2 bytes: how many bytes used _before_ EAs */
i_pad1          /* 2 bytes */
i_uatime        /* 4 bytes: access time (useconds) */

i_uctime /* 4 bytes: change time (useconds) */
i_umtime /* 4 bytes: modification time (useconds) */
i_crtim /* 4 bytes: creation time (seconds) */
i_ucrtime /* 4 bytes: creation time (useconds) */
<EA magic> /* 4 bytes: indication that inode has EA in body */
<list of EAs> /* structure is the same current EA uses */

tested with setfattr/getfattr tools.
there is patch against e2fsprogs-1.33 which adds ability to show EA stored in inode body.

with best regards, Alex

diff -puNr linux-2.5.69/fs/ext3/ialloc.c edited/fs/ext3/ialloc.c
@@ -576,6 +576,17 @@ repeat:
 
 	ei->i_state = EXT3_STATE_NEW;
 
+	ei->i_crtime = CURRENT_TIME;	
+	if (EXT3_INODE_SIZE(inode->i_sb) > EXT3_GOOD_OLD_INODE_SIZE) {
+		ei->i_extra_isize = 	sizeof(__u16)	/* i_extra_isize */
+					+ sizeof(__u16)	/* i_pad1 */
+					+ sizeof(__u32)	/* i_uatime */
+					+ sizeof(__u32)	/* i_uctime */
+					+ sizeof(__u32)	/* i_umtime */
+					+ sizeof(__u32);/* i_udtime */
+	} else
+		ei->i_extra_isize = 0;
+
 	unlock_super(sb);
 	ret = inode;
 	if(DQUOT_ALLOC_INODE(inode)) {

diff -puNr linux-2.5.69/fs/ext3/inode.c edited/fs/ext3/inode.c
@@ -2326,6 +2326,21 @@ void ext3_read_inode(struct inode * inod
 		ei->i_data[block] = iloc.raw_inode->i_block[block];
 	INIT_LIST_HEAD(&ei->i_orphan);
 
+	if (EXT3_INODE_SIZE(inode->i_sb) > EXT3_GOOD_OLD_INODE_SIZE) {
+		ei->i_extra_isize = le16_to_cpu(raw_inode->i_extra_isize);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_uatime))
+			inode->i_atime.tv_nsec = le32_to_cpu(raw_inode->i_uatime);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_uctime))
+			inode->i_ctime.tv_nsec = le32_to_cpu(raw_inode->i_uctime);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_umtime))
+			inode->i_mtime.tv_nsec = le32_to_cpu(raw_inode->i_umtime);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_crtime))
+			ei->i_crtime.tv_sec = le32_to_cpu(raw_inode->i_crtime);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_ucrtime))
+			ei->i_crtime.tv_nsec = le32_to_cpu(raw_inode->i_ucrtime);
+	} else
+		ei->i_extra_isize = 0;
+	
 	brelse (iloc.bh);
 
 	if (S_ISREG(inode->i_mode)) {

@@ -2456,6 +2471,20 @@ static int ext3_do_update_inode(handle_t
 	else for (block = 0; block < EXT3_N_BLOCKS; block++)
 		raw_inode->i_block[block] = ei->i_data[block];
 
+	if (EXT3_INODE_SIZE(inode->i_sb) > EXT3_GOOD_OLD_INODE_SIZE) {
+		raw_inode->i_extra_isize = cpu_to_le16(ei->i_extra_isize);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_uatime))
+			raw_inode->i_uatime = cpu_to_le32(inode->i_atime.tv_nsec);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_uctime))
+			raw_inode->i_uctime = cpu_to_le32(inode->i_ctime.tv_nsec);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_umtime))
+			raw_inode->i_umtime = cpu_to_le32(inode->i_mtime.tv_nsec);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_crtime))
+			raw_inode->i_crtime = cpu_to_le32(ei->i_crtime.tv_sec);
+		if (ei->i_extra_isize >= EXT3_FIELD_END(i_ucrtime))
+			raw_inode->i_ucrtime = cpu_to_le32(ei->i_crtime.tv_nsec);
+	}
+
 	BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata");
 	rc = ext3_journal_dirty_metadata(handle, bh);
 	if (!err)

diff -puNr linux-2.5.69/fs/ext3/xattr.h edited/fs/ext3/xattr.h
@@ -75,7 +75,8 @@ extern int ext3_removexattr(struct dentr
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext3_xattr_list(struct inode *, char *, size_t);
 extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
-extern int ext3_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
+extern int ext3_xattr_set_handle(handle_t *, struct inode *, int, const char *,const void *,size_t,int);
+extern int ext3_xattr_block_set(handle_t *, struct inode *, int, const char *,const void *,size_t,int);
 
 extern void ext3_xattr_delete_inode(handle_t *, struct inode *);
 extern void ext3_xattr_put_super(struct super_block *);

diff -puNr linux-2.5.69/include/linux/ext3_fs_i.h edited/include/linux/ext3_fs_i.h
@@ -98,6 +98,12 @@ struct ext3_inode_info {
 	 */
 	struct rw_semaphore truncate_sem;
 	struct inode vfs_inode;
+
+	/* creation time */
+	struct timespec i_crtime; 
+
+	/* on-disk additional lenght */
+	__u16 i_extra_isize;
 };
 
 #endif	/* _LINUX_EXT3_FS_I */

diff -puNr linux-2.5.69/include/linux/ext3_fs.h edited/include/linux/ext3_fs.h
@@ -270,8 +270,19 @@ struct ext3_inode {
 			__u32	m_i_reserved2[2];
 		} masix2;
 	} osd2;				/* OS dependent 2 */
+	__u16	i_extra_isize;
+	__u16	i_pad1;
+	__u32	i_uatime;	/* access time (useconds) */
+	__u32	i_uctime;	/* change time (useconds) */
+	__u32	i_umtime;	/* modification time (useconds) */
+	__u32	i_crtime;	/* creation time (seconds) */
+	__u32	i_ucrtime;	/* creation time (useconds) */
 };
 
+#define EXT3_FIELD_END(field) (offsetof(struct ext3_inode, field) - \
+				EXT3_GOOD_OLD_INODE_SIZE + \
+				sizeof(*(&((struct ext3_inode *) 0)->field)))
+				
 #define i_size_high	i_dir_acl
 
 #if defined(__KERNEL__) || defined(__linux__)

What Alex didn't mention was the reason why we developed this patch in the first place - speed. Storing EAs in external blocks is slow, and if you have a lot of inodes with external EAs it is fatal, as the EA blocks consume so much RAM that they force all of the inode/directory data out of cache.

Some benchmarks (from Alex):

Some minor improvements could be made, such as not storing user EAs inside the inode if there are system EAs that could be stored there.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

The patch looks very good. A few issues come to mind:

- What's the status of the other fixed fields in the large inodes? Is there agreement on adding exactly the fields in the patch?

- A common operation is to change an attribute's value without changing the value's length. The current code does not optimize this case, but it should. The patch makes this optimization a little harder.

- struct ext3_xattr_entry makes little sense in an inode. The e_value_block and e_hash fields should go away. The e_value_size field could be shrunk from 4 to 2 bytes. This would save 10 bytes per attribute. The code wouldn't become a lot more messy from that.

- In-inode EA support should be configurable. I would like to choose whether I want this feature at compile time, with as few #ifdef's as possible.

The current implementation is not suitable for applications which store unique EAs for very many inodes. Storing the EAs in the inodes is much better, but increasing the inode size also costs space.

Nice. That's probably with unique attributes per inode.

I don't know which strategy will be best here. We don't have to solve this problem now, though.

By the way, you will likely also need dynamically allocated inodes, right? What's the status here?

Cheers,
Andreas.

------------------------------------------------------------------
Andreas Gruenbacher SuSE Linux AG
mailto:[email protected] Deutschherrnstr. 15-19
http://www.suse.de/ D-90429 Nuernberg, Germany

Not specifically - the "microsecond times" were pretty much a given (hmm, which I guess are actually nanosecond times, so maybe should be renamed), I think (especially since the VFS now supports that). I have been considering if we wanted to overload the on-disk fields so that the high bits of the usecond timestamps also serve as bits 32+ of the seconds field, so we can handle y2038 problems in advance. All this would require now is either masking the high bits and possibly shifting the low bits depending on how many bits we wanted (nseconds are < 2^30, so we have 2 bits free right now for ~600 years of time growth), but we could have more at some small expense in time resolution).

Sure.

We did it this way specifically to avoid duplicating all of the code that uses ext3_xattr_entry - we had originally wanted to have a smaller in-inode "entry" struct. I suppose we could convert from the in-inode short format to the larger ext3_xattr_entry before calling the xattr code.

Still, having the same struct does have some advantages - e_value_block could still be used to store larger EA data outside the inode but still keep the entries in the inode for faster lookups. It also facilitates moving EAs from inside the inode to an external block and vice-versa if that becomes desirable (e.g. keeping "system" attributes in the inode, and "user" attributes outside if there are size issues).

I suppose that might be marginally desirable, but if you don't have large inodes you don't activate that code.

That is exactly the situation we are in - each inode has unique EA data. In a test we needed to pass for Lustre (10M files in a single directory), we had ~1.25GB of inode table data, ~256MB of directory data, and a whopping 40GB of EA blocks, of which we needed about 600MB.

Yes. The mbcache doesn't help us at all in this case.

Definitely not.

Yes, eventually. We are in the midst of getting Lustre working on 2.5, and our customers have very big storage needs (2TB just for the metadata filesystem - only inodes, EAs, directories, no file data at all there), so dynamic inode tables isn't #1 on our list of problems to fix. However, we will get there eventually I hope.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/