[Cluster-devel] [PATCH 00/18] Consolidate Posix ACL implementation

[Cluster-devel] [PATCH 00/18] Consolidate Posix ACL implementation

[Christoph Hellwig]

This series consolidates the various cut'n'pasted Posix ACL implementations
into a single common one based on the ->get_acl method Linus added a while
ago and a new ->set_acl counterpart.

This 1600 lines of code and provides a single place to implement various
nasty little gems of the semantics.

Unfortunately the 9p code is still left out - it implements the ACLs
in two very weird ways, one using the common code but on the client only,
and one pasing things straight through to the server.  We could easily
convert it to the new code on the write side if ->set_acl took a dentry,
but there's no cance to do that on the ->get_acl side.  Ideas how to
handle it welcome.

After that we'd be ready to never go into the fs for the ACL attributes
and branch straight to the ACL code below the syscall, repairing the
old API braindamage of overloading ACLs onto the xattrs.

Btw, I'd be almost tempted to do that for all system.* attrs.  Besides
Posix ACLs we only have CIFS and NFSv4 ACL variants, weird advice crap
in f2fs, and the magic mushroom proto name on sockets.

[Cluster-devel] [PATCH 01/18] reiserfs: prefix ACL symbols with reiserfs_

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0001-reiserfs-prefix-ACL-symbols-with-reiserfs_.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/89c7c699/attachment.ksh>

[Cluster-devel] [PATCH 02/18] fs: add get_acl helper

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0002-fs-add-get_acl-helper.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/d171e536/attachment.ksh>

[Cluster-devel] [PATCH 03/18] fs: add a set_acl inode operation

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0003-fs-add-a-set_acl-inode-operation.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/60cda651/attachment.ksh>

[Cluster-devel] [PATCH 04/18] fs: add generic xattr_acl handlers

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0004-fs-add-generic-xattr_acl-handlers.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/ae723c7a/attachment.ksh>

[Cluster-devel] [PATCH 05/18] fs: make posix_acl_chmod more useful

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0005-fs-make-posix_acl_chmod-more-useful.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/c9a025d8/attachment.ksh>

[Cluster-devel] [PATCH 06/18] fs: make posix_acl_create more useful

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0006-fs-make-posix_acl_create-more-useful.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/3aac7557/attachment.ksh>

[Cluster-devel] [PATCH 07/18] btrfs: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0007-btrfs-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/bb74b90e/attachment.ksh>

[Cluster-devel] [PATCH 08/18] ext2/3/4: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0008-ext2-3-4-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/dcf30da7/attachment.ksh>

[Cluster-devel] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0009-f2fs-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/a3947f13/attachment.ksh>

[Cluster-devel] [PATCH 10/18] hfsplus: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0010-hfsplus-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/467729fa/attachment.ksh>

[Cluster-devel] [PATCH 11/18] jffs2: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0011-jffs2-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/04b42cf8/attachment.ksh>

[Cluster-devel] [PATCH 12/18] ocfs2: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0012-ocfs2-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/0d2f3c01/attachment.ksh>

[Cluster-devel] [PATCH 13/18] reiserfs: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0013-reiserfs-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/88e41f2d/attachment.ksh>

[Cluster-devel] [PATCH 14/18] xfs: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0014-xfs-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/8894a6cb/attachment.ksh>

[Cluster-devel] [PATCH 15/18] jfs: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0015-jfs-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/85a106b2/attachment.ksh>

[Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0016-gfs2-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/19e00dc0/attachment.ksh>

[Cluster-devel] [PATCH 17/18] nfs: use generic posix ACL infrastructure for v3 Posix ACLs

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0017-nfs-use-generic-posix-ACL-infrastructure-for-v3-Posi.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/b8782175/attachment.ksh>

[Cluster-devel] [PATCH 18/18] fs: remove generic_acl

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0018-fs-remove-generic_acl.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131201/df191d5d/attachment.ksh>

[Cluster-devel] [PATCH 10/18] hfsplus: use generic posix ACL infrastructure

[Vyacheslav Dubeyko]

On Dec 1, 2013, at 2:59 PM, Christoph Hellwig wrote:

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/hfsplus/acl.h       |    9 +--
> fs/hfsplus/dir.c       |    1 +
> fs/hfsplus/inode.c     |    3 +-
> fs/hfsplus/posix_acl.c |  161 +++++-------------------------------------------
> fs/hfsplus/xattr.c     |    5 +-
> fs/hfsplus/xattr.h     |    2 -
> 6 files changed, 24 insertions(+), 157 deletions(-)
> 

Reviewed-by: Vyacheslav Dubeyko <slava@dubeyko.com>

The patch looks correct. I think only that it makes sense to add debug messages
in hfsplus_get_posix_acl() and hfsplus_set_posix_acl(). Previously, debug
messages were in hfsplus_xattr_get_posix_acl() and hfsplus_xattr_set_posix_acl()
but the patch removes these methods.

Thanks,
Vyacheslav Dubeyko.

[Cluster-devel] [PATCH 02/18] fs: add get_acl helper

[Jan Kara]

On Sun 01-12-13 03:59:05, Christoph Hellwig wrote:
> Factor out the code to get an ACL either from the inode or disk from
> check_acl, so that it can be used elsewhere later on.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/namei.c                |   24 +++---------------------
>  fs/posix_acl.c            |   23 +++++++++++++++++++++++
>  include/linux/posix_acl.h |    2 ++
>  3 files changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index c53d3a9..8acd1e8 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -235,27 +235,9 @@ static int check_acl(struct inode *inode, int mask)
>  	        return posix_acl_permission(inode, acl, mask & ~MAY_NOT_BLOCK);
>  	}
>  
> -	acl = get_cached_acl(inode, ACL_TYPE_ACCESS);
> -
> -	/*
> -	 * A filesystem can force a ACL callback by just never filling the
> -	 * ACL cache. But normally you'd fill the cache either at inode
> -	 * instantiation time, or on the first ->get_acl call.
> -	 *
> -	 * If the filesystem doesn't have a get_acl() function at all, we'll
> -	 * just create the negative cache entry.
> -	 */
> -	if (acl == ACL_NOT_CACHED) {
> -	        if (inode->i_op->get_acl) {
> -			acl = inode->i_op->get_acl(inode, ACL_TYPE_ACCESS);
> -			if (IS_ERR(acl))
> -				return PTR_ERR(acl);
> -		} else {
> -		        set_cached_acl(inode, ACL_TYPE_ACCESS, NULL);
> -		        return -EAGAIN;
> -		}
> -	}
> -
> +	acl = get_acl(inode, ACL_TYPE_ACCESS);
> +	if (IS_ERR(acl))
> +		return PTR_ERR(acl);
>  	if (acl) {
>  	        int error = posix_acl_permission(inode, acl, mask);
>  	        posix_acl_release(acl);
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 8bd2135..9dd03e0 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -418,3 +418,26 @@ posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode)
>  	return err;
>  }
>  EXPORT_SYMBOL(posix_acl_chmod);
> +
> +struct posix_acl *get_acl(struct inode *inode, int type)
> +{
> +	struct posix_acl *acl;
> +
> +	acl = get_cached_acl(inode, type);
> +	if (acl != ACL_NOT_CACHED)
> +		return acl;
> +
> +	/*
> +	 * A filesystem can force a ACL callback by just never filling the
> +	 * ACL cache. But normally you'd fill the cache either at inode
> +	 * instantiation time, or on the first ->get_acl call.
> +	 *
> +	 * If the filesystem doesn't have a get_acl() function at all, we'll
> +	 * just create the negative cache entry.
> +	 */
> +        if (!inode->i_op->get_acl) {
> +	        set_cached_acl(inode, type, NULL);
> +	        return ERR_PTR(-EAGAIN);
> +	}
> +	return inode->i_op->get_acl(inode, type);
> +}
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 7931efe..a8d9918 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -175,4 +175,6 @@ static inline void cache_no_acl(struct inode *inode)
>  #endif
>  }
>  
> +struct posix_acl *get_acl(struct inode *inode, int type);
> +
>  #endif  /* __LINUX_POSIX_ACL_H */
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 01/18] reiserfs: prefix ACL symbols with reiserfs_

[Jan Kara]

On Sun 01-12-13 03:59:04, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/reiserfs/xattr_acl.c |   20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
> index 06c04f7..6f721ea 100644
> --- a/fs/reiserfs/xattr_acl.c
> +++ b/fs/reiserfs/xattr_acl.c
> @@ -16,7 +16,7 @@ static int reiserfs_set_acl(struct reiserfs_transaction_handle *th,
>  			    struct posix_acl *acl);
>  
>  static int
> -posix_acl_set(struct dentry *dentry, const char *name, const void *value,
> +reiserfs_posix_acl_set(struct dentry *dentry, const char *name, const void *value,
>  		size_t size, int flags, int type)
>  {
>  	struct inode *inode = dentry->d_inode;
> @@ -65,7 +65,7 @@ posix_acl_set(struct dentry *dentry, const char *name, const void *value,
>  }
>  
>  static int
> -posix_acl_get(struct dentry *dentry, const char *name, void *buffer,
> +reiserfs_posix_acl_get(struct dentry *dentry, const char *name, void *buffer,
>  		size_t size, int type)
>  {
>  	struct posix_acl *acl;
> @@ -88,7 +88,7 @@ posix_acl_get(struct dentry *dentry, const char *name, void *buffer,
>  /*
>   * Convert from filesystem to in-memory representation.
>   */
> -static struct posix_acl *posix_acl_from_disk(const void *value, size_t size)
> +static struct posix_acl *reiserfs_posix_acl_from_disk(const void *value, size_t size)
>  {
>  	const char *end = (char *)value + size;
>  	int n, count;
> @@ -158,7 +158,7 @@ static struct posix_acl *posix_acl_from_disk(const void *value, size_t size)
>  /*
>   * Convert from in-memory to filesystem representation.
>   */
> -static void *posix_acl_to_disk(const struct posix_acl *acl, size_t * size)
> +static void *reiserfs_posix_acl_to_disk(const struct posix_acl *acl, size_t * size)
>  {
>  	reiserfs_acl_header *ext_acl;
>  	char *e;
> @@ -257,7 +257,7 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type)
>  	} else if (retval < 0) {
>  		acl = ERR_PTR(retval);
>  	} else {
> -		acl = posix_acl_from_disk(value, retval);
> +		acl = reiserfs_posix_acl_from_disk(value, retval);
>  	}
>  	if (!IS_ERR(acl))
>  		set_cached_acl(inode, type, acl);
> @@ -307,7 +307,7 @@ reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode,
>  	}
>  
>  	if (acl) {
> -		value = posix_acl_to_disk(acl, &size);
> +		value = reiserfs_posix_acl_to_disk(acl, &size);
>  		if (IS_ERR(value))
>  			return (int)PTR_ERR(value);
>  	}
> @@ -499,8 +499,8 @@ static size_t posix_acl_access_list(struct dentry *dentry, char *list,
>  const struct xattr_handler reiserfs_posix_acl_access_handler = {
>  	.prefix = POSIX_ACL_XATTR_ACCESS,
>  	.flags = ACL_TYPE_ACCESS,
> -	.get = posix_acl_get,
> -	.set = posix_acl_set,
> +	.get = reiserfs_posix_acl_get,
> +	.set = reiserfs_posix_acl_set,
>  	.list = posix_acl_access_list,
>  };
>  
> @@ -519,7 +519,7 @@ static size_t posix_acl_default_list(struct dentry *dentry, char *list,
>  const struct xattr_handler reiserfs_posix_acl_default_handler = {
>  	.prefix = POSIX_ACL_XATTR_DEFAULT,
>  	.flags = ACL_TYPE_DEFAULT,
> -	.get = posix_acl_get,
> -	.set = posix_acl_set,
> +	.get = reiserfs_posix_acl_get,
> +	.set = reiserfs_posix_acl_set,
>  	.list = posix_acl_default_list,
>  };
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 03/18] fs: add a set_acl inode operation

[Jan Kara]

On Sun 01-12-13 03:59:06, Christoph Hellwig wrote:
> This will allow moving all the Posix ACL handling into the VFS and clean
> up tons of cruft in the filesystems.
  Looks good. Btw I'd merge this with the following patch... Anyway:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/fs.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 121f11f..09f553c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1580,6 +1580,7 @@ struct inode_operations {
>  			   struct file *, unsigned open_flag,
>  			   umode_t create_mode, int *opened);
>  	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
> +	int (*set_acl)(struct inode *, struct posix_acl *, int);
>  } ____cacheline_aligned;
>  
>  ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 04/18] fs: add generic xattr_acl handlers

[Jan Kara]

On Sun 01-12-13 03:59:07, Christoph Hellwig wrote:
> With the ->set_acl inode operation we can implement the Posix ACL
> xattr handlers in generic code instead of duplicating them all
> over the tree.
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

							Honza

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xattr_acl.c                  |   95 +++++++++++++++++++++++++++++++++++++++
>  include/linux/posix_acl_xattr.h |    3 ++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/fs/xattr_acl.c b/fs/xattr_acl.c
> index 9fbea87..932ec76 100644
> --- a/fs/xattr_acl.c
> +++ b/fs/xattr_acl.c
> @@ -10,6 +10,7 @@
>  #include <linux/posix_acl_xattr.h>
>  #include <linux/gfp.h>
>  #include <linux/user_namespace.h>
> +#include <linux/xattr.h>
>  
>  /*
>   * Fix up the uids and gids in posix acl extended attributes in place.
> @@ -178,3 +179,97 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
>  	return real_size;
>  }
>  EXPORT_SYMBOL (posix_acl_to_xattr);
> +
> +static int
> +posix_acl_xattr_get(struct dentry *dentry, const char *name,
> +		void *value, size_t size, int type)
> +{
> +	struct posix_acl *acl;
> +	int error;
> +
> +	if (!IS_POSIXACL(dentry->d_inode))
> +		return -EOPNOTSUPP;
> +
> +	acl = get_acl(dentry->d_inode, type);
> +	if (IS_ERR(acl))
> +		return PTR_ERR(acl);
> +	if (acl == NULL)
> +		return -ENODATA;
> +
> +	error = posix_acl_to_xattr(&init_user_ns, acl, value, size);
> +	posix_acl_release(acl);
> +
> +	return error;
> +}
> +
> +static int
> +posix_acl_xattr_set(struct dentry *dentry, const char *name,
> +		const void *value, size_t size, int flags, int type)
> +{
> +	struct inode *inode = dentry->d_inode;
> +	struct posix_acl *acl = NULL;
> +	int ret;
> +
> +	if (type == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode))
> +		return value ? -EACCES : 0;
> +	if (!inode_owner_or_capable(inode))
> +		return -EPERM;
> +	if (!IS_POSIXACL(inode))
> +		return -EOPNOTSUPP;
> +
> +	if (value) {
> +		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> +		if (IS_ERR(acl))
> +			return PTR_ERR(acl);
> +
> +		if (acl) {
> +			ret = posix_acl_valid(acl);
> +			if (ret)
> +				goto out;
> +		}
> +	}
> +
> +	ret = inode->i_op->set_acl(inode, acl, type);
> +out:
> +	posix_acl_release(acl);
> +	return ret;
> +}
> +
> +static size_t
> +posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size,
> +		const char *name, size_t name_len, int type)
> +{
> +	const char *xname;
> +	size_t size;
> +
> +	if (!IS_POSIXACL(dentry->d_inode))
> +		return -EOPNOTSUPP;
> +
> +	if (type == ACL_TYPE_ACCESS)
> +		xname = POSIX_ACL_XATTR_ACCESS;
> +	else
> +		xname = POSIX_ACL_XATTR_DEFAULT;
> +
> +	size = strlen(xname) + 1;
> +	if (list && size <= list_size)
> +		memcpy(list, xname, size);
> +	return size;
> +}
> +
> +const struct xattr_handler posix_acl_access_xattr_handler = {
> +	.prefix = POSIX_ACL_XATTR_ACCESS,
> +	.flags = ACL_TYPE_ACCESS,
> +	.list = posix_acl_xattr_list,
> +	.get = posix_acl_xattr_get,
> +	.set = posix_acl_xattr_set,
> +};
> +EXPORT_SYMBOL_GPL(posix_acl_access_xattr_handler);
> +
> +const struct xattr_handler posix_acl_default_xattr_handler = {
> +	.prefix = POSIX_ACL_XATTR_DEFAULT,
> +	.flags = ACL_TYPE_DEFAULT,
> +	.list = posix_acl_xattr_list,
> +	.get = posix_acl_xattr_get,
> +	.set = posix_acl_xattr_set,
> +};
> +EXPORT_SYMBOL_GPL(posix_acl_default_xattr_handler);
> diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
> index ad93ad0..6f14ee2 100644
> --- a/include/linux/posix_acl_xattr.h
> +++ b/include/linux/posix_acl_xattr.h
> @@ -69,4 +69,7 @@ struct posix_acl *posix_acl_from_xattr(struct user_namespace *user_ns,
>  int posix_acl_to_xattr(struct user_namespace *user_ns,
>  		       const struct posix_acl *acl, void *buffer, size_t size);
>  
> +extern const struct xattr_handler posix_acl_access_xattr_handler;
> +extern const struct xattr_handler posix_acl_default_xattr_handler;
> +
>  #endif	/* _POSIX_ACL_XATTR_H */
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 05/18] fs: make posix_acl_chmod more useful

[Jan Kara]

On Sun 01-12-13 03:59:08, Christoph Hellwig wrote:
> Rename the current posix_acl_chmod to __posix_acl_chmod and add
> a fully featured ACL chmod helper that uses the ->set_acl inode
> operation.
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

							Honza
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/9p/acl.c               |    2 +-
>  fs/btrfs/acl.c            |    2 +-
>  fs/ext2/acl.c             |    2 +-
>  fs/ext3/acl.c             |    2 +-
>  fs/ext4/acl.c             |    2 +-
>  fs/f2fs/acl.c             |    2 +-
>  fs/generic_acl.c          |    2 +-
>  fs/gfs2/acl.c             |    2 +-
>  fs/hfsplus/posix_acl.c    |    2 +-
>  fs/jffs2/acl.c            |    2 +-
>  fs/jfs/acl.c              |    2 +-
>  fs/ocfs2/acl.c            |    2 +-
>  fs/posix_acl.c            |   30 +++++++++++++++++++++++++++---
>  fs/reiserfs/xattr_acl.c   |    2 +-
>  fs/xfs/xfs_acl.c          |    2 +-
>  include/linux/posix_acl.h |   17 +++++++++++++----
>  16 files changed, 54 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index 7af425f..f5ce5c5 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -156,7 +156,7 @@ int v9fs_acl_chmod(struct inode *inode, struct p9_fid *fid)
>  		return -EOPNOTSUPP;
>  	acl = v9fs_get_cached_acl(inode, ACL_TYPE_ACCESS);
>  	if (acl) {
> -		retval = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +		retval = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  		if (retval)
>  			return retval;
>  		set_cached_acl(inode, ACL_TYPE_ACCESS, acl);
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 0890c83..1af04ff 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -256,7 +256,7 @@ int btrfs_acl_chmod(struct inode *inode)
>  	if (IS_ERR_OR_NULL(acl))
>  		return PTR_ERR(acl);
>  
> -	ret = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	ret = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (ret)
>  		return ret;
>  	ret = btrfs_set_acl(NULL, inode, acl, ACL_TYPE_ACCESS);
> diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
> index 110b6b3..7006ced 100644
> --- a/fs/ext2/acl.c
> +++ b/fs/ext2/acl.c
> @@ -308,7 +308,7 @@ ext2_acl_chmod(struct inode *inode)
>  	acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
> -	error = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (error)
>  		return error;
>  	error = ext2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
> index dbb5ad5..6691a6c 100644
> --- a/fs/ext3/acl.c
> +++ b/fs/ext3/acl.c
> @@ -314,7 +314,7 @@ ext3_acl_chmod(struct inode *inode)
>  	acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
> -	error = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (error)
>  		return error;
>  retry:
> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> index 39a54a0..2eebe02 100644
> --- a/fs/ext4/acl.c
> +++ b/fs/ext4/acl.c
> @@ -320,7 +320,7 @@ ext4_acl_chmod(struct inode *inode)
>  	acl = ext4_get_acl(inode, ACL_TYPE_ACCESS);
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
> -	error = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (error)
>  		return error;
>  retry:
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index d0fc287..14c4df0 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -311,7 +311,7 @@ int f2fs_acl_chmod(struct inode *inode)
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
>  
> -	error = posix_acl_chmod(&acl, GFP_KERNEL, mode);
> +	error = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/generic_acl.c b/fs/generic_acl.c
> index b3f3676..46a5076 100644
> --- a/fs/generic_acl.c
> +++ b/fs/generic_acl.c
> @@ -158,7 +158,7 @@ generic_acl_chmod(struct inode *inode)
>  		return -EOPNOTSUPP;
>  	acl = get_cached_acl(inode, ACL_TYPE_ACCESS);
>  	if (acl) {
> -		error = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +		error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  		if (error)
>  			return error;
>  		set_cached_acl(inode, ACL_TYPE_ACCESS, acl);
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index f69ac0a..3e200c7 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -162,7 +162,7 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
>  	if (!acl)
>  		return gfs2_setattr_simple(inode, attr);
>  
> -	error = posix_acl_chmod(&acl, GFP_NOFS, attr->ia_mode);
> +	error = __posix_acl_chmod(&acl, GFP_NOFS, attr->ia_mode);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c
> index b609cc1..cab5fd6 100644
> --- a/fs/hfsplus/posix_acl.c
> +++ b/fs/hfsplus/posix_acl.c
> @@ -167,7 +167,7 @@ int hfsplus_posix_acl_chmod(struct inode *inode)
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
>  
> -	err = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	err = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (unlikely(err))
>  		return err;
>  
> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
> index 223283c..5853969 100644
> --- a/fs/jffs2/acl.c
> +++ b/fs/jffs2/acl.c
> @@ -335,7 +335,7 @@ int jffs2_acl_chmod(struct inode *inode)
>  	acl = jffs2_get_acl(inode, ACL_TYPE_ACCESS);
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
> -	rc = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	rc = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (rc)
>  		return rc;
>  	rc = jffs2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c
> index d254d6d..9c0fca8 100644
> --- a/fs/jfs/acl.c
> +++ b/fs/jfs/acl.c
> @@ -161,7 +161,7 @@ int jfs_acl_chmod(struct inode *inode)
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
>  
> -	rc = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	rc = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (rc)
>  		return rc;
>  
> diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
> index b4f788e..73ccf0e 100644
> --- a/fs/ocfs2/acl.c
> +++ b/fs/ocfs2/acl.c
> @@ -350,7 +350,7 @@ int ocfs2_acl_chmod(struct inode *inode)
>  	acl = ocfs2_get_acl(inode, ACL_TYPE_ACCESS);
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
> -	ret = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	ret = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (ret)
>  		return ret;
>  	ret = ocfs2_set_acl(NULL, inode, NULL, ACL_TYPE_ACCESS,
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 9dd03e0..9f76aaa 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -338,7 +338,7 @@ static int posix_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
>  /*
>   * Modify the ACL for the chmod syscall.
>   */
> -static int posix_acl_chmod_masq(struct posix_acl *acl, umode_t mode)
> +static int __posix_acl_chmod_masq(struct posix_acl *acl, umode_t mode)
>  {
>  	struct posix_acl_entry *group_obj = NULL, *mask_obj = NULL;
>  	struct posix_acl_entry *pa, *pe;
> @@ -402,12 +402,12 @@ posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p)
>  EXPORT_SYMBOL(posix_acl_create);
>  
>  int
> -posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode)
> +__posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode)
>  {
>  	struct posix_acl *clone = posix_acl_clone(*acl, gfp);
>  	int err = -ENOMEM;
>  	if (clone) {
> -		err = posix_acl_chmod_masq(clone, mode);
> +		err = __posix_acl_chmod_masq(clone, mode);
>  		if (err) {
>  			posix_acl_release(clone);
>  			clone = NULL;
> @@ -417,6 +417,30 @@ posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode)
>  	*acl = clone;
>  	return err;
>  }
> +EXPORT_SYMBOL(__posix_acl_chmod);
> +
> +int
> +posix_acl_chmod(struct inode *inode)
> +{
> +	struct posix_acl *acl;
> +	int ret = 0;
> +
> +	if (S_ISLNK(inode->i_mode))
> +		return -EOPNOTSUPP;
> +	if (!IS_POSIXACL(inode))
> +		return 0;
> +
> +	acl = get_acl(inode, ACL_TYPE_ACCESS);
> +	if (IS_ERR_OR_NULL(acl))
> +		return PTR_ERR(acl);
> +
> +	ret = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	if (ret)
> +		return ret;
> +	ret = inode->i_op->set_acl(inode, acl, ACL_TYPE_ACCESS);
> +	posix_acl_release(acl);
> +	return ret;
> +}
>  EXPORT_SYMBOL(posix_acl_chmod);
>  
>  struct posix_acl *get_acl(struct inode *inode, int type)
> diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
> index 6f721ea..ea4e443 100644
> --- a/fs/reiserfs/xattr_acl.c
> +++ b/fs/reiserfs/xattr_acl.c
> @@ -463,7 +463,7 @@ int reiserfs_acl_chmod(struct inode *inode)
>  		return 0;
>  	if (IS_ERR(acl))
>  		return PTR_ERR(acl);
> -	error = posix_acl_chmod(&acl, GFP_NOFS, inode->i_mode);
> +	error = __posix_acl_chmod(&acl, GFP_NOFS, inode->i_mode);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
> index 370eb3e..4eac105 100644
> --- a/fs/xfs/xfs_acl.c
> +++ b/fs/xfs/xfs_acl.c
> @@ -334,7 +334,7 @@ xfs_acl_chmod(struct inode *inode)
>  	if (IS_ERR(acl) || !acl)
>  		return PTR_ERR(acl);
>  
> -	error = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
>  	if (error)
>  		return error;
>  
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index a8d9918..8b64e78 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -89,12 +89,14 @@ extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
>  extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
>  extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *);
>  extern int posix_acl_create(struct posix_acl **, gfp_t, umode_t *);
> -extern int posix_acl_chmod(struct posix_acl **, gfp_t, umode_t);
> +extern int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t);
>  
>  extern struct posix_acl *get_posix_acl(struct inode *, int);
>  extern int set_posix_acl(struct inode *, int, struct posix_acl *);
>  
>  #ifdef CONFIG_FS_POSIX_ACL
> +extern int posix_acl_chmod(struct inode *);
> +
>  static inline struct posix_acl **acl_by_type(struct inode *inode, int type)
>  {
>  	switch (type) {
> @@ -165,15 +167,22 @@ static inline void forget_all_cached_acls(struct inode *inode)
>  	if (old_default != ACL_NOT_CACHED)
>  		posix_acl_release(old_default);
>  }
> -#endif
>  
>  static inline void cache_no_acl(struct inode *inode)
>  {
> -#ifdef CONFIG_FS_POSIX_ACL
>  	inode->i_acl = NULL;
>  	inode->i_default_acl = NULL;
> -#endif
>  }
> +#else
> +static inline int posix_acl_chmod(struct inode *inode)
> +{
> +	return 0;
> +}
> +
> +static inline void cache_no_acl(struct inode *inode)
> +{
> +}
> +#endif /* CONFIG_FS_POSIX_ACL */
>  
>  struct posix_acl *get_acl(struct inode *inode, int type);
>  
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 06/18] fs: make posix_acl_create more useful

[Jan Kara]

On Sun 01-12-13 03:59:09, Christoph Hellwig wrote:
> Rename the current posix_acl_created to __posix_acl_create and add
> a fully featured helper to set up the ACLs on file creation that
> uses get_acl().
  Looks good, you can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/9p/acl.c               |    2 +-
>  fs/btrfs/acl.c            |    2 +-
>  fs/ext2/acl.c             |    2 +-
>  fs/ext3/acl.c             |    2 +-
>  fs/ext4/acl.c             |    2 +-
>  fs/f2fs/acl.c             |    2 +-
>  fs/generic_acl.c          |    2 +-
>  fs/gfs2/acl.c             |    2 +-
>  fs/hfsplus/posix_acl.c    |    2 +-
>  fs/jffs2/acl.c            |    2 +-
>  fs/jfs/acl.c              |    2 +-
>  fs/nfs/nfs3acl.c          |    2 +-
>  fs/ocfs2/acl.c            |    2 +-
>  fs/posix_acl.c            |   53 +++++++++++++++++++++++++++++++++++++++++++--
>  fs/reiserfs/xattr_acl.c   |    2 +-
>  fs/xfs/xfs_acl.c          |    4 ++--
>  include/linux/posix_acl.h |    6 ++++-
>  17 files changed, 72 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index f5ce5c5..8482f2d 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -200,7 +200,7 @@ int v9fs_acl_mode(struct inode *dir, umode_t *modep,
>  	if (acl) {
>  		if (S_ISDIR(mode))
>  			*dpacl = posix_acl_dup(acl);
> -		retval = posix_acl_create(&acl, GFP_NOFS, &mode);
> +		retval = __posix_acl_create(&acl, GFP_NOFS, &mode);
>  		if (retval < 0)
>  			return retval;
>  		if (retval > 0)
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 1af04ff..b56519d 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -222,7 +222,7 @@ int btrfs_init_acl(struct btrfs_trans_handle *trans,
>  			if (ret)
>  				goto failed;
>  		}
> -		ret = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +		ret = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>  		if (ret < 0)
>  			return ret;
>  
> diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
> index 7006ced..6e842a7 100644
> --- a/fs/ext2/acl.c
> +++ b/fs/ext2/acl.c
> @@ -268,7 +268,7 @@ ext2_init_acl(struct inode *inode, struct inode *dir)
>  			if (error)
>  				goto cleanup;
>  		}
> -		error = posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> +		error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
>  		if (error < 0)
>  			return error;
>  		if (error > 0) {
> diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
> index 6691a6c..4f3d8fa 100644
> --- a/fs/ext3/acl.c
> +++ b/fs/ext3/acl.c
> @@ -271,7 +271,7 @@ ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
>  			if (error)
>  				goto cleanup;
>  		}
> -		error = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +		error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>  		if (error < 0)
>  			return error;
>  
> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> index 2eebe02..f827f3b 100644
> --- a/fs/ext4/acl.c
> +++ b/fs/ext4/acl.c
> @@ -276,7 +276,7 @@ ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
>  			if (error)
>  				goto cleanup;
>  		}
> -		error = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +		error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>  		if (error < 0)
>  			return error;
>  
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index 14c4df0..45e8430 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -285,7 +285,7 @@ int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage)
>  		if (error)
>  			goto cleanup;
>  	}
> -	error = posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> +	error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
>  	if (error < 0)
>  		return error;
>  	if (error > 0)
> diff --git a/fs/generic_acl.c b/fs/generic_acl.c
> index 46a5076..4357f39 100644
> --- a/fs/generic_acl.c
> +++ b/fs/generic_acl.c
> @@ -128,7 +128,7 @@ generic_acl_init(struct inode *inode, struct inode *dir)
>  	if (acl) {
>  		if (S_ISDIR(inode->i_mode))
>  			set_cached_acl(inode, ACL_TYPE_DEFAULT, acl);
> -		error = posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> +		error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
>  		if (error < 0)
>  			return error;
>  		if (error > 0)
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 3e200c7..e82e4ac 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -131,7 +131,7 @@ int gfs2_acl_create(struct gfs2_inode *dip, struct inode *inode)
>  			goto out;
>  	}
>  
> -	error = posix_acl_create(&acl, GFP_NOFS, &mode);
> +	error = __posix_acl_create(&acl, GFP_NOFS, &mode);
>  	if (error < 0)
>  		return error;
>  
> diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c
> index cab5fd6..277942f 100644
> --- a/fs/hfsplus/posix_acl.c
> +++ b/fs/hfsplus/posix_acl.c
> @@ -137,7 +137,7 @@ int hfsplus_init_posix_acl(struct inode *inode, struct inode *dir)
>  				goto init_acl_cleanup;
>  		}
>  
> -		err = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +		err = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>  		if (unlikely(err < 0))
>  			return err;
>  
> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
> index 5853969..4d6e31b 100644
> --- a/fs/jffs2/acl.c
> +++ b/fs/jffs2/acl.c
> @@ -295,7 +295,7 @@ int jffs2_init_acl_pre(struct inode *dir_i, struct inode *inode, umode_t *i_mode
>  		if (S_ISDIR(*i_mode))
>  			set_cached_acl(inode, ACL_TYPE_DEFAULT, acl);
>  
> -		rc = posix_acl_create(&acl, GFP_KERNEL, i_mode);
> +		rc = __posix_acl_create(&acl, GFP_KERNEL, i_mode);
>  		if (rc < 0)
>  			return rc;
>  		if (rc > 0)
> diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c
> index 9c0fca8..28d529a 100644
> --- a/fs/jfs/acl.c
> +++ b/fs/jfs/acl.c
> @@ -132,7 +132,7 @@ int jfs_init_acl(tid_t tid, struct inode *inode, struct inode *dir)
>  			if (rc)
>  				goto cleanup;
>  		}
> -		rc = posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> +		rc = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
>  		if (rc < 0)
>  			goto cleanup; /* posix_acl_release(NULL) is no-op */
>  		if (rc > 0)
> diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
> index 4a1aafb..e859675 100644
> --- a/fs/nfs/nfs3acl.c
> +++ b/fs/nfs/nfs3acl.c
> @@ -428,7 +428,7 @@ int nfs3_proc_set_default_acl(struct inode *dir, struct inode *inode,
>  	if (!dfacl)
>  		return 0;
>  	acl = posix_acl_dup(dfacl);
> -	error = posix_acl_create(&acl, GFP_KERNEL, &mode);
> +	error = __posix_acl_create(&acl, GFP_KERNEL, &mode);
>  	if (error < 0)
>  		goto out_release_dfacl;
>  	error = nfs3_proc_setacls(inode, acl, S_ISDIR(inode->i_mode) ?
> diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
> index 73ccf0e..c0f9d2f 100644
> --- a/fs/ocfs2/acl.c
> +++ b/fs/ocfs2/acl.c
> @@ -401,7 +401,7 @@ int ocfs2_init_acl(handle_t *handle,
>  				goto cleanup;
>  		}
>  		mode = inode->i_mode;
> -		ret = posix_acl_create(&acl, GFP_NOFS, &mode);
> +		ret = __posix_acl_create(&acl, GFP_NOFS, &mode);
>  		if (ret < 0)
>  			return ret;
>  
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 9f76aaa..38d6a49 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -384,7 +384,7 @@ static int __posix_acl_chmod_masq(struct posix_acl *acl, umode_t mode)
>  }
>  
>  int
> -posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p)
> +__posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p)
>  {
>  	struct posix_acl *clone = posix_acl_clone(*acl, gfp);
>  	int err = -ENOMEM;
> @@ -399,7 +399,7 @@ posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p)
>  	*acl = clone;
>  	return err;
>  }
> -EXPORT_SYMBOL(posix_acl_create);
> +EXPORT_SYMBOL(__posix_acl_create);
>  
>  int
>  __posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode)
> @@ -443,6 +443,55 @@ posix_acl_chmod(struct inode *inode)
>  }
>  EXPORT_SYMBOL(posix_acl_chmod);
>  
> +int
> +posix_acl_create(struct inode *dir, umode_t *mode,
> +		struct posix_acl **default_acl, struct posix_acl **acl)
> +{
> +	struct posix_acl *p;
> +	int ret;
> +
> +	if (S_ISLNK(*mode) || !IS_POSIXACL(dir))
> +		goto no_acl;
> +
> +	p = get_acl(dir, ACL_TYPE_DEFAULT);
> +	if (IS_ERR(p))
> +		return PTR_ERR(p);
> +
> +	if (!p) {
> +		*mode &= ~current_umask();
> +		goto no_acl;
> +	}
> +
> +	*acl = posix_acl_clone(p, GFP_NOFS);
> +	if (!*acl)
> +		return -ENOMEM;
> +
> +	ret = posix_acl_create_masq(*acl, mode);
> +	if (ret < 0) {
> +		posix_acl_release(*acl);
> +		return -ENOMEM;
> +	}
> +
> +	if (ret == 0) {
> +		posix_acl_release(*acl);
> +		*acl = NULL;
> +	}
> +
> +	if (!S_ISDIR(*mode)) {
> +		posix_acl_release(p);
> +		*default_acl = NULL;
> +	} else {
> +		*default_acl = p;
> +	}
> +	return 0;
> +
> +no_acl:
> +	*default_acl = NULL;
> +	*acl = NULL;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(posix_acl_create);
> +
>  struct posix_acl *get_acl(struct inode *inode, int type)
>  {
>  	struct posix_acl *acl;
> diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
> index ea4e443..d95c959 100644
> --- a/fs/reiserfs/xattr_acl.c
> +++ b/fs/reiserfs/xattr_acl.c
> @@ -378,7 +378,7 @@ reiserfs_inherit_default_acl(struct reiserfs_transaction_handle *th,
>  
>  		/* Now we reconcile the new ACL and the mode,
>  		   potentially modifying both */
> -		err = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +		err = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>  		if (err < 0)
>  			return err;
>  
> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
> index 4eac105..057ae2d 100644
> --- a/fs/xfs/xfs_acl.c
> +++ b/fs/xfs/xfs_acl.c
> @@ -297,12 +297,12 @@ xfs_inherit_acl(struct inode *inode, struct posix_acl *acl)
>  			goto out;
>  	}
>  
> -	error = posix_acl_create(&acl, GFP_KERNEL, &mode);
> +	error = __posix_acl_create(&acl, GFP_KERNEL, &mode);
>  	if (error < 0)
>  		return error;
>  
>  	/*
> -	 * If posix_acl_create returns a positive value we need to
> +	 * If __posix_acl_create returns a positive value we need to
>  	 * inherit a permission that can't be represented using the Unix
>  	 * mode bits and we actually need to set an ACL.
>  	 */
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 8b64e78..9ec6b45 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -88,14 +88,18 @@ extern int posix_acl_valid(const struct posix_acl *);
>  extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
>  extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
>  extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *);
> -extern int posix_acl_create(struct posix_acl **, gfp_t, umode_t *);
> +extern int __posix_acl_create(struct posix_acl **, gfp_t, umode_t *);
>  extern int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t);
> +extern int posix_acl_prepare(struct inode *dir, struct inode *inode,
> +		umode_t *mode);
>  
>  extern struct posix_acl *get_posix_acl(struct inode *, int);
>  extern int set_posix_acl(struct inode *, int, struct posix_acl *);
>  
>  #ifdef CONFIG_FS_POSIX_ACL
>  extern int posix_acl_chmod(struct inode *);
> +extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **,
> +		struct posix_acl **);
>  
>  static inline struct posix_acl **acl_by_type(struct inode *inode, int type)
>  {
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [Jfs-discussion] [PATCH 15/18] jfs: use generic posix ACL infrastructure

[Dave Kleikamp]

On 12/01/2013 05:59 AM, Christoph Hellwig wrote:
> Copy the scheme I introduced to btrfs many years ago to only use the
> xattr handler for ACLs, but pass plain attrs straight through.

Looks good.

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Kleikamp <dave.kleikamp@oracle.com>

> ---
>  fs/jfs/acl.c       |  105 ++++++++++++++++++++------------------------------
>  fs/jfs/file.c      |    4 +-
>  fs/jfs/jfs_acl.h   |    7 +---
>  fs/jfs/jfs_xattr.h |    2 +
>  fs/jfs/namei.c     |    1 +
>  fs/jfs/super.c     |    2 +
>  fs/jfs/xattr.c     |  108 ++++++++++++++++++----------------------------------
>  7 files changed, 89 insertions(+), 140 deletions(-)

[Cluster-devel] [PATCH 08/18] ext2/3/4: use generic posix ACL infrastructure

[Jan Kara]

On Sun 01-12-13 03:59:11, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

							Honza

> ---
>  fs/ext2/acl.c   |  176 ++++-----------------------------------------
>  fs/ext2/acl.h   |    8 +--
>  fs/ext2/file.c  |    1 +
>  fs/ext2/inode.c |    2 +-
>  fs/ext2/namei.c |    2 +
>  fs/ext2/xattr.c |    8 +--
>  fs/ext2/xattr.h |    2 -
>  fs/ext3/acl.c   |  213 ++++++++-----------------------------------------------
>  fs/ext3/acl.h   |    9 +--
>  fs/ext3/file.c  |    1 +
>  fs/ext3/inode.c |    2 +-
>  fs/ext3/namei.c |    2 +
>  fs/ext3/xattr.c |    8 +--
>  fs/ext3/xattr.h |    2 -
>  fs/ext4/acl.c   |  213 ++++++++-----------------------------------------------
>  fs/ext4/acl.h   |    9 +--
>  fs/ext4/file.c  |    1 +
>  fs/ext4/inode.c |    2 +-
>  fs/ext4/namei.c |    2 +
>  fs/ext4/xattr.c |    8 +--
>  fs/ext4/xattr.h |    2 -
>  21 files changed, 100 insertions(+), 573 deletions(-)
> 
> diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
> index 6e842a7..f04a295 100644
> --- a/fs/ext2/acl.c
> +++ b/fs/ext2/acl.c
> @@ -189,8 +189,8 @@ ext2_get_acl(struct inode *inode, int type)
>  /*
>   * inode->i_mutex: down
>   */
> -static int
> -ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
> +int
> +ext2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  {
>  	int name_index;
>  	void *value = NULL;
> @@ -250,169 +250,21 @@ ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
>  int
>  ext2_init_acl(struct inode *inode, struct inode *dir)
>  {
> -	struct posix_acl *acl = NULL;
> -	int error = 0;
> -
> -	if (!S_ISLNK(inode->i_mode)) {
> -		if (test_opt(dir->i_sb, POSIX_ACL)) {
> -			acl = ext2_get_acl(dir, ACL_TYPE_DEFAULT);
> -			if (IS_ERR(acl))
> -				return PTR_ERR(acl);
> -		}
> -		if (!acl)
> -			inode->i_mode &= ~current_umask();
> -	}
> -	if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> -		if (S_ISDIR(inode->i_mode)) {
> -			error = ext2_set_acl(inode, ACL_TYPE_DEFAULT, acl);
> -			if (error)
> -				goto cleanup;
> -		}
> -		error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> -		if (error < 0)
> -			return error;
> -		if (error > 0) {
> -			/* This is an extended ACL */
> -			error = ext2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> -		}
> -	}
> -cleanup:
> -       posix_acl_release(acl);
> -       return error;
> -}
> -
> -/*
> - * Does chmod for an inode that may have an Access Control List. The
> - * inode->i_mode field must be updated to the desired value by the caller
> - * before calling this function.
> - * Returns 0 on success, or a negative error number.
> - *
> - * We change the ACL rather than storing some ACL entries in the file
> - * mode permission bits (which would be more efficient), because that
> - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> - * for directories) are added. There are no more bits available in the
> - * file mode.
> - *
> - * inode->i_mutex: down
> - */
> -int
> -ext2_acl_chmod(struct inode *inode)
> -{
> -	struct posix_acl *acl;
> -        int error;
> +	struct posix_acl *default_acl, *acl;
> +	int error;
>  
> -	if (!test_opt(inode->i_sb, POSIX_ACL))
> -		return 0;
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -	acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl) || !acl)
> -		return PTR_ERR(acl);
> -	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> +	error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
>  	if (error)
>  		return error;
> -	error = ext2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -/*
> - * Extended attribut handlers
> - */
> -static size_t
> -ext2_xattr_list_acl_access(struct dentry *dentry, char *list, size_t list_size,
> -			   const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_size)
> -		memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> -	return size;
> -}
>  
> -static size_t
> -ext2_xattr_list_acl_default(struct dentry *dentry, char *list, size_t list_size,
> -			    const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_size)
> -		memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> -	return size;
> -}
> -
> -static int
> -ext2_xattr_get_acl(struct dentry *dentry, const char *name, void *buffer,
> -		   size_t size, int type)
> -{
> -	struct posix_acl *acl;
> -	int error;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -
> -	acl = ext2_get_acl(dentry->d_inode, type);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (acl == NULL)
> -		return -ENODATA;
> -	error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
> -	return error;
> -}
> -
> -static int
> -ext2_xattr_set_acl(struct dentry *dentry, const char *name, const void *value,
> -		   size_t size, int flags, int type)
> -{
> -	struct posix_acl *acl;
> -	int error;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -	if (!inode_owner_or_capable(dentry->d_inode))
> -		return -EPERM;
> -
> -	if (value) {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl))
> -			return PTR_ERR(acl);
> -		else if (acl) {
> -			error = posix_acl_valid(acl);
> -			if (error)
> -				goto release_and_out;
> -		}
> -	} else
> -		acl = NULL;
> -
> -	error = ext2_set_acl(dentry->d_inode, type, acl);
> -
> -release_and_out:
> -	posix_acl_release(acl);
> +	if (default_acl) {
> +		error = ext2_set_acl(inode, default_acl, ACL_TYPE_DEFAULT);
> +		posix_acl_release(default_acl);
> +	}
> +	if (acl) {
> +		if (!error)
> +			error = ext2_set_acl(inode, acl, ACL_TYPE_ACCESS);
> +		posix_acl_release(acl);
> +	}
>  	return error;
>  }
> -
> -const struct xattr_handler ext2_xattr_acl_access_handler = {
> -	.prefix	= POSIX_ACL_XATTR_ACCESS,
> -	.flags	= ACL_TYPE_ACCESS,
> -	.list	= ext2_xattr_list_acl_access,
> -	.get	= ext2_xattr_get_acl,
> -	.set	= ext2_xattr_set_acl,
> -};
> -
> -const struct xattr_handler ext2_xattr_acl_default_handler = {
> -	.prefix	= POSIX_ACL_XATTR_DEFAULT,
> -	.flags	= ACL_TYPE_DEFAULT,
> -	.list	= ext2_xattr_list_acl_default,
> -	.get	= ext2_xattr_get_acl,
> -	.set	= ext2_xattr_set_acl,
> -};
> diff --git a/fs/ext2/acl.h b/fs/ext2/acl.h
> index 503bfb0..44937f9 100644
> --- a/fs/ext2/acl.h
> +++ b/fs/ext2/acl.h
> @@ -55,7 +55,7 @@ static inline int ext2_acl_count(size_t size)
>  
>  /* acl.c */
>  extern struct posix_acl *ext2_get_acl(struct inode *inode, int type);
> -extern int ext2_acl_chmod (struct inode *);
> +extern int ext2_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  extern int ext2_init_acl (struct inode *, struct inode *);
>  
>  #else
> @@ -63,12 +63,6 @@ extern int ext2_init_acl (struct inode *, struct inode *);
>  #define ext2_get_acl	NULL
>  #define ext2_set_acl	NULL
>  
> -static inline int
> -ext2_acl_chmod (struct inode *inode)
> -{
> -	return 0;
> -}
> -
>  static inline int ext2_init_acl (struct inode *inode, struct inode *dir)
>  {
>  	return 0;
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index a5b3a5d..44c36e5 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -103,5 +103,6 @@ const struct inode_operations ext2_file_inode_operations = {
>  #endif
>  	.setattr	= ext2_setattr,
>  	.get_acl	= ext2_get_acl,
> +	.set_acl	= ext2_set_acl,
>  	.fiemap		= ext2_fiemap,
>  };
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 8a33764..1be8866 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1566,7 +1566,7 @@ int ext2_setattr(struct dentry *dentry, struct iattr *iattr)
>  	}
>  	setattr_copy(inode, iattr);
>  	if (iattr->ia_valid & ATTR_MODE)
> -		error = ext2_acl_chmod(inode);
> +		error = posix_acl_chmod(inode);
>  	mark_inode_dirty(inode);
>  
>  	return error;
> diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
> index 256dd5f..c268d0a 100644
> --- a/fs/ext2/namei.c
> +++ b/fs/ext2/namei.c
> @@ -421,6 +421,7 @@ const struct inode_operations ext2_dir_inode_operations = {
>  #endif
>  	.setattr	= ext2_setattr,
>  	.get_acl	= ext2_get_acl,
> +	.set_acl	= ext2_set_acl,
>  	.tmpfile	= ext2_tmpfile,
>  };
>  
> @@ -433,4 +434,5 @@ const struct inode_operations ext2_special_inode_operations = {
>  #endif
>  	.setattr	= ext2_setattr,
>  	.get_acl	= ext2_get_acl,
> +	.set_acl	= ext2_set_acl,
>  };
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 2d7557d..9142614 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -103,8 +103,8 @@ static struct mb_cache *ext2_xattr_cache;
>  static const struct xattr_handler *ext2_xattr_handler_map[] = {
>  	[EXT2_XATTR_INDEX_USER]		     = &ext2_xattr_user_handler,
>  #ifdef CONFIG_EXT2_FS_POSIX_ACL
> -	[EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  = &ext2_xattr_acl_access_handler,
> -	[EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &ext2_xattr_acl_default_handler,
> +	[EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  = &posix_acl_access_xattr_handler,
> +	[EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
>  #endif
>  	[EXT2_XATTR_INDEX_TRUSTED]	     = &ext2_xattr_trusted_handler,
>  #ifdef CONFIG_EXT2_FS_SECURITY
> @@ -116,8 +116,8 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
>  	&ext2_xattr_user_handler,
>  	&ext2_xattr_trusted_handler,
>  #ifdef CONFIG_EXT2_FS_POSIX_ACL
> -	&ext2_xattr_acl_access_handler,
> -	&ext2_xattr_acl_default_handler,
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
>  #endif
>  #ifdef CONFIG_EXT2_FS_SECURITY
>  	&ext2_xattr_security_handler,
> diff --git a/fs/ext2/xattr.h b/fs/ext2/xattr.h
> index 5e41ccc..60edf29 100644
> --- a/fs/ext2/xattr.h
> +++ b/fs/ext2/xattr.h
> @@ -57,8 +57,6 @@ struct ext2_xattr_entry {
>  
>  extern const struct xattr_handler ext2_xattr_user_handler;
>  extern const struct xattr_handler ext2_xattr_trusted_handler;
> -extern const struct xattr_handler ext2_xattr_acl_access_handler;
> -extern const struct xattr_handler ext2_xattr_acl_default_handler;
>  extern const struct xattr_handler ext2_xattr_security_handler;
>  
>  extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
> diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
> index 4f3d8fa..e9cb33f 100644
> --- a/fs/ext3/acl.c
> +++ b/fs/ext3/acl.c
> @@ -190,7 +190,7 @@ ext3_get_acl(struct inode *inode, int type)
>   * inode->i_mutex: down unless called from ext3_new_inode
>   */
>  static int
> -ext3_set_acl(handle_t *handle, struct inode *inode, int type,
> +__ext3_set_acl(handle_t *handle, struct inode *inode, int type,
>  	     struct posix_acl *acl)
>  {
>  	int name_index;
> @@ -243,204 +243,49 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type,
>  	return error;
>  }
>  
> -/*
> - * Initialize the ACLs of a new inode. Called from ext3_new_inode.
> - *
> - * dir->i_mutex: down
> - * inode->i_mutex: up (access to inode is still exclusive)
> - */
> -int
> -ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> -{
> -	struct posix_acl *acl = NULL;
> -	int error = 0;
> -
> -	if (!S_ISLNK(inode->i_mode)) {
> -		if (test_opt(dir->i_sb, POSIX_ACL)) {
> -			acl = ext3_get_acl(dir, ACL_TYPE_DEFAULT);
> -			if (IS_ERR(acl))
> -				return PTR_ERR(acl);
> -		}
> -		if (!acl)
> -			inode->i_mode &= ~current_umask();
> -	}
> -	if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> -		if (S_ISDIR(inode->i_mode)) {
> -			error = ext3_set_acl(handle, inode,
> -					     ACL_TYPE_DEFAULT, acl);
> -			if (error)
> -				goto cleanup;
> -		}
> -		error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> -		if (error < 0)
> -			return error;
> -
> -		if (error > 0) {
> -			/* This is an extended ACL */
> -			error = ext3_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> -		}
> -	}
> -cleanup:
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -/*
> - * Does chmod for an inode that may have an Access Control List. The
> - * inode->i_mode field must be updated to the desired value by the caller
> - * before calling this function.
> - * Returns 0 on success, or a negative error number.
> - *
> - * We change the ACL rather than storing some ACL entries in the file
> - * mode permission bits (which would be more efficient), because that
> - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> - * for directories) are added. There are no more bits available in the
> - * file mode.
> - *
> - * inode->i_mutex: down
> - */
>  int
> -ext3_acl_chmod(struct inode *inode)
> +ext3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  {
> -	struct posix_acl *acl;
>  	handle_t *handle;
> -	int retries = 0;
> -        int error;
> +	int error, retries = 0;
>  
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -	if (!test_opt(inode->i_sb, POSIX_ACL))
> -		return 0;
> -	acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl) || !acl)
> -		return PTR_ERR(acl);
> -	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> -	if (error)
> -		return error;
>  retry:
> -	handle = ext3_journal_start(inode,
> -			EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> -	if (IS_ERR(handle)) {
> -		error = PTR_ERR(handle);
> -		ext3_std_error(inode->i_sb, error);
> -		goto out;
> -	}
> -	error = ext3_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> +	handle = ext3_journal_start(inode, EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	error = __ext3_set_acl(handle, inode, type, acl);
>  	ext3_journal_stop(handle);
> -	if (error == -ENOSPC &&
> -	    ext3_should_retry_alloc(inode->i_sb, &retries))
> +	if (error == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;
> -out:
> -	posix_acl_release(acl);
>  	return error;
>  }
>  
>  /*
> - * Extended attribute handlers
> + * Initialize the ACLs of a new inode. Called from ext3_new_inode.
> + *
> + * dir->i_mutex: down
> + * inode->i_mutex: up (access to inode is still exclusive)
>   */
> -static size_t
> -ext3_xattr_list_acl_access(struct dentry *dentry, char *list, size_t list_len,
> -			   const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_len)
> -		memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> -	return size;
> -}
> -
> -static size_t
> -ext3_xattr_list_acl_default(struct dentry *dentry, char *list, size_t list_len,
> -			    const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_len)
> -		memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> -	return size;
> -}
> -
> -static int
> -ext3_xattr_get_acl(struct dentry *dentry, const char *name, void *buffer,
> -		   size_t size, int type)
> +int
> +ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
>  {
> -	struct posix_acl *acl;
> +	struct posix_acl *default_acl, *acl;
>  	int error;
>  
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -
> -	acl = ext3_get_acl(dentry->d_inode, type);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (acl == NULL)
> -		return -ENODATA;
> -	error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
> -	return error;
> -}
> -
> -static int
> -ext3_xattr_set_acl(struct dentry *dentry, const char *name, const void *value,
> -		   size_t size, int flags, int type)
> -{
> -	struct inode *inode = dentry->d_inode;
> -	handle_t *handle;
> -	struct posix_acl *acl;
> -	int error, retries = 0;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(inode->i_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -	if (!inode_owner_or_capable(inode))
> -		return -EPERM;
> -
> -	if (value) {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl))
> -			return PTR_ERR(acl);
> -		else if (acl) {
> -			error = posix_acl_valid(acl);
> -			if (error)
> -				goto release_and_out;
> -		}
> -	} else
> -		acl = NULL;
> -
> -retry:
> -	handle = ext3_journal_start(inode, EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> -	if (IS_ERR(handle))
> -		return PTR_ERR(handle);
> -	error = ext3_set_acl(handle, inode, type, acl);
> -	ext3_journal_stop(handle);
> -	if (error == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
> -		goto retry;
> +	error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> +	if (error)
> +		return error;
>  
> -release_and_out:
> -	posix_acl_release(acl);
> +	if (default_acl) {
> +		error = __ext3_set_acl(handle, inode, ACL_TYPE_DEFAULT,
> +				       default_acl);
> +		posix_acl_release(default_acl);
> +	}
> +	if (acl) {
> +		if (!error)
> +			error = __ext3_set_acl(handle, inode, ACL_TYPE_ACCESS,
> +					       acl);
> +		posix_acl_release(acl);
> +	}
>  	return error;
>  }
> -
> -const struct xattr_handler ext3_xattr_acl_access_handler = {
> -	.prefix	= POSIX_ACL_XATTR_ACCESS,
> -	.flags	= ACL_TYPE_ACCESS,
> -	.list	= ext3_xattr_list_acl_access,
> -	.get	= ext3_xattr_get_acl,
> -	.set	= ext3_xattr_set_acl,
> -};
> -
> -const struct xattr_handler ext3_xattr_acl_default_handler = {
> -	.prefix	= POSIX_ACL_XATTR_DEFAULT,
> -	.flags	= ACL_TYPE_DEFAULT,
> -	.list	= ext3_xattr_list_acl_default,
> -	.get	= ext3_xattr_get_acl,
> -	.set	= ext3_xattr_set_acl,
> -};
> diff --git a/fs/ext3/acl.h b/fs/ext3/acl.h
> index dbc921e..ea1c69e 100644
> --- a/fs/ext3/acl.h
> +++ b/fs/ext3/acl.h
> @@ -55,18 +55,13 @@ static inline int ext3_acl_count(size_t size)
>  
>  /* acl.c */
>  extern struct posix_acl *ext3_get_acl(struct inode *inode, int type);
> -extern int ext3_acl_chmod (struct inode *);
> +extern int ext3_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  extern int ext3_init_acl (handle_t *, struct inode *, struct inode *);
>  
>  #else  /* CONFIG_EXT3_FS_POSIX_ACL */
>  #include <linux/sched.h>
>  #define ext3_get_acl NULL
> -
> -static inline int
> -ext3_acl_chmod(struct inode *inode)
> -{
> -	return 0;
> -}
> +#define ext3_set_acl NULL
>  
>  static inline int
>  ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> diff --git a/fs/ext3/file.c b/fs/ext3/file.c
> index 25cb413..aad0531 100644
> --- a/fs/ext3/file.c
> +++ b/fs/ext3/file.c
> @@ -75,6 +75,7 @@ const struct inode_operations ext3_file_inode_operations = {
>  	.removexattr	= generic_removexattr,
>  #endif
>  	.get_acl	= ext3_get_acl,
> +	.set_acl	= ext3_set_acl,
>  	.fiemap		= ext3_fiemap,
>  };
>  
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 2bd8548..150b6c1 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -3365,7 +3365,7 @@ int ext3_setattr(struct dentry *dentry, struct iattr *attr)
>  	mark_inode_dirty(inode);
>  
>  	if (ia_valid & ATTR_MODE)
> -		rc = ext3_acl_chmod(inode);
> +		rc = posix_acl_chmod(inode);
>  
>  err_out:
>  	ext3_std_error(inode->i_sb, error);
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index f8cde46..f197736 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -2569,6 +2569,7 @@ const struct inode_operations ext3_dir_inode_operations = {
>  	.removexattr	= generic_removexattr,
>  #endif
>  	.get_acl	= ext3_get_acl,
> +	.set_acl	= ext3_set_acl,
>  };
>  
>  const struct inode_operations ext3_special_inode_operations = {
> @@ -2580,4 +2581,5 @@ const struct inode_operations ext3_special_inode_operations = {
>  	.removexattr	= generic_removexattr,
>  #endif
>  	.get_acl	= ext3_get_acl,
> +	.set_acl	= ext3_set_acl,
>  };
> diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
> index b1fc963..c6874be 100644
> --- a/fs/ext3/xattr.c
> +++ b/fs/ext3/xattr.c
> @@ -102,8 +102,8 @@ static struct mb_cache *ext3_xattr_cache;
>  static const struct xattr_handler *ext3_xattr_handler_map[] = {
>  	[EXT3_XATTR_INDEX_USER]		     = &ext3_xattr_user_handler,
>  #ifdef CONFIG_EXT3_FS_POSIX_ACL
> -	[EXT3_XATTR_INDEX_POSIX_ACL_ACCESS]  = &ext3_xattr_acl_access_handler,
> -	[EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT] = &ext3_xattr_acl_default_handler,
> +	[EXT3_XATTR_INDEX_POSIX_ACL_ACCESS]  = &posix_acl_access_xattr_handler,
> +	[EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
>  #endif
>  	[EXT3_XATTR_INDEX_TRUSTED]	     = &ext3_xattr_trusted_handler,
>  #ifdef CONFIG_EXT3_FS_SECURITY
> @@ -115,8 +115,8 @@ const struct xattr_handler *ext3_xattr_handlers[] = {
>  	&ext3_xattr_user_handler,
>  	&ext3_xattr_trusted_handler,
>  #ifdef CONFIG_EXT3_FS_POSIX_ACL
> -	&ext3_xattr_acl_access_handler,
> -	&ext3_xattr_acl_default_handler,
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
>  #endif
>  #ifdef CONFIG_EXT3_FS_SECURITY
>  	&ext3_xattr_security_handler,
> diff --git a/fs/ext3/xattr.h b/fs/ext3/xattr.h
> index 2be4f69..32e93eb 100644
> --- a/fs/ext3/xattr.h
> +++ b/fs/ext3/xattr.h
> @@ -60,8 +60,6 @@ struct ext3_xattr_entry {
>  
>  extern const struct xattr_handler ext3_xattr_user_handler;
>  extern const struct xattr_handler ext3_xattr_trusted_handler;
> -extern const struct xattr_handler ext3_xattr_acl_access_handler;
> -extern const struct xattr_handler ext3_xattr_acl_default_handler;
>  extern const struct xattr_handler ext3_xattr_security_handler;
>  
>  extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> index f827f3b..acaba0f 100644
> --- a/fs/ext4/acl.c
> +++ b/fs/ext4/acl.c
> @@ -196,7 +196,7 @@ ext4_get_acl(struct inode *inode, int type)
>   * inode->i_mutex: down unless called from ext4_new_inode
>   */
>  static int
> -ext4_set_acl(handle_t *handle, struct inode *inode, int type,
> +__ext4_set_acl(handle_t *handle, struct inode *inode, int type,
>  	     struct posix_acl *acl)
>  {
>  	int name_index;
> @@ -248,208 +248,51 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type,
>  	return error;
>  }
>  
> -/*
> - * Initialize the ACLs of a new inode. Called from ext4_new_inode.
> - *
> - * dir->i_mutex: down
> - * inode->i_mutex: up (access to inode is still exclusive)
> - */
> -int
> -ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> -{
> -	struct posix_acl *acl = NULL;
> -	int error = 0;
> -
> -	if (!S_ISLNK(inode->i_mode)) {
> -		if (test_opt(dir->i_sb, POSIX_ACL)) {
> -			acl = ext4_get_acl(dir, ACL_TYPE_DEFAULT);
> -			if (IS_ERR(acl))
> -				return PTR_ERR(acl);
> -		}
> -		if (!acl)
> -			inode->i_mode &= ~current_umask();
> -	}
> -	if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> -		if (S_ISDIR(inode->i_mode)) {
> -			error = ext4_set_acl(handle, inode,
> -					     ACL_TYPE_DEFAULT, acl);
> -			if (error)
> -				goto cleanup;
> -		}
> -		error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> -		if (error < 0)
> -			return error;
> -
> -		if (error > 0) {
> -			/* This is an extended ACL */
> -			error = ext4_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> -		}
> -	}
> -cleanup:
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -/*
> - * Does chmod for an inode that may have an Access Control List. The
> - * inode->i_mode field must be updated to the desired value by the caller
> - * before calling this function.
> - * Returns 0 on success, or a negative error number.
> - *
> - * We change the ACL rather than storing some ACL entries in the file
> - * mode permission bits (which would be more efficient), because that
> - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> - * for directories) are added. There are no more bits available in the
> - * file mode.
> - *
> - * inode->i_mutex: down
> - */
>  int
> -ext4_acl_chmod(struct inode *inode)
> +ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  {
> -	struct posix_acl *acl;
>  	handle_t *handle;
> -	int retries = 0;
> -	int error;
> -
> +	int error, retries = 0;
>  
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -	if (!test_opt(inode->i_sb, POSIX_ACL))
> -		return 0;
> -	acl = ext4_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl) || !acl)
> -		return PTR_ERR(acl);
> -	error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> -	if (error)
> -		return error;
>  retry:
>  	handle = ext4_journal_start(inode, EXT4_HT_XATTR,
>  				    ext4_jbd2_credits_xattr(inode));
> -	if (IS_ERR(handle)) {
> -		error = PTR_ERR(handle);
> -		ext4_std_error(inode->i_sb, error);
> -		goto out;
> -	}
> -	error = ext4_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +
> +	error = __ext4_set_acl(handle, inode, type, acl);
>  	ext4_journal_stop(handle);
> -	if (error == -ENOSPC &&
> -	    ext4_should_retry_alloc(inode->i_sb, &retries))
> +	if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;
> -out:
> -	posix_acl_release(acl);
>  	return error;
>  }
>  
>  /*
> - * Extended attribute handlers
> + * Initialize the ACLs of a new inode. Called from ext4_new_inode.
> + *
> + * dir->i_mutex: down
> + * inode->i_mutex: up (access to inode is still exclusive)
>   */
> -static size_t
> -ext4_xattr_list_acl_access(struct dentry *dentry, char *list, size_t list_len,
> -			   const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_len)
> -		memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> -	return size;
> -}
> -
> -static size_t
> -ext4_xattr_list_acl_default(struct dentry *dentry, char *list, size_t list_len,
> -			    const char *name, size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> -
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return 0;
> -	if (list && size <= list_len)
> -		memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> -	return size;
> -}
> -
> -static int
> -ext4_xattr_get_acl(struct dentry *dentry, const char *name, void *buffer,
> -		   size_t size, int type)
> +int
> +ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
>  {
> -	struct posix_acl *acl;
> +	struct posix_acl *default_acl, *acl;
>  	int error;
>  
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(dentry->d_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -
> -	acl = ext4_get_acl(dentry->d_inode, type);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (acl == NULL)
> -		return -ENODATA;
> -	error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
> -	return error;
> -}
> -
> -static int
> -ext4_xattr_set_acl(struct dentry *dentry, const char *name, const void *value,
> -		   size_t size, int flags, int type)
> -{
> -	struct inode *inode = dentry->d_inode;
> -	handle_t *handle;
> -	struct posix_acl *acl;
> -	int error, retries = 0;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(inode->i_sb, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -	if (!inode_owner_or_capable(inode))
> -		return -EPERM;
> -
> -	if (value) {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl))
> -			return PTR_ERR(acl);
> -		else if (acl) {
> -			error = posix_acl_valid(acl);
> -			if (error)
> -				goto release_and_out;
> -		}
> -	} else
> -		acl = NULL;
> +	error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> +	if (error)
> +		return error;
>  
> -retry:
> -	handle = ext4_journal_start(inode, EXT4_HT_XATTR,
> -				    ext4_jbd2_credits_xattr(inode));
> -	if (IS_ERR(handle)) {
> -		error = PTR_ERR(handle);
> -		goto release_and_out;
> +	if (default_acl) {
> +		error = __ext4_set_acl(handle, inode, ACL_TYPE_DEFAULT,
> +				       default_acl);
> +		posix_acl_release(default_acl);
> +	}
> +	if (acl) {
> +		if (!error)
> +			error = __ext4_set_acl(handle, inode, ACL_TYPE_ACCESS,
> +					       acl);
> +		posix_acl_release(acl);
>  	}
> -	error = ext4_set_acl(handle, inode, type, acl);
> -	ext4_journal_stop(handle);
> -	if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
> -		goto retry;
> -
> -release_and_out:
> -	posix_acl_release(acl);
>  	return error;
>  }
> -
> -const struct xattr_handler ext4_xattr_acl_access_handler = {
> -	.prefix	= POSIX_ACL_XATTR_ACCESS,
> -	.flags	= ACL_TYPE_ACCESS,
> -	.list	= ext4_xattr_list_acl_access,
> -	.get	= ext4_xattr_get_acl,
> -	.set	= ext4_xattr_set_acl,
> -};
> -
> -const struct xattr_handler ext4_xattr_acl_default_handler = {
> -	.prefix	= POSIX_ACL_XATTR_DEFAULT,
> -	.flags	= ACL_TYPE_DEFAULT,
> -	.list	= ext4_xattr_list_acl_default,
> -	.get	= ext4_xattr_get_acl,
> -	.set	= ext4_xattr_set_acl,
> -};
> diff --git a/fs/ext4/acl.h b/fs/ext4/acl.h
> index 18cb39e..da2c795 100644
> --- a/fs/ext4/acl.h
> +++ b/fs/ext4/acl.h
> @@ -55,18 +55,13 @@ static inline int ext4_acl_count(size_t size)
>  
>  /* acl.c */
>  struct posix_acl *ext4_get_acl(struct inode *inode, int type);
> -extern int ext4_acl_chmod(struct inode *);
> +int ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
>  
>  #else  /* CONFIG_EXT4_FS_POSIX_ACL */
>  #include <linux/sched.h>
>  #define ext4_get_acl NULL
> -
> -static inline int
> -ext4_acl_chmod(struct inode *inode)
> -{
> -	return 0;
> -}
> +#define ext4_set_acl NULL
>  
>  static inline int
>  ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 3da2194..43e64f6 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -617,6 +617,7 @@ const struct inode_operations ext4_file_inode_operations = {
>  	.listxattr	= ext4_listxattr,
>  	.removexattr	= generic_removexattr,
>  	.get_acl	= ext4_get_acl,
> +	.set_acl	= ext4_set_acl,
>  	.fiemap		= ext4_fiemap,
>  };
>  
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 0757634..6f69f96 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4675,7 +4675,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
>  		ext4_orphan_del(NULL, inode);
>  
>  	if (!rc && (ia_valid & ATTR_MODE))
> -		rc = ext4_acl_chmod(inode);
> +		rc = posix_acl_chmod(inode);
>  
>  err_out:
>  	ext4_std_error(inode->i_sb, error);
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 5a0408d..e77c1ba 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3225,6 +3225,7 @@ const struct inode_operations ext4_dir_inode_operations = {
>  	.listxattr	= ext4_listxattr,
>  	.removexattr	= generic_removexattr,
>  	.get_acl	= ext4_get_acl,
> +	.set_acl	= ext4_set_acl,
>  	.fiemap         = ext4_fiemap,
>  };
>  
> @@ -3235,4 +3236,5 @@ const struct inode_operations ext4_special_inode_operations = {
>  	.listxattr	= ext4_listxattr,
>  	.removexattr	= generic_removexattr,
>  	.get_acl	= ext4_get_acl,
> +	.set_acl	= ext4_set_acl,
>  };
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 1423c48..e175e94 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -95,8 +95,8 @@ static struct mb_cache *ext4_xattr_cache;
>  static const struct xattr_handler *ext4_xattr_handler_map[] = {
>  	[EXT4_XATTR_INDEX_USER]		     = &ext4_xattr_user_handler,
>  #ifdef CONFIG_EXT4_FS_POSIX_ACL
> -	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &ext4_xattr_acl_access_handler,
> -	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &ext4_xattr_acl_default_handler,
> +	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &posix_acl_access_xattr_handler,
> +	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
>  #endif
>  	[EXT4_XATTR_INDEX_TRUSTED]	     = &ext4_xattr_trusted_handler,
>  #ifdef CONFIG_EXT4_FS_SECURITY
> @@ -108,8 +108,8 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
>  	&ext4_xattr_user_handler,
>  	&ext4_xattr_trusted_handler,
>  #ifdef CONFIG_EXT4_FS_POSIX_ACL
> -	&ext4_xattr_acl_access_handler,
> -	&ext4_xattr_acl_default_handler,
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
>  #endif
>  #ifdef CONFIG_EXT4_FS_SECURITY
>  	&ext4_xattr_security_handler,
> diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
> index c767dbd..819d639 100644
> --- a/fs/ext4/xattr.h
> +++ b/fs/ext4/xattr.h
> @@ -96,8 +96,6 @@ struct ext4_xattr_ibody_find {
>  
>  extern const struct xattr_handler ext4_xattr_user_handler;
>  extern const struct xattr_handler ext4_xattr_trusted_handler;
> -extern const struct xattr_handler ext4_xattr_acl_access_handler;
> -extern const struct xattr_handler ext4_xattr_acl_default_handler;
>  extern const struct xattr_handler ext4_xattr_security_handler;
>  
>  extern ssize_t ext4_listxattr(struct dentry *, char *, size_t);
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 13/18] reiserfs: use generic posix ACL infrastructure

[Jan Kara]

On Sun 01-12-13 03:59:16, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/reiserfs/acl.h       |    4 +-
>  fs/reiserfs/file.c      |    1 +
>  fs/reiserfs/namei.c     |    3 +
>  fs/reiserfs/xattr.c     |    5 +-
>  fs/reiserfs/xattr_acl.c |  175 ++++++++---------------------------------------
>  5 files changed, 36 insertions(+), 152 deletions(-)
> 
> diff --git a/fs/reiserfs/acl.h b/fs/reiserfs/acl.h
> index f096b80..4a211f5 100644
> --- a/fs/reiserfs/acl.h
> +++ b/fs/reiserfs/acl.h
> @@ -48,18 +48,18 @@ static inline int reiserfs_acl_count(size_t size)
>  
>  #ifdef CONFIG_REISERFS_FS_POSIX_ACL
>  struct posix_acl *reiserfs_get_acl(struct inode *inode, int type);
> +int reiserfs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  int reiserfs_acl_chmod(struct inode *inode);
>  int reiserfs_inherit_default_acl(struct reiserfs_transaction_handle *th,
>  				 struct inode *dir, struct dentry *dentry,
>  				 struct inode *inode);
>  int reiserfs_cache_default_acl(struct inode *dir);
> -extern const struct xattr_handler reiserfs_posix_acl_default_handler;
> -extern const struct xattr_handler reiserfs_posix_acl_access_handler;
>  
>  #else
>  
>  #define reiserfs_cache_default_acl(inode) 0
>  #define reiserfs_get_acl NULL
> +#define reiserfs_set_acl NULL
>  
>  static inline int reiserfs_acl_chmod(struct inode *inode)
>  {
> diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c
> index dcaafcf..ed58d84 100644
> --- a/fs/reiserfs/file.c
> +++ b/fs/reiserfs/file.c
> @@ -260,4 +260,5 @@ const struct inode_operations reiserfs_file_inode_operations = {
>  	.removexattr = reiserfs_removexattr,
>  	.permission = reiserfs_permission,
>  	.get_acl = reiserfs_get_acl,
> +	.set_acl = reiserfs_set_acl,
>  };
> diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
> index dc5236f..8ba707e 100644
> --- a/fs/reiserfs/namei.c
> +++ b/fs/reiserfs/namei.c
> @@ -1522,6 +1522,7 @@ const struct inode_operations reiserfs_dir_inode_operations = {
>  	.removexattr = reiserfs_removexattr,
>  	.permission = reiserfs_permission,
>  	.get_acl = reiserfs_get_acl,
> +	.set_acl = reiserfs_set_acl,
>  };
>  
>  /*
> @@ -1539,6 +1540,7 @@ const struct inode_operations reiserfs_symlink_inode_operations = {
>  	.removexattr = reiserfs_removexattr,
>  	.permission = reiserfs_permission,
>  	.get_acl = reiserfs_get_acl,
> +	.set_acl = reiserfs_set_acl,
>  
>  };
>  
> @@ -1553,4 +1555,5 @@ const struct inode_operations reiserfs_special_inode_operations = {
>  	.removexattr = reiserfs_removexattr,
>  	.permission = reiserfs_permission,
>  	.get_acl = reiserfs_get_acl,
> +	.set_acl = reiserfs_set_acl,
>  };
> diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
> index 8a9e2dc..5cdfbd6 100644
> --- a/fs/reiserfs/xattr.c
> +++ b/fs/reiserfs/xattr.c
> @@ -50,6 +50,7 @@
>  #include <linux/stat.h>
>  #include <linux/quotaops.h>
>  #include <linux/security.h>
> +#include <linux/posix_acl_xattr.h>
>  
>  #define PRIVROOT_NAME ".reiserfs_priv"
>  #define XAROOT_NAME   "xattrs"
> @@ -904,8 +905,8 @@ static const struct xattr_handler *reiserfs_xattr_handlers[] = {
>  	&reiserfs_xattr_security_handler,
>  #endif
>  #ifdef CONFIG_REISERFS_FS_POSIX_ACL
> -	&reiserfs_posix_acl_access_handler,
> -	&reiserfs_posix_acl_default_handler,
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
>  #endif
>  	NULL
>  };
> diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
> index d95c959..ff04988 100644
> --- a/fs/reiserfs/xattr_acl.c
> +++ b/fs/reiserfs/xattr_acl.c
> @@ -11,35 +11,19 @@
>  #include "acl.h"
>  #include <asm/uaccess.h>
>  
> -static int reiserfs_set_acl(struct reiserfs_transaction_handle *th,
> +static int __reiserfs_set_acl(struct reiserfs_transaction_handle *th,
>  			    struct inode *inode, int type,
>  			    struct posix_acl *acl);
>  
> -static int
> -reiserfs_posix_acl_set(struct dentry *dentry, const char *name, const void *value,
> -		size_t size, int flags, int type)
> +
> +int
> +reiserfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  {
> -	struct inode *inode = dentry->d_inode;
> -	struct posix_acl *acl;
>  	int error, error2;
>  	struct reiserfs_transaction_handle th;
>  	size_t jcreate_blocks;
> -	if (!reiserfs_posixacl(inode->i_sb))
> -		return -EOPNOTSUPP;
> -	if (!inode_owner_or_capable(inode))
> -		return -EPERM;
> -
> -	if (value) {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl)) {
> -			return PTR_ERR(acl);
> -		} else if (acl) {
> -			error = posix_acl_valid(acl);
> -			if (error)
> -				goto release_and_out;
> -		}
> -	} else
> -		acl = NULL;
> +	int size = acl ? posix_acl_xattr_size(acl->a_count) : 0;
> +
>  
>  	/* Pessimism: We can't assume that anything from the xattr root up
>  	 * has been created. */
> @@ -51,7 +35,7 @@ reiserfs_posix_acl_set(struct dentry *dentry, const char *name, const void *valu
>  	error = journal_begin(&th, inode->i_sb, jcreate_blocks);
>  	reiserfs_write_unlock(inode->i_sb);
>  	if (error == 0) {
> -		error = reiserfs_set_acl(&th, inode, type, acl);
> +		error = __reiserfs_set_acl(&th, inode, type, acl);
>  		reiserfs_write_lock(inode->i_sb);
>  		error2 = journal_end(&th, inode->i_sb, jcreate_blocks);
>  		reiserfs_write_unlock(inode->i_sb);
> @@ -59,29 +43,6 @@ reiserfs_posix_acl_set(struct dentry *dentry, const char *name, const void *valu
>  			error = error2;
>  	}
>  
> -      release_and_out:
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -static int
> -reiserfs_posix_acl_get(struct dentry *dentry, const char *name, void *buffer,
> -		size_t size, int type)
> -{
> -	struct posix_acl *acl;
> -	int error;
> -
> -	if (!reiserfs_posixacl(dentry->d_sb))
> -		return -EOPNOTSUPP;
> -
> -	acl = reiserfs_get_acl(dentry->d_inode, type);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (acl == NULL)
> -		return -ENODATA;
> -	error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
>  	return error;
>  }
>  
> @@ -273,7 +234,7 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type)
>   * BKL held [before 2.5.x]
>   */
>  static int
> -reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode,
> +__reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode,
>  		 int type, struct posix_acl *acl)
>  {
>  	char *name;
> @@ -343,7 +304,7 @@ reiserfs_inherit_default_acl(struct reiserfs_transaction_handle *th,
>  			     struct inode *dir, struct dentry *dentry,
>  			     struct inode *inode)
>  {
> -	struct posix_acl *acl;
> +	struct posix_acl *default_acl, *acl;
>  	int err = 0;
>  
>  	/* ACLs only get applied to files and directories */
> @@ -363,37 +324,28 @@ reiserfs_inherit_default_acl(struct reiserfs_transaction_handle *th,
>  		goto apply_umask;
>  	}
>  
> -	acl = reiserfs_get_acl(dir, ACL_TYPE_DEFAULT);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> +	err = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> +	if (err)
> +		return err;
>  
> +	if (default_acl) {
> +		err = __reiserfs_set_acl(th, inode, ACL_TYPE_DEFAULT,
> +					 default_acl);
> +		posix_acl_release(default_acl);
> +	}
>  	if (acl) {
> -		/* Copy the default ACL to the default ACL of a new directory */
> -		if (S_ISDIR(inode->i_mode)) {
> -			err = reiserfs_set_acl(th, inode, ACL_TYPE_DEFAULT,
> -					       acl);
> -			if (err)
> -				goto cleanup;
> -		}
> -
> -		/* Now we reconcile the new ACL and the mode,
> -		   potentially modifying both */
> -		err = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> -		if (err < 0)
> -			return err;
> -
> -		/* If we need an ACL.. */
> -		if (err > 0)
> -			err = reiserfs_set_acl(th, inode, ACL_TYPE_ACCESS, acl);
> -	      cleanup:
> +		if (!err)
> +			err = __reiserfs_set_acl(th, inode, ACL_TYPE_ACCESS,
> +						 acl);
>  		posix_acl_release(acl);
> -	} else {
> -	      apply_umask:
> -		/* no ACL, apply umask */
> -		inode->i_mode &= ~current_umask();
>  	}
>  
>  	return err;
> +
> +      apply_umask:
> +	/* no ACL, apply umask */
> +	inode->i_mode &= ~current_umask();
> +	return err;
>  }
>  
>  /* This is used to cache the default acl before a new object is created.
> @@ -442,84 +394,11 @@ int reiserfs_cache_default_acl(struct inode *inode)
>   */
>  int reiserfs_acl_chmod(struct inode *inode)
>  {
> -	struct reiserfs_transaction_handle th;
> -	struct posix_acl *acl;
> -	size_t size;
> -	int error;
> -
>  	if (IS_PRIVATE(inode))
>  		return 0;
> -
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -
>  	if (get_inode_sd_version(inode) == STAT_DATA_V1 ||
> -	    !reiserfs_posixacl(inode->i_sb)) {
> +	    !reiserfs_posixacl(inode->i_sb))
>  		return 0;
> -	}
>  
> -	acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (!acl)
> -		return 0;
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	error = __posix_acl_chmod(&acl, GFP_NOFS, inode->i_mode);
> -	if (error)
> -		return error;
> -
> -	size = reiserfs_xattr_nblocks(inode, reiserfs_acl_size(acl->a_count));
> -	reiserfs_write_lock(inode->i_sb);
> -	error = journal_begin(&th, inode->i_sb, size * 2);
> -	reiserfs_write_unlock(inode->i_sb);
> -	if (!error) {
> -		int error2;
> -		error = reiserfs_set_acl(&th, inode, ACL_TYPE_ACCESS, acl);
> -		reiserfs_write_lock(inode->i_sb);
> -		error2 = journal_end(&th, inode->i_sb, size * 2);
> -		reiserfs_write_unlock(inode->i_sb);
> -		if (error2)
> -			error = error2;
> -	}
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -static size_t posix_acl_access_list(struct dentry *dentry, char *list,
> -				    size_t list_size, const char *name,
> -				    size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> -	if (!reiserfs_posixacl(dentry->d_sb))
> -		return 0;
> -	if (list && size <= list_size)
> -		memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> -	return size;
> +	return posix_acl_chmod(inode);
>  }
> -
> -const struct xattr_handler reiserfs_posix_acl_access_handler = {
> -	.prefix = POSIX_ACL_XATTR_ACCESS,
> -	.flags = ACL_TYPE_ACCESS,
> -	.get = reiserfs_posix_acl_get,
> -	.set = reiserfs_posix_acl_set,
> -	.list = posix_acl_access_list,
> -};
> -
> -static size_t posix_acl_default_list(struct dentry *dentry, char *list,
> -				     size_t list_size, const char *name,
> -				     size_t name_len, int type)
> -{
> -	const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> -	if (!reiserfs_posixacl(dentry->d_sb))
> -		return 0;
> -	if (list && size <= list_size)
> -		memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> -	return size;
> -}
> -
> -const struct xattr_handler reiserfs_posix_acl_default_handler = {
> -	.prefix = POSIX_ACL_XATTR_DEFAULT,
> -	.flags = ACL_TYPE_DEFAULT,
> -	.get = reiserfs_posix_acl_get,
> -	.set = reiserfs_posix_acl_set,
> -	.list = posix_acl_default_list,
> -};
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [Jfs-discussion] [PATCH 08/18] ext2/3/4: use generic posix ACL infrastructure

[gary sheppard]

Really nice to see all the sudden activity here in JFS again!


On Mon, Dec 2, 2013 at 2:13 PM, Jan Kara <jack@suse.cz> wrote:

> On Sun 01-12-13 03:59:11, Christoph Hellwig wrote:
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>   Looks good. You can add:
> Reviewed-by: Jan Kara <jack@suse.cz>
>
>                                                         Honza
>
> > ---
> >  fs/ext2/acl.c   |  176 ++++-----------------------------------------
> >  fs/ext2/acl.h   |    8 +--
> >  fs/ext2/file.c  |    1 +
> >  fs/ext2/inode.c |    2 +-
> >  fs/ext2/namei.c |    2 +
> >  fs/ext2/xattr.c |    8 +--
> >  fs/ext2/xattr.h |    2 -
> >  fs/ext3/acl.c   |  213
> ++++++++-----------------------------------------------
> >  fs/ext3/acl.h   |    9 +--
> >  fs/ext3/file.c  |    1 +
> >  fs/ext3/inode.c |    2 +-
> >  fs/ext3/namei.c |    2 +
> >  fs/ext3/xattr.c |    8 +--
> >  fs/ext3/xattr.h |    2 -
> >  fs/ext4/acl.c   |  213
> ++++++++-----------------------------------------------
> >  fs/ext4/acl.h   |    9 +--
> >  fs/ext4/file.c  |    1 +
> >  fs/ext4/inode.c |    2 +-
> >  fs/ext4/namei.c |    2 +
> >  fs/ext4/xattr.c |    8 +--
> >  fs/ext4/xattr.h |    2 -
> >  21 files changed, 100 insertions(+), 573 deletions(-)
> >
> > diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c
> > index 6e842a7..f04a295 100644
> > --- a/fs/ext2/acl.c
> > +++ b/fs/ext2/acl.c
> > @@ -189,8 +189,8 @@ ext2_get_acl(struct inode *inode, int type)
> >  /*
> >   * inode->i_mutex: down
> >   */
> > -static int
> > -ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl)
> > +int
> > +ext2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> >  {
> >       int name_index;
> >       void *value = NULL;
> > @@ -250,169 +250,21 @@ ext2_set_acl(struct inode *inode, int type,
> struct posix_acl *acl)
> >  int
> >  ext2_init_acl(struct inode *inode, struct inode *dir)
> >  {
> > -     struct posix_acl *acl = NULL;
> > -     int error = 0;
> > -
> > -     if (!S_ISLNK(inode->i_mode)) {
> > -             if (test_opt(dir->i_sb, POSIX_ACL)) {
> > -                     acl = ext2_get_acl(dir, ACL_TYPE_DEFAULT);
> > -                     if (IS_ERR(acl))
> > -                             return PTR_ERR(acl);
> > -             }
> > -             if (!acl)
> > -                     inode->i_mode &= ~current_umask();
> > -     }
> > -     if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> > -             if (S_ISDIR(inode->i_mode)) {
> > -                     error = ext2_set_acl(inode, ACL_TYPE_DEFAULT, acl);
> > -                     if (error)
> > -                             goto cleanup;
> > -             }
> > -             error = __posix_acl_create(&acl, GFP_KERNEL,
> &inode->i_mode);
> > -             if (error < 0)
> > -                     return error;
> > -             if (error > 0) {
> > -                     /* This is an extended ACL */
> > -                     error = ext2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> > -             }
> > -     }
> > -cleanup:
> > -       posix_acl_release(acl);
> > -       return error;
> > -}
> > -
> > -/*
> > - * Does chmod for an inode that may have an Access Control List. The
> > - * inode->i_mode field must be updated to the desired value by the
> caller
> > - * before calling this function.
> > - * Returns 0 on success, or a negative error number.
> > - *
> > - * We change the ACL rather than storing some ACL entries in the file
> > - * mode permission bits (which would be more efficient), because that
> > - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> > - * for directories) are added. There are no more bits available in the
> > - * file mode.
> > - *
> > - * inode->i_mutex: down
> > - */
> > -int
> > -ext2_acl_chmod(struct inode *inode)
> > -{
> > -     struct posix_acl *acl;
> > -        int error;
> > +     struct posix_acl *default_acl, *acl;
> > +     int error;
> >
> > -     if (!test_opt(inode->i_sb, POSIX_ACL))
> > -             return 0;
> > -     if (S_ISLNK(inode->i_mode))
> > -             return -EOPNOTSUPP;
> > -     acl = ext2_get_acl(inode, ACL_TYPE_ACCESS);
> > -     if (IS_ERR(acl) || !acl)
> > -             return PTR_ERR(acl);
> > -     error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> > +     error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> >       if (error)
> >               return error;
> > -     error = ext2_set_acl(inode, ACL_TYPE_ACCESS, acl);
> > -     posix_acl_release(acl);
> > -     return error;
> > -}
> > -
> > -/*
> > - * Extended attribut handlers
> > - */
> > -static size_t
> > -ext2_xattr_list_acl_access(struct dentry *dentry, char *list, size_t
> list_size,
> > -                        const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_size)
> > -             memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> > -     return size;
> > -}
> >
> > -static size_t
> > -ext2_xattr_list_acl_default(struct dentry *dentry, char *list, size_t
> list_size,
> > -                         const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_size)
> > -             memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> > -     return size;
> > -}
> > -
> > -static int
> > -ext2_xattr_get_acl(struct dentry *dentry, const char *name, void
> *buffer,
> > -                size_t size, int type)
> > -{
> > -     struct posix_acl *acl;
> > -     int error;
> > -
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -
> > -     acl = ext2_get_acl(dentry->d_inode, type);
> > -     if (IS_ERR(acl))
> > -             return PTR_ERR(acl);
> > -     if (acl == NULL)
> > -             return -ENODATA;
> > -     error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> > -     posix_acl_release(acl);
> > -
> > -     return error;
> > -}
> > -
> > -static int
> > -ext2_xattr_set_acl(struct dentry *dentry, const char *name, const void
> *value,
> > -                size_t size, int flags, int type)
> > -{
> > -     struct posix_acl *acl;
> > -     int error;
> > -
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -     if (!inode_owner_or_capable(dentry->d_inode))
> > -             return -EPERM;
> > -
> > -     if (value) {
> > -             acl = posix_acl_from_xattr(&init_user_ns, value, size);
> > -             if (IS_ERR(acl))
> > -                     return PTR_ERR(acl);
> > -             else if (acl) {
> > -                     error = posix_acl_valid(acl);
> > -                     if (error)
> > -                             goto release_and_out;
> > -             }
> > -     } else
> > -             acl = NULL;
> > -
> > -     error = ext2_set_acl(dentry->d_inode, type, acl);
> > -
> > -release_and_out:
> > -     posix_acl_release(acl);
> > +     if (default_acl) {
> > +             error = ext2_set_acl(inode, default_acl, ACL_TYPE_DEFAULT);
> > +             posix_acl_release(default_acl);
> > +     }
> > +     if (acl) {
> > +             if (!error)
> > +                     error = ext2_set_acl(inode, acl, ACL_TYPE_ACCESS);
> > +             posix_acl_release(acl);
> > +     }
> >       return error;
> >  }
> > -
> > -const struct xattr_handler ext2_xattr_acl_access_handler = {
> > -     .prefix = POSIX_ACL_XATTR_ACCESS,
> > -     .flags  = ACL_TYPE_ACCESS,
> > -     .list   = ext2_xattr_list_acl_access,
> > -     .get    = ext2_xattr_get_acl,
> > -     .set    = ext2_xattr_set_acl,
> > -};
> > -
> > -const struct xattr_handler ext2_xattr_acl_default_handler = {
> > -     .prefix = POSIX_ACL_XATTR_DEFAULT,
> > -     .flags  = ACL_TYPE_DEFAULT,
> > -     .list   = ext2_xattr_list_acl_default,
> > -     .get    = ext2_xattr_get_acl,
> > -     .set    = ext2_xattr_set_acl,
> > -};
> > diff --git a/fs/ext2/acl.h b/fs/ext2/acl.h
> > index 503bfb0..44937f9 100644
> > --- a/fs/ext2/acl.h
> > +++ b/fs/ext2/acl.h
> > @@ -55,7 +55,7 @@ static inline int ext2_acl_count(size_t size)
> >
> >  /* acl.c */
> >  extern struct posix_acl *ext2_get_acl(struct inode *inode, int type);
> > -extern int ext2_acl_chmod (struct inode *);
> > +extern int ext2_set_acl(struct inode *inode, struct posix_acl *acl, int
> type);
> >  extern int ext2_init_acl (struct inode *, struct inode *);
> >
> >  #else
> > @@ -63,12 +63,6 @@ extern int ext2_init_acl (struct inode *, struct
> inode *);
> >  #define ext2_get_acl NULL
> >  #define ext2_set_acl NULL
> >
> > -static inline int
> > -ext2_acl_chmod (struct inode *inode)
> > -{
> > -     return 0;
> > -}
> > -
> >  static inline int ext2_init_acl (struct inode *inode, struct inode *dir)
> >  {
> >       return 0;
> > diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> > index a5b3a5d..44c36e5 100644
> > --- a/fs/ext2/file.c
> > +++ b/fs/ext2/file.c
> > @@ -103,5 +103,6 @@ const struct inode_operations
> ext2_file_inode_operations = {
> >  #endif
> >       .setattr        = ext2_setattr,
> >       .get_acl        = ext2_get_acl,
> > +     .set_acl        = ext2_set_acl,
> >       .fiemap         = ext2_fiemap,
> >  };
> > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> > index 8a33764..1be8866 100644
> > --- a/fs/ext2/inode.c
> > +++ b/fs/ext2/inode.c
> > @@ -1566,7 +1566,7 @@ int ext2_setattr(struct dentry *dentry, struct
> iattr *iattr)
> >       }
> >       setattr_copy(inode, iattr);
> >       if (iattr->ia_valid & ATTR_MODE)
> > -             error = ext2_acl_chmod(inode);
> > +             error = posix_acl_chmod(inode);
> >       mark_inode_dirty(inode);
> >
> >       return error;
> > diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
> > index 256dd5f..c268d0a 100644
> > --- a/fs/ext2/namei.c
> > +++ b/fs/ext2/namei.c
> > @@ -421,6 +421,7 @@ const struct inode_operations
> ext2_dir_inode_operations = {
> >  #endif
> >       .setattr        = ext2_setattr,
> >       .get_acl        = ext2_get_acl,
> > +     .set_acl        = ext2_set_acl,
> >       .tmpfile        = ext2_tmpfile,
> >  };
> >
> > @@ -433,4 +434,5 @@ const struct inode_operations
> ext2_special_inode_operations = {
> >  #endif
> >       .setattr        = ext2_setattr,
> >       .get_acl        = ext2_get_acl,
> > +     .set_acl        = ext2_set_acl,
> >  };
> > diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> > index 2d7557d..9142614 100644
> > --- a/fs/ext2/xattr.c
> > +++ b/fs/ext2/xattr.c
> > @@ -103,8 +103,8 @@ static struct mb_cache *ext2_xattr_cache;
> >  static const struct xattr_handler *ext2_xattr_handler_map[] = {
> >       [EXT2_XATTR_INDEX_USER]              = &ext2_xattr_user_handler,
> >  #ifdef CONFIG_EXT2_FS_POSIX_ACL
> > -     [EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &ext2_xattr_acl_access_handler,
> > -     [EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &ext2_xattr_acl_default_handler,
> > +     [EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &posix_acl_access_xattr_handler,
> > +     [EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &posix_acl_default_xattr_handler,
> >  #endif
> >       [EXT2_XATTR_INDEX_TRUSTED]           = &ext2_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT2_FS_SECURITY
> > @@ -116,8 +116,8 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
> >       &ext2_xattr_user_handler,
> >       &ext2_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT2_FS_POSIX_ACL
> > -     &ext2_xattr_acl_access_handler,
> > -     &ext2_xattr_acl_default_handler,
> > +     &posix_acl_access_xattr_handler,
> > +     &posix_acl_default_xattr_handler,
> >  #endif
> >  #ifdef CONFIG_EXT2_FS_SECURITY
> >       &ext2_xattr_security_handler,
> > diff --git a/fs/ext2/xattr.h b/fs/ext2/xattr.h
> > index 5e41ccc..60edf29 100644
> > --- a/fs/ext2/xattr.h
> > +++ b/fs/ext2/xattr.h
> > @@ -57,8 +57,6 @@ struct ext2_xattr_entry {
> >
> >  extern const struct xattr_handler ext2_xattr_user_handler;
> >  extern const struct xattr_handler ext2_xattr_trusted_handler;
> > -extern const struct xattr_handler ext2_xattr_acl_access_handler;
> > -extern const struct xattr_handler ext2_xattr_acl_default_handler;
> >  extern const struct xattr_handler ext2_xattr_security_handler;
> >
> >  extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
> > diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
> > index 4f3d8fa..e9cb33f 100644
> > --- a/fs/ext3/acl.c
> > +++ b/fs/ext3/acl.c
> > @@ -190,7 +190,7 @@ ext3_get_acl(struct inode *inode, int type)
> >   * inode->i_mutex: down unless called from ext3_new_inode
> >   */
> >  static int
> > -ext3_set_acl(handle_t *handle, struct inode *inode, int type,
> > +__ext3_set_acl(handle_t *handle, struct inode *inode, int type,
> >            struct posix_acl *acl)
> >  {
> >       int name_index;
> > @@ -243,204 +243,49 @@ ext3_set_acl(handle_t *handle, struct inode
> *inode, int type,
> >       return error;
> >  }
> >
> > -/*
> > - * Initialize the ACLs of a new inode. Called from ext3_new_inode.
> > - *
> > - * dir->i_mutex: down
> > - * inode->i_mutex: up (access to inode is still exclusive)
> > - */
> > -int
> > -ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> > -{
> > -     struct posix_acl *acl = NULL;
> > -     int error = 0;
> > -
> > -     if (!S_ISLNK(inode->i_mode)) {
> > -             if (test_opt(dir->i_sb, POSIX_ACL)) {
> > -                     acl = ext3_get_acl(dir, ACL_TYPE_DEFAULT);
> > -                     if (IS_ERR(acl))
> > -                             return PTR_ERR(acl);
> > -             }
> > -             if (!acl)
> > -                     inode->i_mode &= ~current_umask();
> > -     }
> > -     if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> > -             if (S_ISDIR(inode->i_mode)) {
> > -                     error = ext3_set_acl(handle, inode,
> > -                                          ACL_TYPE_DEFAULT, acl);
> > -                     if (error)
> > -                             goto cleanup;
> > -             }
> > -             error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> > -             if (error < 0)
> > -                     return error;
> > -
> > -             if (error > 0) {
> > -                     /* This is an extended ACL */
> > -                     error = ext3_set_acl(handle, inode,
> ACL_TYPE_ACCESS, acl);
> > -             }
> > -     }
> > -cleanup:
> > -     posix_acl_release(acl);
> > -     return error;
> > -}
> > -
> > -/*
> > - * Does chmod for an inode that may have an Access Control List. The
> > - * inode->i_mode field must be updated to the desired value by the
> caller
> > - * before calling this function.
> > - * Returns 0 on success, or a negative error number.
> > - *
> > - * We change the ACL rather than storing some ACL entries in the file
> > - * mode permission bits (which would be more efficient), because that
> > - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> > - * for directories) are added. There are no more bits available in the
> > - * file mode.
> > - *
> > - * inode->i_mutex: down
> > - */
> >  int
> > -ext3_acl_chmod(struct inode *inode)
> > +ext3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> >  {
> > -     struct posix_acl *acl;
> >       handle_t *handle;
> > -     int retries = 0;
> > -        int error;
> > +     int error, retries = 0;
> >
> > -     if (S_ISLNK(inode->i_mode))
> > -             return -EOPNOTSUPP;
> > -     if (!test_opt(inode->i_sb, POSIX_ACL))
> > -             return 0;
> > -     acl = ext3_get_acl(inode, ACL_TYPE_ACCESS);
> > -     if (IS_ERR(acl) || !acl)
> > -             return PTR_ERR(acl);
> > -     error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> > -     if (error)
> > -             return error;
> >  retry:
> > -     handle = ext3_journal_start(inode,
> > -                     EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> > -     if (IS_ERR(handle)) {
> > -             error = PTR_ERR(handle);
> > -             ext3_std_error(inode->i_sb, error);
> > -             goto out;
> > -     }
> > -     error = ext3_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> > +     handle = ext3_journal_start(inode,
> EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> > +     if (IS_ERR(handle))
> > +             return PTR_ERR(handle);
> > +     error = __ext3_set_acl(handle, inode, type, acl);
> >       ext3_journal_stop(handle);
> > -     if (error == -ENOSPC &&
> > -         ext3_should_retry_alloc(inode->i_sb, &retries))
> > +     if (error == -ENOSPC && ext3_should_retry_alloc(inode->i_sb,
> &retries))
> >               goto retry;
> > -out:
> > -     posix_acl_release(acl);
> >       return error;
> >  }
> >
> >  /*
> > - * Extended attribute handlers
> > + * Initialize the ACLs of a new inode. Called from ext3_new_inode.
> > + *
> > + * dir->i_mutex: down
> > + * inode->i_mutex: up (access to inode is still exclusive)
> >   */
> > -static size_t
> > -ext3_xattr_list_acl_access(struct dentry *dentry, char *list, size_t
> list_len,
> > -                        const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_len)
> > -             memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> > -     return size;
> > -}
> > -
> > -static size_t
> > -ext3_xattr_list_acl_default(struct dentry *dentry, char *list, size_t
> list_len,
> > -                         const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_len)
> > -             memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> > -     return size;
> > -}
> > -
> > -static int
> > -ext3_xattr_get_acl(struct dentry *dentry, const char *name, void
> *buffer,
> > -                size_t size, int type)
> > +int
> > +ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> >  {
> > -     struct posix_acl *acl;
> > +     struct posix_acl *default_acl, *acl;
> >       int error;
> >
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -
> > -     acl = ext3_get_acl(dentry->d_inode, type);
> > -     if (IS_ERR(acl))
> > -             return PTR_ERR(acl);
> > -     if (acl == NULL)
> > -             return -ENODATA;
> > -     error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> > -     posix_acl_release(acl);
> > -
> > -     return error;
> > -}
> > -
> > -static int
> > -ext3_xattr_set_acl(struct dentry *dentry, const char *name, const void
> *value,
> > -                size_t size, int flags, int type)
> > -{
> > -     struct inode *inode = dentry->d_inode;
> > -     handle_t *handle;
> > -     struct posix_acl *acl;
> > -     int error, retries = 0;
> > -
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(inode->i_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -     if (!inode_owner_or_capable(inode))
> > -             return -EPERM;
> > -
> > -     if (value) {
> > -             acl = posix_acl_from_xattr(&init_user_ns, value, size);
> > -             if (IS_ERR(acl))
> > -                     return PTR_ERR(acl);
> > -             else if (acl) {
> > -                     error = posix_acl_valid(acl);
> > -                     if (error)
> > -                             goto release_and_out;
> > -             }
> > -     } else
> > -             acl = NULL;
> > -
> > -retry:
> > -     handle = ext3_journal_start(inode,
> EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
> > -     if (IS_ERR(handle))
> > -             return PTR_ERR(handle);
> > -     error = ext3_set_acl(handle, inode, type, acl);
> > -     ext3_journal_stop(handle);
> > -     if (error == -ENOSPC && ext3_should_retry_alloc(inode->i_sb,
> &retries))
> > -             goto retry;
> > +     error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> > +     if (error)
> > +             return error;
> >
> > -release_and_out:
> > -     posix_acl_release(acl);
> > +     if (default_acl) {
> > +             error = __ext3_set_acl(handle, inode, ACL_TYPE_DEFAULT,
> > +                                    default_acl);
> > +             posix_acl_release(default_acl);
> > +     }
> > +     if (acl) {
> > +             if (!error)
> > +                     error = __ext3_set_acl(handle, inode,
> ACL_TYPE_ACCESS,
> > +                                            acl);
> > +             posix_acl_release(acl);
> > +     }
> >       return error;
> >  }
> > -
> > -const struct xattr_handler ext3_xattr_acl_access_handler = {
> > -     .prefix = POSIX_ACL_XATTR_ACCESS,
> > -     .flags  = ACL_TYPE_ACCESS,
> > -     .list   = ext3_xattr_list_acl_access,
> > -     .get    = ext3_xattr_get_acl,
> > -     .set    = ext3_xattr_set_acl,
> > -};
> > -
> > -const struct xattr_handler ext3_xattr_acl_default_handler = {
> > -     .prefix = POSIX_ACL_XATTR_DEFAULT,
> > -     .flags  = ACL_TYPE_DEFAULT,
> > -     .list   = ext3_xattr_list_acl_default,
> > -     .get    = ext3_xattr_get_acl,
> > -     .set    = ext3_xattr_set_acl,
> > -};
> > diff --git a/fs/ext3/acl.h b/fs/ext3/acl.h
> > index dbc921e..ea1c69e 100644
> > --- a/fs/ext3/acl.h
> > +++ b/fs/ext3/acl.h
> > @@ -55,18 +55,13 @@ static inline int ext3_acl_count(size_t size)
> >
> >  /* acl.c */
> >  extern struct posix_acl *ext3_get_acl(struct inode *inode, int type);
> > -extern int ext3_acl_chmod (struct inode *);
> > +extern int ext3_set_acl(struct inode *inode, struct posix_acl *acl, int
> type);
> >  extern int ext3_init_acl (handle_t *, struct inode *, struct inode *);
> >
> >  #else  /* CONFIG_EXT3_FS_POSIX_ACL */
> >  #include <linux/sched.h>
> >  #define ext3_get_acl NULL
> > -
> > -static inline int
> > -ext3_acl_chmod(struct inode *inode)
> > -{
> > -     return 0;
> > -}
> > +#define ext3_set_acl NULL
> >
> >  static inline int
> >  ext3_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> > diff --git a/fs/ext3/file.c b/fs/ext3/file.c
> > index 25cb413..aad0531 100644
> > --- a/fs/ext3/file.c
> > +++ b/fs/ext3/file.c
> > @@ -75,6 +75,7 @@ const struct inode_operations
> ext3_file_inode_operations = {
> >       .removexattr    = generic_removexattr,
> >  #endif
> >       .get_acl        = ext3_get_acl,
> > +     .set_acl        = ext3_set_acl,
> >       .fiemap         = ext3_fiemap,
> >  };
> >
> > diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> > index 2bd8548..150b6c1 100644
> > --- a/fs/ext3/inode.c
> > +++ b/fs/ext3/inode.c
> > @@ -3365,7 +3365,7 @@ int ext3_setattr(struct dentry *dentry, struct
> iattr *attr)
> >       mark_inode_dirty(inode);
> >
> >       if (ia_valid & ATTR_MODE)
> > -             rc = ext3_acl_chmod(inode);
> > +             rc = posix_acl_chmod(inode);
> >
> >  err_out:
> >       ext3_std_error(inode->i_sb, error);
> > diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> > index f8cde46..f197736 100644
> > --- a/fs/ext3/namei.c
> > +++ b/fs/ext3/namei.c
> > @@ -2569,6 +2569,7 @@ const struct inode_operations
> ext3_dir_inode_operations = {
> >       .removexattr    = generic_removexattr,
> >  #endif
> >       .get_acl        = ext3_get_acl,
> > +     .set_acl        = ext3_set_acl,
> >  };
> >
> >  const struct inode_operations ext3_special_inode_operations = {
> > @@ -2580,4 +2581,5 @@ const struct inode_operations
> ext3_special_inode_operations = {
> >       .removexattr    = generic_removexattr,
> >  #endif
> >       .get_acl        = ext3_get_acl,
> > +     .set_acl        = ext3_set_acl,
> >  };
> > diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c
> > index b1fc963..c6874be 100644
> > --- a/fs/ext3/xattr.c
> > +++ b/fs/ext3/xattr.c
> > @@ -102,8 +102,8 @@ static struct mb_cache *ext3_xattr_cache;
> >  static const struct xattr_handler *ext3_xattr_handler_map[] = {
> >       [EXT3_XATTR_INDEX_USER]              = &ext3_xattr_user_handler,
> >  #ifdef CONFIG_EXT3_FS_POSIX_ACL
> > -     [EXT3_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &ext3_xattr_acl_access_handler,
> > -     [EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &ext3_xattr_acl_default_handler,
> > +     [EXT3_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &posix_acl_access_xattr_handler,
> > +     [EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &posix_acl_default_xattr_handler,
> >  #endif
> >       [EXT3_XATTR_INDEX_TRUSTED]           = &ext3_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT3_FS_SECURITY
> > @@ -115,8 +115,8 @@ const struct xattr_handler *ext3_xattr_handlers[] = {
> >       &ext3_xattr_user_handler,
> >       &ext3_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT3_FS_POSIX_ACL
> > -     &ext3_xattr_acl_access_handler,
> > -     &ext3_xattr_acl_default_handler,
> > +     &posix_acl_access_xattr_handler,
> > +     &posix_acl_default_xattr_handler,
> >  #endif
> >  #ifdef CONFIG_EXT3_FS_SECURITY
> >       &ext3_xattr_security_handler,
> > diff --git a/fs/ext3/xattr.h b/fs/ext3/xattr.h
> > index 2be4f69..32e93eb 100644
> > --- a/fs/ext3/xattr.h
> > +++ b/fs/ext3/xattr.h
> > @@ -60,8 +60,6 @@ struct ext3_xattr_entry {
> >
> >  extern const struct xattr_handler ext3_xattr_user_handler;
> >  extern const struct xattr_handler ext3_xattr_trusted_handler;
> > -extern const struct xattr_handler ext3_xattr_acl_access_handler;
> > -extern const struct xattr_handler ext3_xattr_acl_default_handler;
> >  extern const struct xattr_handler ext3_xattr_security_handler;
> >
> >  extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
> > diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> > index f827f3b..acaba0f 100644
> > --- a/fs/ext4/acl.c
> > +++ b/fs/ext4/acl.c
> > @@ -196,7 +196,7 @@ ext4_get_acl(struct inode *inode, int type)
> >   * inode->i_mutex: down unless called from ext4_new_inode
> >   */
> >  static int
> > -ext4_set_acl(handle_t *handle, struct inode *inode, int type,
> > +__ext4_set_acl(handle_t *handle, struct inode *inode, int type,
> >            struct posix_acl *acl)
> >  {
> >       int name_index;
> > @@ -248,208 +248,51 @@ ext4_set_acl(handle_t *handle, struct inode
> *inode, int type,
> >       return error;
> >  }
> >
> > -/*
> > - * Initialize the ACLs of a new inode. Called from ext4_new_inode.
> > - *
> > - * dir->i_mutex: down
> > - * inode->i_mutex: up (access to inode is still exclusive)
> > - */
> > -int
> > -ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> > -{
> > -     struct posix_acl *acl = NULL;
> > -     int error = 0;
> > -
> > -     if (!S_ISLNK(inode->i_mode)) {
> > -             if (test_opt(dir->i_sb, POSIX_ACL)) {
> > -                     acl = ext4_get_acl(dir, ACL_TYPE_DEFAULT);
> > -                     if (IS_ERR(acl))
> > -                             return PTR_ERR(acl);
> > -             }
> > -             if (!acl)
> > -                     inode->i_mode &= ~current_umask();
> > -     }
> > -     if (test_opt(inode->i_sb, POSIX_ACL) && acl) {
> > -             if (S_ISDIR(inode->i_mode)) {
> > -                     error = ext4_set_acl(handle, inode,
> > -                                          ACL_TYPE_DEFAULT, acl);
> > -                     if (error)
> > -                             goto cleanup;
> > -             }
> > -             error = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> > -             if (error < 0)
> > -                     return error;
> > -
> > -             if (error > 0) {
> > -                     /* This is an extended ACL */
> > -                     error = ext4_set_acl(handle, inode,
> ACL_TYPE_ACCESS, acl);
> > -             }
> > -     }
> > -cleanup:
> > -     posix_acl_release(acl);
> > -     return error;
> > -}
> > -
> > -/*
> > - * Does chmod for an inode that may have an Access Control List. The
> > - * inode->i_mode field must be updated to the desired value by the
> caller
> > - * before calling this function.
> > - * Returns 0 on success, or a negative error number.
> > - *
> > - * We change the ACL rather than storing some ACL entries in the file
> > - * mode permission bits (which would be more efficient), because that
> > - * would break once additional permissions (like  ACL_APPEND, ACL_DELETE
> > - * for directories) are added. There are no more bits available in the
> > - * file mode.
> > - *
> > - * inode->i_mutex: down
> > - */
> >  int
> > -ext4_acl_chmod(struct inode *inode)
> > +ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> >  {
> > -     struct posix_acl *acl;
> >       handle_t *handle;
> > -     int retries = 0;
> > -     int error;
> > -
> > +     int error, retries = 0;
> >
> > -     if (S_ISLNK(inode->i_mode))
> > -             return -EOPNOTSUPP;
> > -     if (!test_opt(inode->i_sb, POSIX_ACL))
> > -             return 0;
> > -     acl = ext4_get_acl(inode, ACL_TYPE_ACCESS);
> > -     if (IS_ERR(acl) || !acl)
> > -             return PTR_ERR(acl);
> > -     error = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> > -     if (error)
> > -             return error;
> >  retry:
> >       handle = ext4_journal_start(inode, EXT4_HT_XATTR,
> >                                   ext4_jbd2_credits_xattr(inode));
> > -     if (IS_ERR(handle)) {
> > -             error = PTR_ERR(handle);
> > -             ext4_std_error(inode->i_sb, error);
> > -             goto out;
> > -     }
> > -     error = ext4_set_acl(handle, inode, ACL_TYPE_ACCESS, acl);
> > +     if (IS_ERR(handle))
> > +             return PTR_ERR(handle);
> > +
> > +     error = __ext4_set_acl(handle, inode, type, acl);
> >       ext4_journal_stop(handle);
> > -     if (error == -ENOSPC &&
> > -         ext4_should_retry_alloc(inode->i_sb, &retries))
> > +     if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb,
> &retries))
> >               goto retry;
> > -out:
> > -     posix_acl_release(acl);
> >       return error;
> >  }
> >
> >  /*
> > - * Extended attribute handlers
> > + * Initialize the ACLs of a new inode. Called from ext4_new_inode.
> > + *
> > + * dir->i_mutex: down
> > + * inode->i_mutex: up (access to inode is still exclusive)
> >   */
> > -static size_t
> > -ext4_xattr_list_acl_access(struct dentry *dentry, char *list, size_t
> list_len,
> > -                        const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_ACCESS);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_len)
> > -             memcpy(list, POSIX_ACL_XATTR_ACCESS, size);
> > -     return size;
> > -}
> > -
> > -static size_t
> > -ext4_xattr_list_acl_default(struct dentry *dentry, char *list, size_t
> list_len,
> > -                         const char *name, size_t name_len, int type)
> > -{
> > -     const size_t size = sizeof(POSIX_ACL_XATTR_DEFAULT);
> > -
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return 0;
> > -     if (list && size <= list_len)
> > -             memcpy(list, POSIX_ACL_XATTR_DEFAULT, size);
> > -     return size;
> > -}
> > -
> > -static int
> > -ext4_xattr_get_acl(struct dentry *dentry, const char *name, void
> *buffer,
> > -                size_t size, int type)
> > +int
> > +ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> >  {
> > -     struct posix_acl *acl;
> > +     struct posix_acl *default_acl, *acl;
> >       int error;
> >
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(dentry->d_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -
> > -     acl = ext4_get_acl(dentry->d_inode, type);
> > -     if (IS_ERR(acl))
> > -             return PTR_ERR(acl);
> > -     if (acl == NULL)
> > -             return -ENODATA;
> > -     error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> > -     posix_acl_release(acl);
> > -
> > -     return error;
> > -}
> > -
> > -static int
> > -ext4_xattr_set_acl(struct dentry *dentry, const char *name, const void
> *value,
> > -                size_t size, int flags, int type)
> > -{
> > -     struct inode *inode = dentry->d_inode;
> > -     handle_t *handle;
> > -     struct posix_acl *acl;
> > -     int error, retries = 0;
> > -
> > -     if (strcmp(name, "") != 0)
> > -             return -EINVAL;
> > -     if (!test_opt(inode->i_sb, POSIX_ACL))
> > -             return -EOPNOTSUPP;
> > -     if (!inode_owner_or_capable(inode))
> > -             return -EPERM;
> > -
> > -     if (value) {
> > -             acl = posix_acl_from_xattr(&init_user_ns, value, size);
> > -             if (IS_ERR(acl))
> > -                     return PTR_ERR(acl);
> > -             else if (acl) {
> > -                     error = posix_acl_valid(acl);
> > -                     if (error)
> > -                             goto release_and_out;
> > -             }
> > -     } else
> > -             acl = NULL;
> > +     error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> > +     if (error)
> > +             return error;
> >
> > -retry:
> > -     handle = ext4_journal_start(inode, EXT4_HT_XATTR,
> > -                                 ext4_jbd2_credits_xattr(inode));
> > -     if (IS_ERR(handle)) {
> > -             error = PTR_ERR(handle);
> > -             goto release_and_out;
> > +     if (default_acl) {
> > +             error = __ext4_set_acl(handle, inode, ACL_TYPE_DEFAULT,
> > +                                    default_acl);
> > +             posix_acl_release(default_acl);
> > +     }
> > +     if (acl) {
> > +             if (!error)
> > +                     error = __ext4_set_acl(handle, inode,
> ACL_TYPE_ACCESS,
> > +                                            acl);
> > +             posix_acl_release(acl);
> >       }
> > -     error = ext4_set_acl(handle, inode, type, acl);
> > -     ext4_journal_stop(handle);
> > -     if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb,
> &retries))
> > -             goto retry;
> > -
> > -release_and_out:
> > -     posix_acl_release(acl);
> >       return error;
> >  }
> > -
> > -const struct xattr_handler ext4_xattr_acl_access_handler = {
> > -     .prefix = POSIX_ACL_XATTR_ACCESS,
> > -     .flags  = ACL_TYPE_ACCESS,
> > -     .list   = ext4_xattr_list_acl_access,
> > -     .get    = ext4_xattr_get_acl,
> > -     .set    = ext4_xattr_set_acl,
> > -};
> > -
> > -const struct xattr_handler ext4_xattr_acl_default_handler = {
> > -     .prefix = POSIX_ACL_XATTR_DEFAULT,
> > -     .flags  = ACL_TYPE_DEFAULT,
> > -     .list   = ext4_xattr_list_acl_default,
> > -     .get    = ext4_xattr_get_acl,
> > -     .set    = ext4_xattr_set_acl,
> > -};
> > diff --git a/fs/ext4/acl.h b/fs/ext4/acl.h
> > index 18cb39e..da2c795 100644
> > --- a/fs/ext4/acl.h
> > +++ b/fs/ext4/acl.h
> > @@ -55,18 +55,13 @@ static inline int ext4_acl_count(size_t size)
> >
> >  /* acl.c */
> >  struct posix_acl *ext4_get_acl(struct inode *inode, int type);
> > -extern int ext4_acl_chmod(struct inode *);
> > +int ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type);
> >  extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
> >
> >  #else  /* CONFIG_EXT4_FS_POSIX_ACL */
> >  #include <linux/sched.h>
> >  #define ext4_get_acl NULL
> > -
> > -static inline int
> > -ext4_acl_chmod(struct inode *inode)
> > -{
> > -     return 0;
> > -}
> > +#define ext4_set_acl NULL
> >
> >  static inline int
> >  ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
> > diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> > index 3da2194..43e64f6 100644
> > --- a/fs/ext4/file.c
> > +++ b/fs/ext4/file.c
> > @@ -617,6 +617,7 @@ const struct inode_operations
> ext4_file_inode_operations = {
> >       .listxattr      = ext4_listxattr,
> >       .removexattr    = generic_removexattr,
> >       .get_acl        = ext4_get_acl,
> > +     .set_acl        = ext4_set_acl,
> >       .fiemap         = ext4_fiemap,
> >  };
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 0757634..6f69f96 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -4675,7 +4675,7 @@ int ext4_setattr(struct dentry *dentry, struct
> iattr *attr)
> >               ext4_orphan_del(NULL, inode);
> >
> >       if (!rc && (ia_valid & ATTR_MODE))
> > -             rc = ext4_acl_chmod(inode);
> > +             rc = posix_acl_chmod(inode);
> >
> >  err_out:
> >       ext4_std_error(inode->i_sb, error);
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index 5a0408d..e77c1ba 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -3225,6 +3225,7 @@ const struct inode_operations
> ext4_dir_inode_operations = {
> >       .listxattr      = ext4_listxattr,
> >       .removexattr    = generic_removexattr,
> >       .get_acl        = ext4_get_acl,
> > +     .set_acl        = ext4_set_acl,
> >       .fiemap         = ext4_fiemap,
> >  };
> >
> > @@ -3235,4 +3236,5 @@ const struct inode_operations
> ext4_special_inode_operations = {
> >       .listxattr      = ext4_listxattr,
> >       .removexattr    = generic_removexattr,
> >       .get_acl        = ext4_get_acl,
> > +     .set_acl        = ext4_set_acl,
> >  };
> > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> > index 1423c48..e175e94 100644
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -95,8 +95,8 @@ static struct mb_cache *ext4_xattr_cache;
> >  static const struct xattr_handler *ext4_xattr_handler_map[] = {
> >       [EXT4_XATTR_INDEX_USER]              = &ext4_xattr_user_handler,
> >  #ifdef CONFIG_EXT4_FS_POSIX_ACL
> > -     [EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &ext4_xattr_acl_access_handler,
> > -     [EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &ext4_xattr_acl_default_handler,
> > +     [EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  =
> &posix_acl_access_xattr_handler,
> > +     [EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> &posix_acl_default_xattr_handler,
> >  #endif
> >       [EXT4_XATTR_INDEX_TRUSTED]           = &ext4_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT4_FS_SECURITY
> > @@ -108,8 +108,8 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
> >       &ext4_xattr_user_handler,
> >       &ext4_xattr_trusted_handler,
> >  #ifdef CONFIG_EXT4_FS_POSIX_ACL
> > -     &ext4_xattr_acl_access_handler,
> > -     &ext4_xattr_acl_default_handler,
> > +     &posix_acl_access_xattr_handler,
> > +     &posix_acl_default_xattr_handler,
> >  #endif
> >  #ifdef CONFIG_EXT4_FS_SECURITY
> >       &ext4_xattr_security_handler,
> > diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
> > index c767dbd..819d639 100644
> > --- a/fs/ext4/xattr.h
> > +++ b/fs/ext4/xattr.h
> > @@ -96,8 +96,6 @@ struct ext4_xattr_ibody_find {
> >
> >  extern const struct xattr_handler ext4_xattr_user_handler;
> >  extern const struct xattr_handler ext4_xattr_trusted_handler;
> > -extern const struct xattr_handler ext4_xattr_acl_access_handler;
> > -extern const struct xattr_handler ext4_xattr_acl_default_handler;
> >  extern const struct xattr_handler ext4_xattr_security_handler;
> >
> >  extern ssize_t ext4_listxattr(struct dentry *, char *, size_t);
> > --
> > 1.7.10.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel"
> in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics
> Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> _______________________________________________
> Jfs-discussion mailing list
> Jfs-discussion at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jfs-discussion
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131202/c84f7fef/attachment.htm>

[Cluster-devel] [PATCH 12/18] ocfs2: use generic posix ACL infrastructure

[Jan Kara]

On Sun 01-12-13 03:59:15, Christoph Hellwig wrote:
> This contains some major refactoring for the create path so that
> inodes are created with the right mode to start with instead of
> fixing it up later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
...
> -int ocfs2_acl_chmod(struct inode *inode)
> -{
> -	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> -	struct posix_acl *acl;
> -	int ret;
> -
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -
> -	if (!(osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL))
> -		return 0;
> -
> -	acl = ocfs2_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl) || !acl)
> -		return PTR_ERR(acl);
> -	ret = __posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> -	if (ret)
> -		return ret;
> -	ret = ocfs2_set_acl(NULL, inode, NULL, ACL_TYPE_ACCESS,
> -			    acl, NULL, NULL);
> -	posix_acl_release(acl);
> -	return ret;
> -}
...

> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 6fff128..ac371ad 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1236,7 +1236,7 @@ bail:
>  		dqput(transfer_to[qtype]);
>  
>  	if (!status && attr->ia_valid & ATTR_MODE) {
> -		status = ocfs2_acl_chmod(inode);
> +		status = posix_acl_chmod(inode);
>  		if (status < 0)
>  			mlog_errno(status);
>  	}
  Hum, this changes the cluster locking. Previously ocfs2_acl_get() used
from ocfs2_acl_chmod() grabbed cluster wide inode lock. Now getting of ACL
isn't protected by the inode lock. That being said the cluster locking
around setattr looks fishy anyway - if two processes on different
nodes are changing attributes of the same file, changing ACLs post fact
after dropping inode lock could cause interesting effects. Also I'm
wondering how inode_change_ok() can ever be safe without holding inode
lock... Until we grab that other node is free to change e.g. owner of the
inode thus leading even to security implications. But maybe I'm missing
something. Mark, Joel?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Cluster-devel] [PATCH 14/18] xfs: use generic posix ACL infrastructure

[Dave Chinner]

On Sun, Dec 01, 2013 at 03:59:17AM -0800, Christoph Hellwig wrote:
> Also create inodes with the proper mode instead of fixing it up later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Nice cleanup work, Christoph.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david at fromorbit.com

[Cluster-devel] [PATCH 12/18] ocfs2: use generic posix ACL infrastructure

[Christoph Hellwig]

On Tue, Dec 03, 2013 at 12:00:07AM +0100, Jan Kara wrote:
>   Hum, this changes the cluster locking. Previously ocfs2_acl_get() used
> from ocfs2_acl_chmod() grabbed cluster wide inode lock. Now getting of ACL
> isn't protected by the inode lock. That being said the cluster locking
> around setattr looks fishy anyway - if two processes on different
> nodes are changing attributes of the same file, changing ACLs post fact
> after dropping inode lock could cause interesting effects. Also I'm
> wondering how inode_change_ok() can ever be safe without holding inode
> lock... Until we grab that other node is free to change e.g. owner of the
> inode thus leading even to security implications. But maybe I'm missing
> something. Mark, Joel?

Hmm, indeed.  How does ocfs2_iop_get_acl get away without that lock?

Btw, ocfs2 changes will need careful testing as I couldn't find any easy
way to run xfstests on ocfs2 out of the box.

[Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure

[Steven Whitehouse]

Hi,

On Sun, 2013-12-01 at 03:59 -0800, Christoph Hellwig wrote:
> plain text document attachment
> (0016-gfs2-use-generic-posix-ACL-infrastructure.patch)
> This contains some major refactoring for the create path so that
> inodes are created with the right mode to start with instead of
> fixing it up later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/gfs2/acl.c   |  229 +++++++------------------------------------------------
>  fs/gfs2/acl.h   |    4 +-
>  fs/gfs2/inode.c |   33 ++++++--
>  fs/gfs2/xattr.c |    4 +-
>  4 files changed, 61 insertions(+), 209 deletions(-)
> 
Looks very good. I'd really like to be able to do something similar with
the security xattrs, in terms of the refactoring that at inode creation
to give the xattrs ahead of the inode allocation itself. That way it
should be possible to allocate the xattr blocks at the same time as the
inode, rather than as an after thought.

Some more comments below....

> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index e82e4ac..e6c7a2c 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
[snip]
> -
> -static int gfs2_xattr_system_set(struct dentry *dentry, const char *name,
> -				 const void *value, size_t size, int flags,
> -				 int xtype)
> -{
> -	struct inode *inode = dentry->d_inode;
> -	struct gfs2_sbd *sdp = GFS2_SB(inode);
> -	struct posix_acl *acl = NULL;
> -	int error = 0, type;
> -
> -	if (!sdp->sd_args.ar_posix_acl)
> -		return -EOPNOTSUPP;
> -
> -	type = gfs2_acl_type(name);
> -	if (type < 0)
> -		return type;
> -	if (flags & XATTR_CREATE)
> -		return -EINVAL;
> -	if (type == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode))
> -		return value ? -EACCES : 0;
> -	if (!uid_eq(current_fsuid(), inode->i_uid) && !capable(CAP_FOWNER))
> -		return -EPERM;
> -	if (S_ISLNK(inode->i_mode))
> -		return -EOPNOTSUPP;
> -
> -	if (!value)
> -		goto set_acl;
>  
> -	acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -	if (!acl) {
> -		/*
> -		 * acl_set_file(3) may request that we set default ACLs with
> -		 * zero length -- defend (gracefully) against that here.
> -		 */
> -		goto out;
> -	}
> -	if (IS_ERR(acl)) {
> -		error = PTR_ERR(acl);
> -		goto out;
> -	}
> -
> -	error = posix_acl_valid(acl);
> -	if (error)
> -		goto out_release;
> -
> -	error = -EINVAL;
>  	if (acl->a_count > GFS2_ACL_MAX_ENTRIES)
> -		goto out_release;
> +		return -EINVAL;
>  
>  	if (type == ACL_TYPE_ACCESS) {
>  		umode_t mode = inode->i_mode;
> +
>  		error = posix_acl_equiv_mode(acl, &mode);
> +		if (error < 0)
>  
Andy Price has pointed out a missing "return error;" here

> -		if (error <= 0) {
> -			posix_acl_release(acl);
> +		if (error == 0)
>  			acl = NULL;
>  
> -			if (error < 0)
> -				return error;
> -		}
> -

Also, there seems to be a white space error in the xfs patch around line
170 in fs/xfs/xfs_iops.c where there is an added "if (default_acl)" with
a space before the tab,

Steve.

[Cluster-devel] [PATCH 00/18] Consolidate Posix ACL implementation

[Andreas Gruenbacher]

Christoph,

nice work, and a pretty diffstat.

I see that get_acl and set_acl are being defined in some but not all symlink inode operations (for example, btrfs them while ext4 does not), and that posix_acl_xattr_set() doesn't check if set_acl is defined.  Symlinks cannot have ACLs, so set_acl should either never be defined for symlinks (and a NULL check is then needed in posix_acl_xattr_set()), or it is defined in all inode operations, and S_ISNLNK() check is needed in posix_acl_xattr_set().  That latter check should probably be added in any case to be on the safe side.

  Test case:

  setfattr -h -n system.posix_acl_access \
           -v 0sAgAAAAEABgD/////AgAGABMEAAAEAAYA/////xAABgD/////IAAEAP////8= \
           symlink

Patch 6 also declares posix_acl_prepare() but this function is never introduced; this must be a leftover from a previous version.

Thanks,
Andreas

[Cluster-devel] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

[Jaegeuk Kim]

2013-12-01 (?), 03:59 -0800, Christoph Hellwig:

> f2fs has some weird mode bit handling, so still using the old
> chmod code for now.

f2fs caches a new mode bit for a while to make the consistency between
xattr's acl mode and the inode mode.
Anyway, it's a very good job.
Thanks,

You can add:
Reviewed-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/f2fs/acl.c   |  140 +++++++++----------------------------------------------
>  fs/f2fs/acl.h   |    1 +
>  fs/f2fs/file.c  |    1 +
>  fs/f2fs/namei.c |    2 +
>  fs/f2fs/xattr.c |    9 ++--
>  fs/f2fs/xattr.h |    2 -
>  6 files changed, 30 insertions(+), 125 deletions(-)
> 
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index 45e8430..4f52fe0f 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -205,7 +205,7 @@ struct posix_acl *f2fs_get_acl(struct inode *inode, int type)
>  	return acl;
>  }
>  
> -static int f2fs_set_acl(struct inode *inode, int type,
> +static int __f2fs_set_acl(struct inode *inode, int type,
>  			struct posix_acl *acl, struct page *ipage)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> @@ -261,37 +261,32 @@ static int f2fs_set_acl(struct inode *inode, int type,
>  	return error;
>  }
>  
> +int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> +{
> +	return __f2fs_set_acl(inode, type, acl, NULL);
> +}
> +
>  int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage)
>  {
> -	struct f2fs_sb_info *sbi = F2FS_SB(dir->i_sb);
> -	struct posix_acl *acl = NULL;
> +	struct posix_acl *default_acl, *acl;
>  	int error = 0;
>  
> -	if (!S_ISLNK(inode->i_mode)) {
> -		if (test_opt(sbi, POSIX_ACL)) {
> -			acl = f2fs_get_acl(dir, ACL_TYPE_DEFAULT);
> -			if (IS_ERR(acl))
> -				return PTR_ERR(acl);
> -		}
> -		if (!acl)
> -			inode->i_mode &= ~current_umask();
> -	}
> -
> -	if (!test_opt(sbi, POSIX_ACL) || !acl)
> -		goto cleanup;
> +	error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl);
> +	if (error)
> +		return error;
>  
> -	if (S_ISDIR(inode->i_mode)) {
> -		error = f2fs_set_acl(inode, ACL_TYPE_DEFAULT, acl, ipage);
> +	if (default_acl) {
> +		error = __f2fs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl,
> +				       ipage);
> +		posix_acl_release(default_acl);
> +	}
> +	if (acl) {
>  		if (error)
> -			goto cleanup;
> +			error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl,
> +					       ipage);
> +		posix_acl_release(acl);
>  	}
> -	error = __posix_acl_create(&acl, GFP_KERNEL, &inode->i_mode);
> -	if (error < 0)
> -		return error;
> -	if (error > 0)
> -		error = f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, ipage);
> -cleanup:
> -	posix_acl_release(acl);
> +
>  	return error;
>  }
>  
> @@ -315,100 +310,7 @@ int f2fs_acl_chmod(struct inode *inode)
>  	if (error)
>  		return error;
>  
> -	error = f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
> -	posix_acl_release(acl);
> -	return error;
> -}
> -
> -static size_t f2fs_xattr_list_acl(struct dentry *dentry, char *list,
> -		size_t list_size, const char *name, size_t name_len, int type)
> -{
> -	struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> -	const char *xname = POSIX_ACL_XATTR_DEFAULT;
> -	size_t size;
> -
> -	if (!test_opt(sbi, POSIX_ACL))
> -		return 0;
> -
> -	if (type == ACL_TYPE_ACCESS)
> -		xname = POSIX_ACL_XATTR_ACCESS;
> -
> -	size = strlen(xname) + 1;
> -	if (list && size <= list_size)
> -		memcpy(list, xname, size);
> -	return size;
> -}
> -
> -static int f2fs_xattr_get_acl(struct dentry *dentry, const char *name,
> -		void *buffer, size_t size, int type)
> -{
> -	struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> -	struct posix_acl *acl;
> -	int error;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(sbi, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -
> -	acl = f2fs_get_acl(dentry->d_inode, type);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (!acl)
> -		return -ENODATA;
> -	error = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
> -	posix_acl_release(acl);
> -
> -	return error;
> -}
> -
> -static int f2fs_xattr_set_acl(struct dentry *dentry, const char *name,
> -		const void *value, size_t size, int flags, int type)
> -{
> -	struct f2fs_sb_info *sbi = F2FS_SB(dentry->d_sb);
> -	struct inode *inode = dentry->d_inode;
> -	struct posix_acl *acl = NULL;
> -	int error;
> -
> -	if (strcmp(name, "") != 0)
> -		return -EINVAL;
> -	if (!test_opt(sbi, POSIX_ACL))
> -		return -EOPNOTSUPP;
> -	if (!inode_owner_or_capable(inode))
> -		return -EPERM;
> -
> -	if (value) {
> -		acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -		if (IS_ERR(acl))
> -			return PTR_ERR(acl);
> -		if (acl) {
> -			error = posix_acl_valid(acl);
> -			if (error)
> -				goto release_and_out;
> -		}
> -	} else {
> -		acl = NULL;
> -	}
> -
> -	error = f2fs_set_acl(inode, type, acl, NULL);
> -
> -release_and_out:
> +	error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
>  	posix_acl_release(acl);
>  	return error;
>  }
> -
> -const struct xattr_handler f2fs_xattr_acl_default_handler = {
> -	.prefix = POSIX_ACL_XATTR_DEFAULT,
> -	.flags = ACL_TYPE_DEFAULT,
> -	.list = f2fs_xattr_list_acl,
> -	.get = f2fs_xattr_get_acl,
> -	.set = f2fs_xattr_set_acl,
> -};
> -
> -const struct xattr_handler f2fs_xattr_acl_access_handler = {
> -	.prefix = POSIX_ACL_XATTR_ACCESS,
> -	.flags = ACL_TYPE_ACCESS,
> -	.list = f2fs_xattr_list_acl,
> -	.get = f2fs_xattr_get_acl,
> -	.set = f2fs_xattr_set_acl,
> -};
> diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
> index 4963313..2af31fe 100644
> --- a/fs/f2fs/acl.h
> +++ b/fs/f2fs/acl.h
> @@ -37,6 +37,7 @@ struct f2fs_acl_header {
>  #ifdef CONFIG_F2FS_FS_POSIX_ACL
>  
>  extern struct posix_acl *f2fs_get_acl(struct inode *, int);
> +extern int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  extern int f2fs_acl_chmod(struct inode *);
>  extern int f2fs_init_acl(struct inode *, struct inode *, struct page *);
>  #else
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7d714f4..13eff60 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -405,6 +405,7 @@ const struct inode_operations f2fs_file_inode_operations = {
>  	.getattr	= f2fs_getattr,
>  	.setattr	= f2fs_setattr,
>  	.get_acl	= f2fs_get_acl,
> +	.set_acl	= f2fs_set_acl,
>  #ifdef CONFIG_F2FS_FS_XATTR
>  	.setxattr	= generic_setxattr,
>  	.getxattr	= generic_getxattr,
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 575adac..5846eeb 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -496,6 +496,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
>  	.getattr	= f2fs_getattr,
>  	.setattr	= f2fs_setattr,
>  	.get_acl	= f2fs_get_acl,
> +	.set_acl	= f2fs_set_acl,
>  #ifdef CONFIG_F2FS_FS_XATTR
>  	.setxattr	= generic_setxattr,
>  	.getxattr	= generic_getxattr,
> @@ -522,6 +523,7 @@ const struct inode_operations f2fs_special_inode_operations = {
>  	.getattr	= f2fs_getattr,
>  	.setattr        = f2fs_setattr,
>  	.get_acl	= f2fs_get_acl,
> +	.set_acl	= f2fs_set_acl,
>  #ifdef CONFIG_F2FS_FS_XATTR
>  	.setxattr       = generic_setxattr,
>  	.getxattr       = generic_getxattr,
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index aa7a3f1..e2b9299 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -21,6 +21,7 @@
>  #include <linux/rwsem.h>
>  #include <linux/f2fs_fs.h>
>  #include <linux/security.h>
> +#include <linux/posix_acl_xattr.h>
>  #include "f2fs.h"
>  #include "xattr.h"
>  
> @@ -216,8 +217,8 @@ const struct xattr_handler f2fs_xattr_security_handler = {
>  static const struct xattr_handler *f2fs_xattr_handler_map[] = {
>  	[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
>  #ifdef CONFIG_F2FS_FS_POSIX_ACL
> -	[F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &f2fs_xattr_acl_access_handler,
> -	[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &f2fs_xattr_acl_default_handler,
> +	[F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler,
> +	[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
>  #endif
>  	[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
>  #ifdef CONFIG_F2FS_FS_SECURITY
> @@ -229,8 +230,8 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
>  const struct xattr_handler *f2fs_xattr_handlers[] = {
>  	&f2fs_xattr_user_handler,
>  #ifdef CONFIG_F2FS_FS_POSIX_ACL
> -	&f2fs_xattr_acl_access_handler,
> -	&f2fs_xattr_acl_default_handler,
> +	&posix_acl_access_xattr_handler,
> +	&posix_acl_default_xattr_handler,
>  #endif
>  	&f2fs_xattr_trusted_handler,
>  #ifdef CONFIG_F2FS_FS_SECURITY
> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> index 02a08fb..b21d9eb 100644
> --- a/fs/f2fs/xattr.h
> +++ b/fs/f2fs/xattr.h
> @@ -108,8 +108,6 @@ struct f2fs_xattr_entry {
>  #ifdef CONFIG_F2FS_FS_XATTR
>  extern const struct xattr_handler f2fs_xattr_user_handler;
>  extern const struct xattr_handler f2fs_xattr_trusted_handler;
> -extern const struct xattr_handler f2fs_xattr_acl_access_handler;
> -extern const struct xattr_handler f2fs_xattr_acl_default_handler;
>  extern const struct xattr_handler f2fs_xattr_advise_handler;
>  extern const struct xattr_handler f2fs_xattr_security_handler;
>  

-- 
Jaegeuk Kim
Samsung

[Cluster-devel] [PATCH 00/18] Consolidate Posix ACL implementation

[Christoph Hellwig]

On Thu, Dec 05, 2013 at 06:57:14PM +0100, Andreas Gruenbacher wrote:
> I see that get_acl and set_acl are being defined in some but not all symlink inode operations (for example, btrfs them while ext4 does not), and that posix_acl_xattr_set() doesn't check if set_acl is defined.  Symlinks cannot have ACLs, so set_acl should either never be defined for symlinks (and a NULL check is then needed in posix_acl_xattr_set()), or it is defined in all inode operations, and S_ISNLNK() check is needed in posix_acl_xattr_set().  That latter check should probably be added in any case to be on the safe side.

Yes, we should add the check.  We also in general should not have
set_acl/get_acl on links and I'll look over it.

> Patch 6 also declares posix_acl_prepare() but this function is never introduced; this must be a leftover from a previous version.

Indeed.

Thanks for the review!

[Cluster-devel] [PATCH 16/18] gfs2: use generic posix ACL infrastructure

[Christoph Hellwig]

On Wed, Dec 04, 2013 at 12:12:37PM +0000, Steven Whitehouse wrote:
> >  		error = posix_acl_equiv_mode(acl, &mode);
> > +		if (error < 0)
> >  
> Andy Price has pointed out a missing "return error;" here
> 
> > -		if (error <= 0) {
> > -			posix_acl_release(acl);
> > +		if (error == 0)
> >  			acl = NULL;
> >  
> > -			if (error < 0)
> > -				return error;
> > -		}
> > -
> 
> Also, there seems to be a white space error in the xfs patch around line
> 170 in fs/xfs/xfs_iops.c where there is an added "if (default_acl)" with
> a space before the tab,

I'll take care of these for the next version.

[Cluster-devel] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

[Christoph Hellwig]

On Fri, Dec 06, 2013 at 10:37:34AM +0900, Jaegeuk Kim wrote:
> f2fs caches a new mode bit for a while to make the consistency between
> xattr's acl mode and the inode mode.

Can you explain what exactly you're trying to do there?  I've been
trying to unwrap what's going on and can't really see the point:

 - i_acl_mode and FI_ACL_MODE get set in __setattr_copy, but right
   after that call, still under i_mutex and before marking the inode
   dirty f2fs_acl_chmod makes use of it, and it gets cleared right
   after. Is there any race that could happen with a locked inode
   not marked dirty yet on f2fs?  We could pass a mode argument
   to posix_acl_create, but I'd prefer to avoid that if we can.
 - on the set_acl side it gets set in __f2fs_set_acl, and then
   i_mode is update in __f2fs_setxattr which could easily done with
   a stack variable.

RFC patch below:


diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index 4f52fe0f..6647545 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -17,9 +17,6 @@
 #include "xattr.h"
 #include "acl.h"
 
-#define get_inode_mode(i)	((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
-					(F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
-
 static inline size_t f2fs_acl_size(int count)
 {
 	if (count <= 4) {
@@ -209,11 +206,11 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 			struct posix_acl *acl, struct page *ipage)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
-	struct f2fs_inode_info *fi = F2FS_I(inode);
 	int name_index;
 	void *value = NULL;
 	size_t size = 0;
 	int error;
+	umode_t mode = 0;
 
 	if (!test_opt(sbi, POSIX_ACL))
 		return 0;
@@ -224,10 +221,10 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 	case ACL_TYPE_ACCESS:
 		name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
 		if (acl) {
-			error = posix_acl_equiv_mode(acl, &inode->i_mode);
+			mode = inode->i_mode;
+			error = posix_acl_equiv_mode(acl, &mode);
 			if (error < 0)
 				return error;
-			set_acl_inode(fi, inode->i_mode);
 			if (error == 0)
 				acl = NULL;
 		}
@@ -245,19 +242,15 @@ static int __f2fs_set_acl(struct inode *inode, int type,
 
 	if (acl) {
 		value = f2fs_acl_to_disk(acl, &size);
-		if (IS_ERR(value)) {
-			cond_clear_inode_flag(fi, FI_ACL_MODE);
+		if (IS_ERR(value))
 			return (int)PTR_ERR(value);
-		}
 	}
 
-	error = f2fs_setxattr(inode, name_index, "", value, size, ipage);
+	error = f2fs_setxattr(inode, name_index, "", value, size, ipage, mode);
 
 	kfree(value);
 	if (!error)
 		set_cached_acl(inode, type, acl);
-
-	cond_clear_inode_flag(fi, FI_ACL_MODE);
 	return error;
 }
 
@@ -289,28 +282,3 @@ int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage)
 
 	return error;
 }
-
-int f2fs_acl_chmod(struct inode *inode)
-{
-	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
-	struct posix_acl *acl;
-	int error;
-	umode_t mode = get_inode_mode(inode);
-
-	if (!test_opt(sbi, POSIX_ACL))
-		return 0;
-	if (S_ISLNK(mode))
-		return -EOPNOTSUPP;
-
-	acl = f2fs_get_acl(inode, ACL_TYPE_ACCESS);
-	if (IS_ERR(acl) || !acl)
-		return PTR_ERR(acl);
-
-	error = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
-	if (error)
-		return error;
-
-	error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
-	posix_acl_release(acl);
-	return error;
-}
diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
index 2af31fe..e086465 100644
--- a/fs/f2fs/acl.h
+++ b/fs/f2fs/acl.h
@@ -38,18 +38,12 @@ struct f2fs_acl_header {
 
 extern struct posix_acl *f2fs_get_acl(struct inode *, int);
 extern int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
-extern int f2fs_acl_chmod(struct inode *);
 extern int f2fs_init_acl(struct inode *, struct inode *, struct page *);
 #else
 #define f2fs_check_acl	NULL
 #define f2fs_get_acl	NULL
 #define f2fs_set_acl	NULL
 
-static inline int f2fs_acl_chmod(struct inode *inode)
-{
-	return 0;
-}
-
 static inline int f2fs_init_acl(struct inode *inode, struct inode *dir,
 							struct page *page)
 {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 89dc750..1e774e6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -181,7 +181,6 @@ struct f2fs_inode_info {
 	unsigned char i_advise;		/* use to give file attribute hints */
 	unsigned int i_current_depth;	/* use only in directory structure */
 	unsigned int i_pino;		/* parent inode number */
-	umode_t i_acl_mode;		/* keep file acl mode temporarily */
 
 	/* Use below internally in f2fs*/
 	unsigned long flags;		/* use to pass per-file flags */
@@ -872,7 +871,6 @@ enum {
 	FI_NEW_INODE,		/* indicate newly allocated inode */
 	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
 	FI_INC_LINK,		/* need to increment i_nlink */
-	FI_ACL_MODE,		/* indicate acl mode */
 	FI_NO_ALLOC,		/* should not allocate any blocks */
 	FI_UPDATE_DIR,		/* should update inode block for consistency */
 	FI_DELAY_IPUT,		/* used for the recovery */
@@ -894,21 +892,6 @@ static inline void clear_inode_flag(struct f2fs_inode_info *fi, int flag)
 	clear_bit(flag, &fi->flags);
 }
 
-static inline void set_acl_inode(struct f2fs_inode_info *fi, umode_t mode)
-{
-	fi->i_acl_mode = mode;
-	set_inode_flag(fi, FI_ACL_MODE);
-}
-
-static inline int cond_clear_inode_flag(struct f2fs_inode_info *fi, int flag)
-{
-	if (is_inode_flag_set(fi, FI_ACL_MODE)) {
-		clear_inode_flag(fi, FI_ACL_MODE);
-		return 1;
-	}
-	return 0;
-}
-
 static inline void get_inline_info(struct f2fs_inode_info *fi,
 					struct f2fs_inode *ri)
 {
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 13eff60..80ef669 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -339,41 +339,9 @@ int f2fs_getattr(struct vfsmount *mnt,
 	return 0;
 }
 
-#ifdef CONFIG_F2FS_FS_POSIX_ACL
-static void __setattr_copy(struct inode *inode, const struct iattr *attr)
-{
-	struct f2fs_inode_info *fi = F2FS_I(inode);
-	unsigned int ia_valid = attr->ia_valid;
-
-	if (ia_valid & ATTR_UID)
-		inode->i_uid = attr->ia_uid;
-	if (ia_valid & ATTR_GID)
-		inode->i_gid = attr->ia_gid;
-	if (ia_valid & ATTR_ATIME)
-		inode->i_atime = timespec_trunc(attr->ia_atime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		inode->i_mtime = timespec_trunc(attr->ia_mtime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		inode->i_ctime = timespec_trunc(attr->ia_ctime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MODE) {
-		umode_t mode = attr->ia_mode;
-
-		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
-			mode &= ~S_ISGID;
-		set_acl_inode(fi, mode);
-	}
-}
-#else
-#define __setattr_copy setattr_copy
-#endif
-
 int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 {
 	struct inode *inode = dentry->d_inode;
-	struct f2fs_inode_info *fi = F2FS_I(inode);
 	int err;
 
 	err = inode_change_ok(inode, attr);
@@ -387,15 +355,9 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 		f2fs_balance_fs(F2FS_SB(inode->i_sb));
 	}
 
-	__setattr_copy(inode, attr);
-
-	if (attr->ia_valid & ATTR_MODE) {
-		err = f2fs_acl_chmod(inode);
-		if (err || is_inode_flag_set(fi, FI_ACL_MODE)) {
-			inode->i_mode = fi->i_acl_mode;
-			clear_inode_flag(fi, FI_ACL_MODE);
-		}
-	}
+	setattr_copy(inode, attr);
+	if (attr->ia_valid & ATTR_MODE)
+		err = posix_acl_chmod(inode);
 
 	mark_inode_dirty(inode);
 	return err;
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index e2b9299..8820857 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -108,7 +108,7 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
 	if (strcmp(name, "") == 0)
 		return -EINVAL;
 
-	return f2fs_setxattr(dentry->d_inode, type, name, value, size, NULL);
+	return f2fs_setxattr(dentry->d_inode, type, name, value, size, NULL, 0);
 }
 
 static size_t f2fs_xattr_advise_list(struct dentry *dentry, char *list,
@@ -157,7 +157,7 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
 #ifdef CONFIG_F2FS_FS_SECURITY
 static int __f2fs_setxattr(struct inode *inode, int name_index,
 			const char *name, const void *value, size_t value_len,
-			struct page *ipage);
+			struct page *ipage, umode_t mode);
 static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
 		void *page)
 {
@@ -167,7 +167,7 @@ static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
 	for (xattr = xattr_array; xattr->name != NULL; xattr++) {
 		err = __f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY,
 				xattr->name, xattr->value,
-				xattr->value_len, (struct page *)page);
+				xattr->value_len, (struct page *)page, 0);
 		if (err < 0)
 			break;
 	}
@@ -475,9 +475,8 @@ cleanup:
 
 static int __f2fs_setxattr(struct inode *inode, int name_index,
 			const char *name, const void *value, size_t value_len,
-			struct page *ipage)
+			struct page *ipage, umode_t mode)
 {
-	struct f2fs_inode_info *fi = F2FS_I(inode);
 	struct f2fs_xattr_entry *here, *last;
 	void *base_addr;
 	int found, newsize;
@@ -566,10 +565,9 @@ static int __f2fs_setxattr(struct inode *inode, int name_index,
 	if (error)
 		goto exit;
 
-	if (is_inode_flag_set(fi, FI_ACL_MODE)) {
-		inode->i_mode = fi->i_acl_mode;
+	if (mode) {
+		inode->i_mode = mode;
 		inode->i_ctime = CURRENT_TIME;
-		clear_inode_flag(fi, FI_ACL_MODE);
 	}
 
 	if (ipage)
@@ -582,7 +580,8 @@ exit:
 }
 
 int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
-			const void *value, size_t value_len, struct page *ipage)
+			const void *value, size_t value_len, struct page *ipage,
+			umode_t mode)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
 	int err;
@@ -590,7 +589,8 @@ int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
 	f2fs_balance_fs(sbi);
 
 	f2fs_lock_op(sbi);
-	err = __f2fs_setxattr(inode, name_index, name, value, value_len, ipage);
+	err = __f2fs_setxattr(inode, name_index, name, value, value_len, ipage,
+			      mode);
 	f2fs_unlock_op(sbi);
 
 	return err;
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index b21d9eb..c73588a 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -114,14 +114,15 @@ extern const struct xattr_handler f2fs_xattr_security_handler;
 extern const struct xattr_handler *f2fs_xattr_handlers[];
 
 extern int f2fs_setxattr(struct inode *, int, const char *,
-				const void *, size_t, struct page *);
+				const void *, size_t, struct page *, umode_t);
 extern int f2fs_getxattr(struct inode *, int, const char *, void *, size_t);
 extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
 #else
 
 #define f2fs_xattr_handlers	NULL
 static inline int f2fs_setxattr(struct inode *inode, int name_index,
-		const char *name, const void *value, size_t value_len)
+		const char *name, const void *value, size_t value_len,
+		umode_t mode)
 {
 	return -EOPNOTSUPP;
 }

[Cluster-devel] [PATCH 09/18] f2fs: use generic posix ACL infrastructure

[Jaegeuk Kim]

2013-12-08 (?), 01:14 -0800, Christoph Hellwig:
> On Fri, Dec 06, 2013 at 10:37:34AM +0900, Jaegeuk Kim wrote:
> > f2fs caches a new mode bit for a while to make the consistency between
> > xattr's acl mode and the inode mode.
> 
> Can you explain what exactly you're trying to do there?  I've been
> trying to unwrap what's going on and can't really see the point:
> 
>  - i_acl_mode and FI_ACL_MODE get set in __setattr_copy, but right
>    after that call, still under i_mutex and before marking the inode
>    dirty f2fs_acl_chmod makes use of it, and it gets cleared right
>    after. Is there any race that could happen with a locked inode
>    not marked dirty yet on f2fs?

As you guess, there is no race problem, but the problem is on acl
consistency between xattr->mode and inode->mode.

Previously, f2fs_setattr triggers:
              new_mode inode->mode xattr->mode iblock->mode
f2fs_setattr     x    ->    x           y         y       
[update_inode]              x    ---  [ y ]  ---> x
[checkpoint]                x           y         x
__f2fs_setxattr             x     ->    x         x

In this flow, f2fs is able to break the consistency between xattr->mode
and iblock->mode after checkpoint followed by sudden-power-off.

So, fi->mode was introduced to address the problem.
The new f2fs_setattr triggers:
              new_mode inode->mode fi->mode xattr->mode iblock->mode
f2fs_setattr     x    ---  [y]  --->   x          y          y
[update_inode]              y          x          y          y
[checkpoint]                y          x          y          y
__f2fs_setxattr             x    <-    x    ->    x     ->   x

Finally, __f2fs_setxattr synchronizes inode->mode, xattr->mode, and
iblock->mode all together.

The root question is "is it possible to call update_inode in the
i_mutex-covered region like f2fs_setattr?".
The update_inode of f2fs is called from a bunch of places so currently
I'm not sure it can be impossible.

Thanks,

> We could pass a mode argument
>    to posix_acl_create, but I'd prefer to avoid that if we can.
>  - on the set_acl side it gets set in __f2fs_set_acl, and then
>    i_mode is update in __f2fs_setxattr which could easily done with
>    a stack variable.
> 
> RFC patch below:
> 
> 
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index 4f52fe0f..6647545 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -17,9 +17,6 @@
>  #include "xattr.h"
>  #include "acl.h"
>  
> -#define get_inode_mode(i)	((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
> -					(F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
> -
>  static inline size_t f2fs_acl_size(int count)
>  {
>  	if (count <= 4) {
> @@ -209,11 +206,11 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>  			struct posix_acl *acl, struct page *ipage)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> -	struct f2fs_inode_info *fi = F2FS_I(inode);
>  	int name_index;
>  	void *value = NULL;
>  	size_t size = 0;
>  	int error;
> +	umode_t mode = 0;
>  
>  	if (!test_opt(sbi, POSIX_ACL))
>  		return 0;
> @@ -224,10 +221,10 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>  	case ACL_TYPE_ACCESS:
>  		name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS;
>  		if (acl) {
> -			error = posix_acl_equiv_mode(acl, &inode->i_mode);
> +			mode = inode->i_mode;
> +			error = posix_acl_equiv_mode(acl, &mode);
>  			if (error < 0)
>  				return error;
> -			set_acl_inode(fi, inode->i_mode);
>  			if (error == 0)
>  				acl = NULL;
>  		}
> @@ -245,19 +242,15 @@ static int __f2fs_set_acl(struct inode *inode, int type,
>  
>  	if (acl) {
>  		value = f2fs_acl_to_disk(acl, &size);
> -		if (IS_ERR(value)) {
> -			cond_clear_inode_flag(fi, FI_ACL_MODE);
> +		if (IS_ERR(value))
>  			return (int)PTR_ERR(value);
> -		}
>  	}
>  
> -	error = f2fs_setxattr(inode, name_index, "", value, size, ipage);
> +	error = f2fs_setxattr(inode, name_index, "", value, size, ipage, mode);
>  
>  	kfree(value);
>  	if (!error)
>  		set_cached_acl(inode, type, acl);
> -
> -	cond_clear_inode_flag(fi, FI_ACL_MODE);
>  	return error;
>  }
>  
> @@ -289,28 +282,3 @@ int f2fs_init_acl(struct inode *inode, struct inode *dir, struct page *ipage)
>  
>  	return error;
>  }
> -
> -int f2fs_acl_chmod(struct inode *inode)
> -{
> -	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> -	struct posix_acl *acl;
> -	int error;
> -	umode_t mode = get_inode_mode(inode);
> -
> -	if (!test_opt(sbi, POSIX_ACL))
> -		return 0;
> -	if (S_ISLNK(mode))
> -		return -EOPNOTSUPP;
> -
> -	acl = f2fs_get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl) || !acl)
> -		return PTR_ERR(acl);
> -
> -	error = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
> -	if (error)
> -		return error;
> -
> -	error = __f2fs_set_acl(inode, ACL_TYPE_ACCESS, acl, NULL);
> -	posix_acl_release(acl);
> -	return error;
> -}
> diff --git a/fs/f2fs/acl.h b/fs/f2fs/acl.h
> index 2af31fe..e086465 100644
> --- a/fs/f2fs/acl.h
> +++ b/fs/f2fs/acl.h
> @@ -38,18 +38,12 @@ struct f2fs_acl_header {
>  
>  extern struct posix_acl *f2fs_get_acl(struct inode *, int);
>  extern int f2fs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
> -extern int f2fs_acl_chmod(struct inode *);
>  extern int f2fs_init_acl(struct inode *, struct inode *, struct page *);
>  #else
>  #define f2fs_check_acl	NULL
>  #define f2fs_get_acl	NULL
>  #define f2fs_set_acl	NULL
>  
> -static inline int f2fs_acl_chmod(struct inode *inode)
> -{
> -	return 0;
> -}
> -
>  static inline int f2fs_init_acl(struct inode *inode, struct inode *dir,
>  							struct page *page)
>  {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 89dc750..1e774e6 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -181,7 +181,6 @@ struct f2fs_inode_info {
>  	unsigned char i_advise;		/* use to give file attribute hints */
>  	unsigned int i_current_depth;	/* use only in directory structure */
>  	unsigned int i_pino;		/* parent inode number */
> -	umode_t i_acl_mode;		/* keep file acl mode temporarily */
>  
>  	/* Use below internally in f2fs*/
>  	unsigned long flags;		/* use to pass per-file flags */
> @@ -872,7 +871,6 @@ enum {
>  	FI_NEW_INODE,		/* indicate newly allocated inode */
>  	FI_DIRTY_INODE,		/* indicate inode is dirty or not */
>  	FI_INC_LINK,		/* need to increment i_nlink */
> -	FI_ACL_MODE,		/* indicate acl mode */
>  	FI_NO_ALLOC,		/* should not allocate any blocks */
>  	FI_UPDATE_DIR,		/* should update inode block for consistency */
>  	FI_DELAY_IPUT,		/* used for the recovery */
> @@ -894,21 +892,6 @@ static inline void clear_inode_flag(struct f2fs_inode_info *fi, int flag)
>  	clear_bit(flag, &fi->flags);
>  }
>  
> -static inline void set_acl_inode(struct f2fs_inode_info *fi, umode_t mode)
> -{
> -	fi->i_acl_mode = mode;
> -	set_inode_flag(fi, FI_ACL_MODE);
> -}
> -
> -static inline int cond_clear_inode_flag(struct f2fs_inode_info *fi, int flag)
> -{
> -	if (is_inode_flag_set(fi, FI_ACL_MODE)) {
> -		clear_inode_flag(fi, FI_ACL_MODE);
> -		return 1;
> -	}
> -	return 0;
> -}
> -
>  static inline void get_inline_info(struct f2fs_inode_info *fi,
>  					struct f2fs_inode *ri)
>  {
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 13eff60..80ef669 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -339,41 +339,9 @@ int f2fs_getattr(struct vfsmount *mnt,
>  	return 0;
>  }
>  
> -#ifdef CONFIG_F2FS_FS_POSIX_ACL
> -static void __setattr_copy(struct inode *inode, const struct iattr *attr)
> -{
> -	struct f2fs_inode_info *fi = F2FS_I(inode);
> -	unsigned int ia_valid = attr->ia_valid;
> -
> -	if (ia_valid & ATTR_UID)
> -		inode->i_uid = attr->ia_uid;
> -	if (ia_valid & ATTR_GID)
> -		inode->i_gid = attr->ia_gid;
> -	if (ia_valid & ATTR_ATIME)
> -		inode->i_atime = timespec_trunc(attr->ia_atime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_MTIME)
> -		inode->i_mtime = timespec_trunc(attr->ia_mtime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_CTIME)
> -		inode->i_ctime = timespec_trunc(attr->ia_ctime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_MODE) {
> -		umode_t mode = attr->ia_mode;
> -
> -		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
> -			mode &= ~S_ISGID;
> -		set_acl_inode(fi, mode);
> -	}
> -}
> -#else
> -#define __setattr_copy setattr_copy
> -#endif
> -
>  int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
>  {
>  	struct inode *inode = dentry->d_inode;
> -	struct f2fs_inode_info *fi = F2FS_I(inode);
>  	int err;
>  
>  	err = inode_change_ok(inode, attr);
> @@ -387,15 +355,9 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
>  		f2fs_balance_fs(F2FS_SB(inode->i_sb));
>  	}
>  
> -	__setattr_copy(inode, attr);
> -
> -	if (attr->ia_valid & ATTR_MODE) {
> -		err = f2fs_acl_chmod(inode);
> -		if (err || is_inode_flag_set(fi, FI_ACL_MODE)) {
> -			inode->i_mode = fi->i_acl_mode;
> -			clear_inode_flag(fi, FI_ACL_MODE);
> -		}
> -	}
> +	setattr_copy(inode, attr);
> +	if (attr->ia_valid & ATTR_MODE)
> +		err = posix_acl_chmod(inode);
>  
>  	mark_inode_dirty(inode);
>  	return err;
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index e2b9299..8820857 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -108,7 +108,7 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
>  	if (strcmp(name, "") == 0)
>  		return -EINVAL;
>  
> -	return f2fs_setxattr(dentry->d_inode, type, name, value, size, NULL);
> +	return f2fs_setxattr(dentry->d_inode, type, name, value, size, NULL, 0);
>  }
>  
>  static size_t f2fs_xattr_advise_list(struct dentry *dentry, char *list,
> @@ -157,7 +157,7 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
>  #ifdef CONFIG_F2FS_FS_SECURITY
>  static int __f2fs_setxattr(struct inode *inode, int name_index,
>  			const char *name, const void *value, size_t value_len,
> -			struct page *ipage);
> +			struct page *ipage, umode_t mode);
>  static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
>  		void *page)
>  {
> @@ -167,7 +167,7 @@ static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
>  	for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>  		err = __f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY,
>  				xattr->name, xattr->value,
> -				xattr->value_len, (struct page *)page);
> +				xattr->value_len, (struct page *)page, 0);
>  		if (err < 0)
>  			break;
>  	}
> @@ -475,9 +475,8 @@ cleanup:
>  
>  static int __f2fs_setxattr(struct inode *inode, int name_index,
>  			const char *name, const void *value, size_t value_len,
> -			struct page *ipage)
> +			struct page *ipage, umode_t mode)
>  {
> -	struct f2fs_inode_info *fi = F2FS_I(inode);
>  	struct f2fs_xattr_entry *here, *last;
>  	void *base_addr;
>  	int found, newsize;
> @@ -566,10 +565,9 @@ static int __f2fs_setxattr(struct inode *inode, int name_index,
>  	if (error)
>  		goto exit;
>  
> -	if (is_inode_flag_set(fi, FI_ACL_MODE)) {
> -		inode->i_mode = fi->i_acl_mode;
> +	if (mode) {
> +		inode->i_mode = mode;
>  		inode->i_ctime = CURRENT_TIME;
> -		clear_inode_flag(fi, FI_ACL_MODE);
>  	}
>  
>  	if (ipage)
> @@ -582,7 +580,8 @@ exit:
>  }
>  
>  int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
> -			const void *value, size_t value_len, struct page *ipage)
> +			const void *value, size_t value_len, struct page *ipage,
> +			umode_t mode)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
>  	int err;
> @@ -590,7 +589,8 @@ int f2fs_setxattr(struct inode *inode, int name_index, const char *name,
>  	f2fs_balance_fs(sbi);
>  
>  	f2fs_lock_op(sbi);
> -	err = __f2fs_setxattr(inode, name_index, name, value, value_len, ipage);
> +	err = __f2fs_setxattr(inode, name_index, name, value, value_len, ipage,
> +			      mode);
>  	f2fs_unlock_op(sbi);
>  
>  	return err;
> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> index b21d9eb..c73588a 100644
> --- a/fs/f2fs/xattr.h
> +++ b/fs/f2fs/xattr.h
> @@ -114,14 +114,15 @@ extern const struct xattr_handler f2fs_xattr_security_handler;
>  extern const struct xattr_handler *f2fs_xattr_handlers[];
>  
>  extern int f2fs_setxattr(struct inode *, int, const char *,
> -				const void *, size_t, struct page *);
> +				const void *, size_t, struct page *, umode_t);
>  extern int f2fs_getxattr(struct inode *, int, const char *, void *, size_t);
>  extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
>  #else
>  
>  #define f2fs_xattr_handlers	NULL
>  static inline int f2fs_setxattr(struct inode *inode, int name_index,
> -		const char *name, const void *value, size_t value_len)
> +		const char *name, const void *value, size_t value_len,
> +		umode_t mode)
>  {
>  	return -EOPNOTSUPP;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jaegeuk Kim
Samsung

[Cluster-devel] [PATCH 12/18] ocfs2: use generic posix ACL infrastructure

[Christoph Hellwig]

An embedded and charset-unspecified text was scrubbed...
Name: 0012-ocfs2-use-generic-posix-ACL-infrastructure.patch
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20131211/88e1778c/attachment.ksh>