Add extended attributes to ext2/3

PATCH In review [email protected] 6 participants 10 messages Oct 15, 2002 – Oct 19, 2002 2 messages not in the archive

Subsystems (3)

VFS NFS server ext2 file system

lore

Share & tools

lore

[email protected]

author

2002-10-15 22:17 UTC #0 lore raw

PATCH 2/3 Add extended attributes to ext2/3

This patch adds extended attribute support to the ext3 filesystem. This uses the generic extended attribute patch which was developed by Andreas Gruenbacher and the XFS team. As a result, the user space utilities which work for XFS will also work with these patches.

- Ted

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
#

# fs/Config.help           |    7 
# fs/Config.in             |    1 
# fs/ext3/Makefile         |    6 
# fs/ext3/file.c           |    6 
# fs/ext3/ialloc.c         |    3 
# fs/ext3/inode.c          |   35 -
# fs/ext3/namei.c          |   22 
# fs/ext3/super.c          |   23 
# fs/ext3/symlink.c        |   16 
# fs/ext3/xattr.c          | 1127 +++++++++++++++++++++++++++++++++++++++++++++++
# fs/ext3/xattr.h          |  133 +++++
# fs/ext3/xattr_user.c     |   99 ++++
# include/linux/ext3_fs.h  |   31 -
# include/linux/ext3_jbd.h |    8

# 14 files changed, 1468 insertions(+), 49 deletions(-)
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/10/15 [email protected] 1.852
# Port of (bugfixed) 0.8.50 xattr-ext3 to 2.5. (hch cleanup) #
# This patch adds extended attribute support to the ext3 filesystem. This

# uses the generic extended attribute patch which was developed by Andreas   
# Gruenbacher and the XFS team.  As a result, the user space utilities
# which work for XFS will also work with these patches.
# --------------------------------------------
#
diff -Nru a/fs/Config.help b/fs/Config.help
--- a/fs/Config.help	Tue Oct 15 16:58:13 2002
+++ b/fs/Config.help	Tue Oct 15 16:58:13 2002
@@ -154,6 +154,13 @@
   of your root partition (the one containing the directory /) cannot
   be compiled as a module, and so this may be dangerous.

+CONFIG_EXT3_FS_XATTR

+  Extended attributes are name:value pairs associated with inodes by
+  the kernel or by users (see the attr(5) manual page, or visit
+  <http://acl.bestbits.at/> for details).

+
+ If unsure, say N.
+
CONFIG_JBD
This is a generic journaling layer for block devices. It is currently used by the ext3 file system, but it could also be used to

fs/Config.in +1 -0

diff -Nru a/fs/Config.in b/fs/Config.in
@@ -27,6 +27,7 @@
 dep_tristate 'BFS file system support (EXPERIMENTAL)' CONFIG_BFS_FS $CONFIG_EXPERIMENTAL
 
 tristate 'Ext3 journalling file system support' CONFIG_EXT3_FS
+dep_mbool '  Ext3 extended attributes' CONFIG_EXT3_FS_XATTR $CONFIG_EXT3_FS
 # CONFIG_JBD could be its own option (even modular), but until there are
 # other users than ext3, we will simply make it be the same as CONFIG_EXT3_FS
 # dep_tristate '  Journal Block Device support (JBD for ext3)' CONFIG_JBD $CONFIG_EXT3_FS

fs/ext3/Makefile +6 -0

diff -Nru a/fs/ext3/Makefile b/fs/ext3/Makefile
@@ -7,4 +7,10 @@
 ext3-objs    := balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
 		ioctl.o namei.o super.o symlink.o hash.o
 
+export-objs += xattr.o
+
+ifeq ($(CONFIG_EXT3_FS_XATTR),y)
+ext3-objs += xattr.o xattr_user.o
+endif
+
 include $(TOPDIR)/Rules.make

fs/ext3/file.c +5 -1

diff -Nru a/fs/ext3/file.c b/fs/ext3/file.c
@@ -23,7 +23,7 @@
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/ext3_jbd.h>
-#include <linux/smp_lock.h>
+#include "xattr.h"
 
 /*
  * Called when an inode is released. Note that this is different

@@ -98,5 +98,9 @@
 struct inode_operations ext3_file_inode_operations = {
 	.truncate	= ext3_truncate,
 	.setattr	= ext3_setattr,
+	.setxattr	= ext3_setxattr,
+	.getxattr	= ext3_getxattr,
+	.listxattr	= ext3_listxattr,
+	.removexattr	= ext3_removexattr,
 };

fs/ext3/ialloc.c +3 -0

diff -Nru a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
@@ -25,6 +25,8 @@
 #include <asm/bitops.h>
 #include <asm/byteorder.h>
 
+#include "xattr.h"
+
 /*
  * ialloc.c contains the inodes allocation and deallocation routines
  */

@@ -118,6 +120,7 @@
 	 * as writing the quota to disk may need the lock as well.
 	 */
 	DQUOT_INIT(inode);
+	ext3_xattr_delete_inode(handle, inode);
 	DQUOT_FREE_INODE(inode);
 	DQUOT_DROP(inode);

fs/ext3/inode.c +22 ~~-13~~

diff -Nru a/fs/ext3/inode.c b/fs/ext3/inode.c
@@ -42,6 +42,18 @@
  */
 #undef SEARCH_FROM_ZERO
 
+/*
+ * Test whether an inode is a fast symlink.
+ */
+static inline int ext3_inode_is_fast_symlink(struct inode *inode)
+{
+	int ea_blocks = EXT3_I(inode)->i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;
+
+	return (S_ISLNK(inode->i_mode) &&
+		inode->i_blocks - ea_blocks == 0);
+}
+
 /* The ext3 forget function must perform a revoke if we are freeing data
  * which has been journaled.  Metadata (eg. indirect blocks) must be
  * revoked in all cases.

@@ -51,7 +63,7 @@
  * still needs to be revoked.
  */
 
-static int ext3_forget(handle_t *handle, int is_metadata,
+int ext3_forget(handle_t *handle, int is_metadata,
 		       struct inode *inode, struct buffer_head *bh,
 		       int blocknr)
 {

@@ -167,9 +179,7 @@
 {
 	handle_t *handle;
 	
-	if (is_bad_inode(inode) ||
-	    inode->i_ino == EXT3_ACL_IDX_INO ||
-	    inode->i_ino == EXT3_ACL_DATA_INO)
+	if (is_bad_inode(inode))
 		goto no_delete;
 
 	lock_kernel();

@@ -1979,6 +1989,8 @@
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	    S_ISLNK(inode->i_mode)))
 		return;
+	if (ext3_inode_is_fast_symlink(inode))
+		return;
 	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
 		return;

@@ -2130,8 +2142,6 @@
 	struct ext3_group_desc * gdp;
 		
 	if ((inode->i_ino != EXT3_ROOT_INO &&
-		inode->i_ino != EXT3_ACL_IDX_INO &&
-		inode->i_ino != EXT3_ACL_DATA_INO &&
 		inode->i_ino != EXT3_JOURNAL_INO &&
 		inode->i_ino < EXT3_FIRST_INO(inode->i_sb)) ||
 		inode->i_ino > le32_to_cpu(

@@ -2263,10 +2273,7 @@
 
 	brelse (iloc.bh);
 
-	if (inode->i_ino == EXT3_ACL_IDX_INO ||
-	    inode->i_ino == EXT3_ACL_DATA_INO)
-		/* Nothing to do */ ;
-	else if (S_ISREG(inode->i_mode)) {
+	if (S_ISREG(inode->i_mode)) {
 		inode->i_op = &ext3_file_inode_operations;
 		inode->i_fop = &ext3_file_operations;
 		if (ext3_should_writeback_data(inode))

@@ -2277,18 +2284,20 @@
 		inode->i_op = &ext3_dir_inode_operations;
 		inode->i_fop = &ext3_dir_operations;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (!inode->i_blocks)
+		if (ext3_inode_is_fast_symlink(inode))
 			inode->i_op = &ext3_fast_symlink_inode_operations;
 		else {
-			inode->i_op = &page_symlink_inode_operations;
+			inode->i_op = &ext3_symlink_inode_operations;
 			if (ext3_should_writeback_data(inode))
 				inode->i_mapping->a_ops = &ext3_writeback_aops;
 			else
 				inode->i_mapping->a_ops = &ext3_aops;
 		}
-	} else 
+	} else {
+		inode->i_op = &ext3_special_inode_operations;
 		init_special_inode(inode, inode->i_mode,
 				   le32_to_cpu(iloc.raw_inode->i_block[0]));
+	}
 	if (ei->i_flags & EXT3_SYNC_FL)
 		inode->i_flags |= S_SYNC;
 	if (ei->i_flags & EXT3_APPEND_FL)

fs/ext3/namei.c +16 -6

diff -Nru a/fs/ext3/namei.c b/fs/ext3/namei.c
@@ -36,6 +36,7 @@
 #include <linux/quotaops.h>
 #include <linux/buffer_head.h>
 #include <linux/smp_lock.h>
+#include "xattr.h"
 
 
 /*

@@ -1654,7 +1655,7 @@
 	if (IS_DIRSYNC(dir))
 		handle->h_sync = 1;
 
-	inode = ext3_new_inode (handle, dir, S_IFDIR);
+	inode = ext3_new_inode (handle, dir, S_IFDIR | mode);
 	err = PTR_ERR(inode);
 	if (IS_ERR(inode))
 		goto out_stop;

@@ -1662,7 +1663,6 @@
 	inode->i_op = &ext3_dir_inode_operations;
 	inode->i_fop = &ext3_dir_operations;
 	inode->i_size = EXT3_I(inode)->i_disksize = inode->i_sb->s_blocksize;
-	inode->i_blocks = 0;	
 	dir_block = ext3_bread (handle, inode, 0, 1, &err);
 	if (!dir_block) {
 		inode->i_nlink--; /* is this nlink == 0? */

@@ -1689,9 +1689,6 @@
 	BUFFER_TRACE(dir_block, "call ext3_journal_dirty_metadata");
 	ext3_journal_dirty_metadata(handle, dir_block);
 	brelse (dir_block);
-	inode->i_mode = S_IFDIR | mode;
-	if (dir->i_mode & S_ISGID)
-		inode->i_mode |= S_ISGID;
 	ext3_mark_inode_dirty(handle, inode);
 	err = ext3_add_entry (handle, dentry, inode);
 	if (err) {

@@ -2068,7 +2065,7 @@
 		goto out_stop;
 
 	if (l > sizeof (EXT3_I(inode)->i_data)) {
-		inode->i_op = &page_symlink_inode_operations;
+		inode->i_op = &ext3_symlink_inode_operations;
 		if (ext3_should_writeback_data(inode))
 			inode->i_mapping->a_ops = &ext3_writeback_aops;
 		else

@@ -2284,4 +2281,17 @@
 	.rmdir		= ext3_rmdir,
 	.mknod		= ext3_mknod,
 	.rename		= ext3_rename,
+	.setxattr	= ext3_setxattr,	
+	.getxattr	= ext3_getxattr,	
+	.listxattr	= ext3_listxattr,	
+	.removexattr	= ext3_removexattr,
 };
+
+struct inode_operations ext3_special_inode_operations = {
+	.setxattr	= ext3_setxattr,
+	.getxattr	= ext3_getxattr,
+	.listxattr	= ext3_listxattr,
+	.removexattr	= ext3_removexattr,
+};
+
+

fs/ext3/super.c +22 -1

diff -Nru a/fs/ext3/super.c b/fs/ext3/super.c
@@ -30,6 +30,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <asm/uaccess.h>
+#include "xattr.h"
 
 #ifdef CONFIG_JBD_DEBUG
 static int ext3_ro_after; /* Make fs read-only after this many jiffies */

@@ -405,6 +406,7 @@
 	struct ext3_super_block *es = sbi->s_es;
 	int i;
 
+	ext3_xattr_put_super(sb);
 	journal_destroy(sbi->s_journal);
 	if (!(sb->s_flags & MS_RDONLY)) {
 		EXT3_CLEAR_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);

@@ -554,6 +556,7 @@
 			  int is_remount)
 {
 	unsigned long *mount_options = &sbi->s_mount_opt;
+	
 	uid_t *resuid = &sbi->s_resuid;
 	gid_t *resgid = &sbi->s_resgid;
 	char * this_char;

@@ -566,6 +569,13 @@
 			continue;
 		if ((value = strchr (this_char, '=')) != NULL)
 			*value++ = 0;
+#ifdef CONFIG_EXT3_FS_XATTR
+		if (!strcmp (this_char, "user_xattr"))
+			set_opt (*mount_options, XATTR_USER);
+		else if (!strcmp (this_char, "nouser_xattr"))
+			clear_opt (*mount_options, XATTR_USER);
+		else
+#endif
 		if (!strcmp (this_char, "bsddf"))
 			clear_opt (*mount_options, MINIX_DF);
 		else if (!strcmp (this_char, "nouid32")) {

@@ -982,6 +992,12 @@
 	sbi->s_mount_opt = 0;
 	sbi->s_resuid = EXT3_DEF_RESUID;
 	sbi->s_resgid = EXT3_DEF_RESGID;
+
+	/* Default extended attribute flags */
+#ifdef CONFIG_EXT3_FS_XATTR
+	set_opt(sbi->s_mount_opt, XATTR_USER);
+#endif
+
 	if (!parse_options ((char *) data, &sb_block, sbi, &journal_inum, 0))
 		goto out_fail;

@@ -1820,7 +1836,10 @@
 
 static int __init init_ext3_fs(void)
 {
-	int err = init_inodecache();
+	int err = init_ext3_xattr();
+	if (err)
+		return err;
+	err = init_inodecache();
 	if (err)
 		goto out1;
         err = register_filesystem(&ext3_fs_type);

@@ -1830,6 +1849,7 @@
 out:
 	destroy_inodecache();
 out1:
+ 	exit_ext3_xattr();
 	return err;
 }

@@ -1837,6 +1857,7 @@
 {
 	unregister_filesystem(&ext3_fs_type);
 	destroy_inodecache();
+	exit_ext3_xattr();
 }
 
 MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others");

fs/ext3/symlink.c +15 -1

diff -Nru a/fs/ext3/symlink.c b/fs/ext3/symlink.c
@@ -20,6 +20,7 @@
 #include <linux/fs.h>
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
+#include "xattr.h"
 
 static int ext3_readlink(struct dentry *dentry, char *buffer, int buflen)
 {

@@ -33,7 +34,20 @@
 	return vfs_follow_link(nd, (char*)ei->i_data);
 }
 
+struct inode_operations ext3_symlink_inode_operations = {
+	.readlink	= page_readlink,
+	.follow_link	= page_follow_link,
+	.setxattr	= ext3_setxattr,
+	.getxattr	= ext3_getxattr,
+	.listxattr	= ext3_listxattr,
+	.removexattr	= ext3_removexattr,
+};
+
 struct inode_operations ext3_fast_symlink_inode_operations = {
-	.readlink	= ext3_readlink,		/* BKL not held.  Don't need */
+	.readlink	= ext3_readlink,	/* BKL not held.  Don't need */
 	.follow_link	= ext3_follow_link,	/* BKL not held.  Don't need */
+	.setxattr	= ext3_setxattr,
+	.getxattr	= ext3_getxattr,
+	.listxattr	= ext3_listxattr,
+	.removexattr	= ext3_removexattr,
 };

fs/ext3/xattr.c new file +1127 -0

1,127 lines not on this page · raw message

fs/ext3/xattr.h new file +133 -0

133 lines not on this page · raw message

fs/ext3/xattr_user.c new file +99 -0

99 lines not on this page · raw message

include/linux/ext3_fs.h +5 ~~-26~~

diff -Nru a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
@@ -64,8 +64,6 @@
  */
 #define	EXT3_BAD_INO		 1	/* Bad blocks inode */
 #define EXT3_ROOT_INO		 2	/* Root inode */
-#define EXT3_ACL_IDX_INO	 3	/* ACL inode */
-#define EXT3_ACL_DATA_INO	 4	/* ACL inode */
 #define EXT3_BOOT_LOADER_INO	 5	/* Boot loader inode */
 #define EXT3_UNDEL_DIR_INO	 6	/* Undelete directory inode */
 #define EXT3_RESIZE_INO		 7	/* Reserved group descriptors inode */

@@ -95,7 +93,6 @@
 #else
 # define EXT3_BLOCK_SIZE(s)		(EXT3_MIN_BLOCK_SIZE << (s)->s_log_block_size)
 #endif
-#define EXT3_ACLE_PER_BLOCK(s)		(EXT3_BLOCK_SIZE(s) / sizeof (struct ext3_acl_entry))
 #define	EXT3_ADDR_PER_BLOCK(s)		(EXT3_BLOCK_SIZE(s) / sizeof (__u32))
 #ifdef __KERNEL__
 # define EXT3_BLOCK_SIZE_BITS(s)	((s)->s_blocksize_bits)

@@ -130,28 +127,6 @@
 #endif
 
 /*
- * ACL structures
- */
-struct ext3_acl_header	/* Header of Access Control Lists */
-{
-	__u32	aclh_size;
-	__u32	aclh_file_count;
-	__u32	aclh_acle_count;
-	__u32	aclh_first_acle;
-};
-
-struct ext3_acl_entry	/* Access Control List Entry */
-{
-	__u32	acle_size;
-	__u16	acle_perms;	/* Access permissions */
-	__u16	acle_type;	/* Type of entry */
-	__u16	acle_tag;	/* User or group identity */
-	__u16	acle_pad1;
-	__u32	acle_next;	/* Pointer on next entry for the */
-					/* same inode or on next free entry */
-};
-
-/*
  * Structure of a blocks group descriptor
  */
 struct ext3_group_desc

@@ -347,6 +322,7 @@
   #define EXT3_MOUNT_WRITEBACK_DATA	0x0C00	/* No data ordering */
 #define EXT3_MOUNT_UPDATE_JOURNAL	0x1000	/* Update the journal format */
 #define EXT3_MOUNT_NO_UID32		0x2000  /* Disable 32-bit UIDs */
+#define EXT3_MOUNT_XATTR_USER		0x4000	/* Extended user attributes */
 
 /* Compatibility, for having both ext2_fs.h and ext3_fs.h included at once */
 #ifndef _LINUX_EXT2_FS_H

@@ -529,7 +505,7 @@
 #define EXT3_FEATURE_INCOMPAT_RECOVER		0x0004 /* Needs recovery */
 #define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV	0x0008 /* Journal device */
 
-#define EXT3_FEATURE_COMPAT_SUPP	0
+#define EXT3_FEATURE_COMPAT_SUPP	EXT2_FEATURE_COMPAT_EXT_ATTR
 #define EXT3_FEATURE_INCOMPAT_SUPP	(EXT3_FEATURE_INCOMPAT_FILETYPE| \
 					 EXT3_FEATURE_INCOMPAT_RECOVER)
 #define EXT3_FEATURE_RO_COMPAT_SUPP	(EXT3_FEATURE_RO_COMPAT_SPARSE_SUPER| \

@@ -713,6 +689,7 @@
 
 
 /* inode.c */
+extern int ext3_forget(handle_t *, int, struct inode *, struct buffer_head *, int);
 extern struct buffer_head * ext3_getblk (handle_t *, struct inode *, long, int, int *);
 extern struct buffer_head * ext3_bread (handle_t *, struct inode *, int, int, int *);

@@ -781,8 +758,10 @@
 
 /* namei.c */
 extern struct inode_operations ext3_dir_inode_operations;
+extern struct inode_operations ext3_special_inode_operations;
 
 /* symlink.c */
+extern struct inode_operations ext3_symlink_inode_operations;
 extern struct inode_operations ext3_fast_symlink_inode_operations;

include/linux/ext3_jbd.h +7 -1

diff -Nru a/include/linux/ext3_jbd.h b/include/linux/ext3_jbd.h
@@ -30,13 +30,19 @@
 
 #define EXT3_SINGLEDATA_TRANS_BLOCKS	8
 
+/* Extended attributes may touch two data buffers, two bitmap buffers,
+ * and two group and summaries. */
+
+#define EXT3_XATTR_TRANS_BLOCKS		8
+
 /* Define the minimum size for a transaction which modifies data.  This
  * needs to take into account the fact that we may end up modifying two
  * quota files too (one for the group, one for the user quota).  The
  * superblock only gets updated once, of course, so don't bother
  * counting that again for the quota updates. */
 
-#define EXT3_DATA_TRANS_BLOCKS		(3 * EXT3_SINGLEDATA_TRANS_BLOCKS - 2)
+#define EXT3_DATA_TRANS_BLOCKS		(3 * EXT3_SINGLEDATA_TRANS_BLOCKS + \
+					 EXT3_XATTR_TRANS_BLOCKS - 2)
 
 extern int ext3_writepage_trans_blocks(struct inode *inode);

Henning P. Schmiedehausen

2002-10-16 09:32 UTC #1 lore raw

fs/ext3/inode.c line 51

Quoted 2 lines · [email protected] writes:

fs/ext3/inode.c +2 -0

+	int ea_blocks = EXT3_I(inode)->i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;

Sometimes I wonder if we shouldn't have the block size (512) and the bit shift (9) as defines somewhere and gradually shift away from hard coded values...

If we ever decide to change the block size of ext2/ext3, we're in for a "looking for nines"... :-)

Regards
Henning

Signature · 5 lines

Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     [email protected]

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   [email protected]
D-91054 Buckenhof     Fax.: 09131 / 50654-20

Theodore Ts'o

author

2002-10-16 16:10 UTC #2 lore raw

Quoted 11 lines · Henning P. Schmiedehausen

fs/ext3/inode.c line 51
Quoted 2 lines · [email protected] writes:
fs/ext3/inode.c +2 -0
+	int ea_blocks = EXT3_I(inode)->i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;
Sometimes I wonder if we shouldn't have the block size (512) and the bit shift (9) as defines somewhere and gradually shift away from hard coded values...
If we ever decide to change the block size of ext2/ext3, we're in for a "looking for nines"... :-)

We already have different block sizes for ext2/3; we support 1k, 2k, and 4k block sizes. The bit shift is only there because the superblock stores the blocksize shifted by 9 for "historical reasons". (i.e., I probably wouldn't have bothered with it, but Remy Card did it that way, and it doesn't hurt, so make everyone suffer with an incompatible format change?)

We do have one place where the 512 byte sector count is used, and that's in inode->i_blocks, which is stored as 512 byte sectors, regardless of the blocksize. *That's* due to st_blocks in the stat structure being returned in 512 byte sectors, and for no other good reason. As a result of this particular bit of history, ext2 filesystems are limited to 2TB, even when using a 4k blocksize. Without this bit of "design history", we'd be able to support 16TB filesystems with 2.6's CONFIG_LBD support, without needing to going to a 64-bit block numbers. Making this change is actually pretty easy, and I may try to get that change to Linus before 2.6 closes.

- Ted

Andreas Dilger

2002-10-16 17:02 UTC #3 lore raw

Quoted 10 lines · Theodore Ts'o

We do have one place where the 512 byte sector count is used, and that's in inode->i_blocks, which is stored as 512 byte sectors, regardless of the blocksize. *That's* due to st_blocks in the stat structure being returned in 512 byte sectors, and for no other good reason. As a result of this particular bit of history, ext2 filesystems are limited to 2TB, even when using a 4k blocksize. Without this bit of "design history", we'd be able to support 16TB filesystems with 2.6's CONFIG_LBD support, without needing to going to a 64-bit block numbers. Making this change is actually pretty easy, and I may try to get that change to Linus before 2.6 closes.

Err, wouldn't that be a 2TB FILE limit, and not a FILESYSTEM limit?

Also, this limit only applies for allocated space within the file and not the total size of the file (for sparse files). However, I don't thing we have any checking in the kernel to ensure we don't overflow 2^32 allocated blocks for a single file (probably needs to be done in ext*_alloc_block() or similar to ensure we have less than or equal to "2^32 - (sb->blocksize >> 9)" blocks allocated to a file. I don't know if we still have the old "limit total file size to 2TB" check in there still.

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

Theodore Ts'o

author

2002-10-16 19:24 UTC #4 lore raw

Quoted 1 line · Andreas Dilger

Err, wouldn't that be a 2TB FILE limit, and not a FILESYSTEM limit?

Yes, sorry about that.

I just took a quick look at this, and the limit is unfortunately fundamental to the VFS; the st_blocks in struct inode and struct kstat is an unsigned long, which means that we're stuck with the 2TB limit for *all* filesystems, even using the LFS API. (Or rather, as you point out, since no one apparently is doing any overflow checking, so st_blocks will just be incorrect, even to callers using the stat64 call.)

A quick fix would be to make st_blocks in struct inode and struct kstat take a __u64 instead of a unsigned long, but it will have a performance impact. What do folks think?

- Ted

Jan Kara

reviewer

2002-10-19 18:30 UTC #5 lore raw

Quoted 13 lines · Andreas Dilger

Quoted 1 line
Err, wouldn't that be a 2TB FILE limit, and not a FILESYSTEM limit?
Yes, sorry about that.
I just took a quick look at this, and the limit is unfortunately fundamental to the VFS; the st_blocks in struct inode and struct kstat is an unsigned long, which means that we're stuck with the 2TB limit for *all* filesystems, even using the LFS API. (Or rather, as you point out, since no one apparently is doing any overflow checking, so st_blocks will just be incorrect, even to callers using the stat64 call.)

Quoted 3 lines

A quick fix would be to make st_blocks in struct inode and struct kstat take a __u64 instead of a unsigned long, but it will have a performance impact. What do folks think?

I'd vote for it. And also for storing used space in bytes - then you can remove i_bytes field quota is using from inode and inode_set_bytes, inode_add_bytes and such inline functions from fs.h ...

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SuSE CR Labs

Lorenzo Allegrucci

2002-10-16 17:47 UTC #6 lore raw

In reply to Theodore Ts'o

Quoted 9 lines · Theodore Ts'o

Quoted 8 lines · Henning P. Schmiedehausen
fs/ext3/inode.c line 51
Quoted 2 lines · [email protected] writes:
fs/ext3/inode.c +2 -0
+	int ea_blocks = EXT3_I(inode)->i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;
Sometimes I wonder if we shouldn't have the block size (512) and the bit shift (9) as defines somewhere and gradually shift away from hard coded values...

Quoted 5 lines

Quoted 2 lines
If we ever decide to change the block size of ext2/ext3, we're in for a "looking for nines"... :-)
We already have different block sizes for ext2/3; we support 1k, 2k, and 4k block sizes.

BTW, why doesn't ext2/3 support 512 byte block sizes?

Andreas Dilger

2002-10-16 18:29 UTC #7 lore raw

Quoted 5 lines · Lorenzo Allegrucci

Quoted 2 lines · Theodore Ts'o
We already have different block sizes for ext2/3; we support 1k, 2k, and 4k block sizes.
BTW, why doesn't ext2/3 support 512 byte block sizes?

Too inefficient, and impose too many limitations on the filesystem (which sets up some filesystem structures based on the blocksize). Already, 1kB block size is too small for many things, and 4kB is preferred. It would probably support larger blocksizes already on ia32 if the page size was larger.

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Andreas Gruenbacher

2002-10-16 20:26 UTC #10 lore raw

PATCH 1/1 linux-2.5.43-mm1: Further xattr/acl cleanups

This is against
<http://www.zipworld.com.au/~akpm/linux/patches/2.5/2.5.43/2.5.43-mm1/>.
Please look inside for comments. Seems to work pretty well; it has already passed my basic test suite.

--Andreas.

Changes:

* Minimize the calls to mark_inode_dirty() and ext3_mark_inode_dirty() by removing extra calls from ext[23]_init_acl()
* ext3_acl_chmod() cleanup: Don't pass in transaction handle anymore
* Remove 2.4 locking leftover in fs/ext3/inode.c:ext3_setattr()
* Remove the [gs]et_posix_acl inode operations

fs/ext2/acl.c +3 ~~-18~~

diff -Nur linux-2.5.43-mm1/fs/ext2/acl.c linux-2.5.43-mm1+/fs/ext2/acl.c
@@ -124,13 +124,7 @@
 	return ERR_PTR(-EINVAL);
 }
 
-/*
- * Inode operation get_posix_acl().
- *
- * inode->i_sem: down
- * BKL held [before 2.5.x]
- */
-struct posix_acl *
+static struct posix_acl *
 ext2_get_acl(struct inode *inode, int type)
 {
 	int name_index;

@@ -177,13 +171,7 @@
 	return acl;
 }
 
-/*
- * Inode operation set_posix_acl().
- *
- * inode->i_sem: down
- * BKL held [before 2.5.x]
- */
-int
+static int
 ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
 {
 	int name_index;

@@ -348,10 +336,8 @@
 			if (IS_ERR(acl))
 				return PTR_ERR(acl);
 		}
-		if (!acl) {
+		if (!acl)
 			inode->i_mode &= ~current->fs->umask;
-			mark_inode_dirty(inode);
-		}
 	}
 	if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
                struct posix_acl *clone;

@@ -370,7 +356,6 @@
 		error = posix_acl_create_masq(clone, &mode);
 		if (error >= 0) {
 			inode->i_mode = mode;
-			mark_inode_dirty(inode);
 			if (error > 0) {
 				/* This is an extended ACL */
 				error = ext2_set_acl(inode,

fs/ext2/acl.h +0 -4

diff -Nur linux-2.5.43-mm1/fs/ext2/acl.h linux-2.5.43-mm1+/fs/ext2/acl.h
@@ -62,8 +62,6 @@
 /* acl.c */
 extern int ext2_permission (struct inode *, int);
 extern int ext2_permission_locked (struct inode *, int);
-extern struct posix_acl *ext2_get_acl (struct inode *, int);
-extern int ext2_set_acl (struct inode *, int, struct posix_acl *);
 extern int ext2_acl_chmod (struct inode *);
 extern int ext2_init_acl (struct inode *, struct inode *);

@@ -73,8 +71,6 @@
 #else
 #include <linux/sched.h>
 #define ext2_permission NULL
-#define ext2_get_acl	NULL
-#define ext2_set_acl	NULL
 
 static inline int
 ext2_acl_chmod (struct inode *inode)

fs/ext2/file.c +0 -2

diff -Nur linux-2.5.43-mm1/fs/ext2/file.c linux-2.5.43-mm1+/fs/ext2/file.c
@@ -63,6 +63,4 @@
 	.removexattr	= ext2_removexattr,
 	.setattr	= ext2_setattr,
 	.permission	= ext2_permission,
-	.get_posix_acl	= ext2_get_acl,
-	.set_posix_acl	= ext2_set_acl,
 };

fs/ext2/namei.c +0 -4

diff -Nur linux-2.5.43-mm1/fs/ext2/namei.c linux-2.5.43-mm1+/fs/ext2/namei.c
@@ -379,8 +379,6 @@
 	.removexattr	= ext2_removexattr,
 	.setattr	= ext2_setattr,
 	.permission	= ext2_permission,
-	.get_posix_acl	= ext2_get_acl,
-	.set_posix_acl	= ext2_set_acl,
 };
 
 struct inode_operations ext2_special_inode_operations = {

@@ -390,6 +388,4 @@
 	.removexattr	= ext2_removexattr,
 	.setattr	= ext2_setattr,
 	.permission	= ext2_permission,
-	.get_posix_acl	= ext2_get_acl,
-	.set_posix_acl	= ext2_set_acl,
 };

fs/ext3/acl.c +20 ~~-43~~

120 lines not on this page · raw message

fs/ext3/acl.h +2 -8

diff -Nur linux-2.5.43-mm1/fs/ext3/acl.h linux-2.5.43-mm1+/fs/ext3/acl.h
@@ -62,12 +62,8 @@
 /* acl.c */
 extern int ext3_permission (struct inode *, int);
 extern int ext3_permission_locked (struct inode *, int);
-extern struct posix_acl *ext3_get_acl (struct inode *, int);
-extern int ext3_set_acl (struct inode *, int, struct posix_acl *);
-extern int ext3_acl_chmod (handle_t *, struct inode *);
+extern int ext3_acl_chmod (struct inode *);
 extern int ext3_init_acl (handle_t *, struct inode *, struct inode *);
-extern int ext3_get_acl_xattr (struct inode *, int, void *, size_t);
-extern int ext3_set_acl_xattr (struct inode *, int, void *, size_t);
 
 extern int init_ext3_acl(void);
 extern void exit_ext3_acl(void);

@@ -75,11 +71,9 @@
 #else  /* CONFIG_EXT3_FS_POSIX_ACL */
 #include <linux/sched.h>
 #define ext3_permission NULL
-#define ext3_get_acl	NULL
-#define ext3_set_acl	NULL
 
 static inline int
-ext3_acl_chmod(handle_t *handle, struct inode *inode)
+ext3_acl_chmod(struct inode *inode)
 {
 	return 0;
 }

fs/ext3/file.c +0 -2

diff -Nur linux-2.5.43-mm1/fs/ext3/file.c linux-2.5.43-mm1+/fs/ext3/file.c
@@ -104,7 +104,5 @@
 	.listxattr	= ext3_listxattr,
 	.removexattr	= ext3_removexattr,
 	.permission	= ext3_permission,
-	.get_posix_acl	= ext3_get_acl,
-	.set_posix_acl	= ext3_set_acl,
 };

fs/ext3/inode.c +2 ~~-17~~

diff -Nur linux-2.5.43-mm1/fs/ext3/inode.c linux-2.5.43-mm1+/fs/ext3/inode.c
@@ -2559,23 +2559,8 @@
 	if (S_ISREG(inode->i_mode) && inode->i_nlink)
 		ext3_orphan_del(NULL, inode);
 
-#ifdef CONFIG_EXT3_FS_POSIX_ACL
-	if (!rc && test_opt(inode->i_sb, POSIX_ACL) && (ia_valid & ATTR_MODE)) {
-		handle_t *handle;
-
-		handle = ext3_journal_start(inode, EXT3_XATTR_TRANS_BLOCKS);
-		if (IS_ERR(handle)) {
-			error = PTR_ERR(handle);
-			goto err_out;
-		}
-		if (!(ia_valid & ATTR_SIZE))
-			down(&inode->i_sem);
-		rc = ext3_acl_chmod(handle, inode);
-		if (!(ia_valid & ATTR_SIZE))
-			up(&inode->i_sem);
-		ext3_journal_stop(handle, inode);
-	}
-#endif
+	if (!rc && (ia_valid & ATTR_MODE))
+		rc = ext3_acl_chmod(inode);
 
 err_out:
 	ext3_std_error(inode->i_sb, error);

fs/ext3/namei.c +0 -4

diff -Nur linux-2.5.43-mm1/fs/ext3/namei.c linux-2.5.43-mm1+/fs/ext3/namei.c
@@ -2290,8 +2290,6 @@
 	.listxattr	= ext3_listxattr,	
 	.removexattr	= ext3_removexattr,
 	.permission	= ext3_permission,
-	.get_posix_acl	= ext3_get_acl,
-	.set_posix_acl	= ext3_set_acl,
 };
 
 struct inode_operations ext3_special_inode_operations = {

@@ -2301,8 +2299,6 @@
 	.listxattr	= ext3_listxattr,
 	.removexattr	= ext3_removexattr,
 	.permission	= ext3_permission,
-	.get_posix_acl	= ext3_get_acl,
-	.set_posix_acl	= ext3_set_acl,
 };

fs/posix_acl.c +0 ~~-34~~

diff -Nur linux-2.5.43-mm1/fs/posix_acl.c linux-2.5.43-mm1+/fs/posix_acl.c
@@ -410,37 +410,3 @@
 
 	return 0;
 }
-
-/*
- * Get the POSIX ACL of an inode.
- */
-struct posix_acl *
-get_posix_acl(struct inode *inode, int type)
-{
-	struct posix_acl *acl;
-
-	if (!inode->i_op->get_posix_acl)
-		return ERR_PTR(-EOPNOTSUPP);
-	down(&inode->i_sem);
-	acl = inode->i_op->get_posix_acl(inode, type);
-	up(&inode->i_sem);
-
-	return acl;
-}
-
-/*
- * Set the POSIX ACL of an inode.
- */
-int
-set_posix_acl(struct inode *inode, int type, struct posix_acl *acl)
-{
-	int error;
-
-	if (!inode->i_op->set_posix_acl)
-		return -EOPNOTSUPP;
-	down(&inode->i_sem);
-	error = inode->i_op->set_posix_acl(inode, type, acl);
-	up(&inode->i_sem);
-
-	return error;
-}

include/linux/fs.h +0 -2

diff -Nur linux-2.5.43-mm1/include/linux/fs.h linux-2.5.43-mm1+/include/linux/fs.h
@@ -792,8 +792,6 @@
 	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
 	int (*removexattr) (struct dentry *, const char *);
-	struct posix_acl *(*get_posix_acl) (struct inode *, int);
-	int (*set_posix_acl) (struct inode *, int, struct posix_acl *);
 };
 
 struct seq_file;

include/linux/posix_acl.h +0 -3

diff -Nur linux-2.5.43-mm1/include/linux/posix_acl.h linux-2.5.43-mm1+/include/linux/posix_acl.h
@@ -81,7 +81,4 @@
 extern int posix_acl_chmod_masq(struct posix_acl *, mode_t);
 extern int posix_acl_masq_nfs_mode(struct posix_acl *, mode_t *);
 
-extern struct posix_acl *get_posix_acl(struct inode *, int);
-extern int set_posix_acl(struct inode *, int, struct posix_acl *);
-
 #endif  /* __LINUX_POSIX_ACL_H */

Andreas Gruenbacher

2002-10-16 21:09 UTC #11 lore raw

Re: [PATCH] linux-2.5.43-mm1: Further xattr/acl cleanups

This should do for now as a replacement for nfsd_get_posix_acl(), the only caller of ->[gs]et_posix_acl(). Please add along with the other NFS patches. (Did Ted port them already?)

--Andreas.

This version of nfsd_get_posix_acl uses getxattr instead of the discarded get_posix_acl inode operation.

fs/nfsd/vfs.c +27 -0

diff -Nur linux-2.5.43-mm1+/fs/nfsd/vfs.c linux-2.5.43-mm1++/fs/nfsd/vfs.c
@@ -42,6 +42,7 @@
 #endif /* CONFIG_NFSD_V3 */
 #include <linux/nfsd/nfsfh.h>
 #include <linux/quotaops.h>
+#include <linux/xattr_acl.h>
 
 #include <asm/uaccess.h>

@@ -1609,3 +1610,29 @@
 	nfsdstats.ra_size = cache_size;
 	return 0;
 }
+
+#ifdef CONFIG_FS_POSIX_ACL
+struct posix_acl *
+nfsd_get_posix_acl(struct svc_fh *fhp, int type)
+{
+	struct inode *inode = fhp->fh_dentry->d_inode;
+	struct posix_acl *acl = NULL;
+
+	if (IS_POSIXACL(inode)) {
+		int err, size = xattr_acl_size(64);  /* reasonable max.size */
+		void *buffer = kmalloc(size, GFP_KERNEL);
+		if (!buffer)
+			return ERR_PTR(-ENOMEM);
+		if (!fhp->fh_locked)
+			fh_lock(fhp);  /* automatic unlocking */
+		err = inode->i_op->getxattr(fhp->fh_dentry,
+					    XATTR_NAME_ACL_ACCESS,
+					    buffer, size);
+		acl = ERR_PTR(err);
+		if (err >= 0)
+			acl = posix_acl_from_xattr(buffer, err);
+		kfree(buffer);
+	}
+	return acl;
+}
+#endif