Re: [PATCH 2/3] simplify xfs_setattr

[Timothy Shimmin <tes@sgi.com>]

Looks reasonable to me :)

(odd notes below)

--Tim



Christoph Hellwig wrote:
> On Fri, Jun 27, 2008 at 05:45:57PM +0200, Christoph Hellwig wrote:
>> Now that xfs_setattr is only used for attributes set from ->setattr it
>> can be switched to take struct iattr directly and thus simplify the
>> implementation greatly.  Also rename the ATTR_ flags to XFS_ATTR_ to
>> not conflict with the ATTR_ flags used by the VFS.
> 
> As per discussion with Tim here's a version that doesn't require the
> generic ACL patch applied before:
> 
> 

o stop using bhv_vattr_t and move over to struct iattr.
  where va_mask -> ia_valid

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/dmapi/xfs_dm.c	2008-07-04 14:59:13.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c	2008-07-04 14:59:21.000000000 +0200
> @@ -2460,7 +2460,8 @@ xfs_dm_punch_hole(
>  #endif
>  
>  	error = xfs_change_file_space(ip, XFS_IOC_UNRESVSP, &bf,
> -				(xfs_off_t)off, sys_cred, ATTR_DMI|ATTR_NOLOCK);
> +				(xfs_off_t)off, sys_cred,
> +				XFS_ATTR_DMI|XFS_ATTR_NOLOCK);
>  
>  	/*
>  	 * if punching to end of file, kill any blocks past EOF that
> @@ -2663,7 +2664,7 @@ xfs_dm_set_fileattr(
>  	dm_fileattr_t	__user *statp)
>  {
>  	dm_fileattr_t	stat;
> -	bhv_vattr_t	vat;
> +	struct iattr	iattr;
>  	int		error;
>  
>  	/* Returns negative errors to DMAPI */
> @@ -2674,52 +2675,52 @@ xfs_dm_set_fileattr(
>  	if (copy_from_user( &stat, statp, sizeof(stat)))
>  		return(-EFAULT);
>  
> -	vat.va_mask = 0;
> +	iattr.ia_valid = 0;
>  
>  	if (mask & DM_AT_MODE) {
> -		vat.va_mask |= XFS_AT_MODE;
> -		vat.va_mode = stat.fa_mode;
> +		iattr.ia_valid |= ATTR_MODE;
> +		iattr.ia_mode = stat.fa_mode;
>  	}
>  	if (mask & DM_AT_UID) {
> -		vat.va_mask |= XFS_AT_UID;
> -		vat.va_uid = stat.fa_uid;
> +		iattr.ia_valid |= ATTR_UID;
> +		iattr.ia_uid = stat.fa_uid;
>  	}
>  	if (mask & DM_AT_GID) {
> -		vat.va_mask |= XFS_AT_GID;
> -		vat.va_gid = stat.fa_gid;
> +		iattr.ia_valid |= ATTR_GID;
> +		iattr.ia_gid = stat.fa_gid;
>  	}
>  	if (mask & DM_AT_ATIME) {
> -		vat.va_mask |= XFS_AT_ATIME;
> -		vat.va_atime.tv_sec = stat.fa_atime;
> -		vat.va_atime.tv_nsec = 0;
> +		iattr.ia_valid |= ATTR_ATIME;
> +		iattr.ia_atime.tv_sec = stat.fa_atime;
> +		iattr.ia_atime.tv_nsec = 0;
>                  inode->i_atime.tv_sec = stat.fa_atime;
>  	}
>  	if (mask & DM_AT_MTIME) {
> -		vat.va_mask |= XFS_AT_MTIME;
> -		vat.va_mtime.tv_sec = stat.fa_mtime;
> -		vat.va_mtime.tv_nsec = 0;
> +		iattr.ia_valid |= ATTR_MTIME;
> +		iattr.ia_mtime.tv_sec = stat.fa_mtime;
> +		iattr.ia_mtime.tv_nsec = 0;
>  	}
>  	if (mask & DM_AT_CTIME) {
> -		vat.va_mask |= XFS_AT_CTIME;
> -		vat.va_ctime.tv_sec = stat.fa_ctime;
> -		vat.va_ctime.tv_nsec = 0;
> +		iattr.ia_valid |= ATTR_CTIME;
> +		iattr.ia_ctime.tv_sec = stat.fa_ctime;
> +		iattr.ia_ctime.tv_nsec = 0;
>  	}
>  
> -	/* DM_AT_DTIME only takes effect if DM_AT_CTIME is not specified.  We
> -	   overload ctime to also act as dtime, i.e. DM_CONFIG_DTIME_OVERLOAD.
> -	*/
> -
> +	/*
> +	 * DM_AT_DTIME only takes effect if DM_AT_CTIME is not specified.  We
> +	 * overload ctime to also act as dtime, i.e. DM_CONFIG_DTIME_OVERLOAD.
> +	 */
>  	if ((mask & DM_AT_DTIME) && !(mask & DM_AT_CTIME)) {
> -		vat.va_mask |= XFS_AT_CTIME;
> -		vat.va_ctime.tv_sec = stat.fa_dtime;
> -		vat.va_ctime.tv_nsec = 0;
> +		iattr.ia_valid |= ATTR_CTIME;
> +		iattr.ia_ctime.tv_sec = stat.fa_dtime;
> +		iattr.ia_ctime.tv_nsec = 0;
>  	}
>  	if (mask & DM_AT_SIZE) {
> -		vat.va_mask |= XFS_AT_SIZE;
> -		vat.va_size = stat.fa_size;
> +		iattr.ia_valid |= ATTR_SIZE;
> +		iattr.ia_size = stat.fa_size;
>  	}
>  
> -	error = xfs_setattr(XFS_I(inode), &vat, ATTR_DMI, NULL);
> +	error = xfs_setattr(XFS_I(inode), &iattr, XFS_ATTR_DMI, NULL);
>  	if (!error)
>  		vn_revalidate(vn_from_inode(inode));	/* update Linux inode flags */
>  	return(-error); /* Return negative error to DMAPI */

Looks good.

> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c	2008-07-04 14:59:11.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c	2008-07-04 15:03:53.000000000 +0200
> @@ -656,54 +656,20 @@ xfs_vn_getattr(
>  STATIC int
>  xfs_vn_setattr(
>  	struct dentry	*dentry,
> -	struct iattr	*attr)
> +	struct iattr	*iattr)
>  {
>  	struct inode	*inode = dentry->d_inode;
> -	unsigned int	ia_valid = attr->ia_valid;
> -	bhv_vattr_t	vattr = { 0 };
> -	int		flags = 0;
>  	int		error;
>  
> -	if (ia_valid & ATTR_UID) {
> -		vattr.va_mask |= XFS_AT_UID;
> -		vattr.va_uid = attr->ia_uid;
> -	}
> -	if (ia_valid & ATTR_GID) {
> -		vattr.va_mask |= XFS_AT_GID;
> -		vattr.va_gid = attr->ia_gid;
> -	}
> -	if (ia_valid & ATTR_SIZE) {
> -		vattr.va_mask |= XFS_AT_SIZE;
> -		vattr.va_size = attr->ia_size;
> -	}
> -	if (ia_valid & ATTR_ATIME) {
> -		vattr.va_mask |= XFS_AT_ATIME;
> -		vattr.va_atime = attr->ia_atime;
> -		inode->i_atime = attr->ia_atime;
> -	}
> -	if (ia_valid & ATTR_MTIME) {
> -		vattr.va_mask |= XFS_AT_MTIME;
> -		vattr.va_mtime = attr->ia_mtime;
> -	}
> -	if (ia_valid & ATTR_CTIME) {
> -		vattr.va_mask |= XFS_AT_CTIME;
> -		vattr.va_ctime = attr->ia_ctime;
> -	}
> -	if (ia_valid & ATTR_MODE) {
> -		vattr.va_mask |= XFS_AT_MODE;
> -		vattr.va_mode = attr->ia_mode;
> +	if (iattr->ia_valid & ATTR_ATIME)
> +		inode->i_atime = iattr->ia_atime;
> +
> +	if (iattr->ia_valid & ATTR_MODE) {
>  		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
>  			inode->i_mode &= ~S_ISGID;
>  	}
>  
So just need to keep the changes to the inode, don't need to set vattr
as we are just passing thru iattr into xfs_setattr now.
Fine.

> -	if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET))
> -		flags |= ATTR_UTIME;
> -#ifdef ATTR_NO_BLOCK
> -	if ((ia_valid & ATTR_NO_BLOCK))
> -		flags |= ATTR_NONBLOCK;
> -#endif
> -

So this code looks different.
We are now dropping the flags. Why is that?
Presumably because we were mapping ia_valid's:
   ATTR_MTIME_SET or ATTR_ATIME_SET --> ATTR_UTIME
   ATTR_NO_BLOCK -> ATTR_NONBLOCK
But now we pass ATTR_?TIME_SET and ATTR_NO_BLOCK straight thru.
So previously we didn't map them onto va_mask bits but as separate flags
instead.



> -	error = xfs_setattr(XFS_I(inode), &vattr, flags, NULL);
> +	error = xfs_setattr(XFS_I(inode), iattr, 0, NULL);

So no flags here now.

>  	if (likely(!error))
>  		vn_revalidate(vn_from_inode(inode));
>  	return -error;
> @@ -747,18 +713,18 @@ xfs_vn_fallocate(
>  
>  	xfs_ilock(ip, XFS_IOLOCK_EXCL);
>  	error = xfs_change_file_space(ip, XFS_IOC_RESVSP, &bf,
> -						0, NULL, ATTR_NOLOCK);
> +				      0, NULL, XFS_ATTR_NOLOCK);
>  	if (!error && !(mode & FALLOC_FL_KEEP_SIZE) &&
>  	    offset + len > i_size_read(inode))
>  		new_size = offset + len;
>  
>  	/* Change file size if needed */
>  	if (new_size) {
> -		bhv_vattr_t	va;
> +		struct iattr iattr;
>  
> -		va.va_mask = XFS_AT_SIZE;
> -		va.va_size = new_size;
> -		error = xfs_setattr(ip, &va, ATTR_NOLOCK, NULL);
> +		iattr.ia_valid = ATTR_SIZE;
> +		iattr.ia_size = new_size;
> +		error = xfs_setattr(ip, &iattr, XFS_ATTR_NOLOCK, NULL);
>  	}
>  
Fine.

>  	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_vnode.h
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_vnode.h	2008-07-04 14:59:14.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_vnode.h	2008-07-04 14:59:21.000000000 +0200
> @@ -19,7 +19,6 @@
>  #define __XFS_VNODE_H__
>  
>  struct file;
> -struct bhv_vattr;
>  struct xfs_iomap;
>  struct attrlist_cursor_kern;
>  
> @@ -66,69 +65,6 @@ static inline struct inode *vn_to_inode(
>  					   Prevent VM access to the pages until
>  					   the operation completes. */
>  
> -/*
> - * Vnode attributes.  va_mask indicates those attributes the caller
> - * wants to set or extract.
> - */
> -typedef struct bhv_vattr {
> -	int		va_mask;	/* bit-mask of attributes present */
> -	mode_t		va_mode;	/* file access mode and type */
> -	xfs_nlink_t	va_nlink;	/* number of references to file */
> -	uid_t		va_uid;		/* owner user id */
> -	gid_t		va_gid;		/* owner group id */
> -	xfs_ino_t	va_nodeid;	/* file id */
> -	xfs_off_t	va_size;	/* file size in bytes */
> -	u_long		va_blocksize;	/* blocksize preferred for i/o */
> -	struct timespec	va_atime;	/* time of last access */
> -	struct timespec	va_mtime;	/* time of last modification */
> -	struct timespec	va_ctime;	/* time file changed */
> -	u_int		va_gen;		/* generation number of file */
> -	xfs_dev_t	va_rdev;	/* device the special file represents */
> -	__int64_t	va_nblocks;	/* number of blocks allocated */
> -	u_long		va_xflags;	/* random extended file flags */
> -	u_long		va_extsize;	/* file extent size */
> -	u_long		va_nextents;	/* number of extents in file */
> -	u_long		va_anextents;	/* number of attr extents in file */
> -	prid_t		va_projid;	/* project id */
> -} bhv_vattr_t;
> -
> -/*
> - * setattr or getattr attributes
> - */
> -#define XFS_AT_TYPE		0x00000001
> -#define XFS_AT_MODE		0x00000002
> -#define XFS_AT_UID		0x00000004
> -#define XFS_AT_GID		0x00000008
> -#define XFS_AT_FSID		0x00000010
> -#define XFS_AT_NODEID		0x00000020
> -#define XFS_AT_NLINK		0x00000040
> -#define XFS_AT_SIZE		0x00000080
> -#define XFS_AT_ATIME		0x00000100
> -#define XFS_AT_MTIME		0x00000200
> -#define XFS_AT_CTIME		0x00000400
> -#define XFS_AT_RDEV		0x00000800
> -#define XFS_AT_BLKSIZE		0x00001000
> -#define XFS_AT_NBLOCKS		0x00002000
> -#define XFS_AT_VCODE		0x00004000
> -#define XFS_AT_MAC		0x00008000
> -#define XFS_AT_UPDATIME		0x00010000
> -#define XFS_AT_UPDMTIME		0x00020000
> -#define XFS_AT_UPDCTIME		0x00040000
> -#define XFS_AT_ACL		0x00080000
> -#define XFS_AT_CAP		0x00100000
> -#define XFS_AT_INF		0x00200000
> -#define XFS_AT_NEXTENTS		0x01000000
> -#define XFS_AT_ANEXTENTS	0x02000000
> -#define XFS_AT_SIZE_NOPERM	0x08000000
> -#define XFS_AT_GENCOUNT		0x10000000
> -
> -#define XFS_AT_TIMES	(XFS_AT_ATIME|XFS_AT_MTIME|XFS_AT_CTIME)
> -
> -#define XFS_AT_UPDTIMES	(XFS_AT_UPDATIME|XFS_AT_UPDMTIME|XFS_AT_UPDCTIME)
> -
> -#define XFS_AT_NOSET	(XFS_AT_NLINK|XFS_AT_RDEV|XFS_AT_FSID|XFS_AT_NODEID|\
> -		XFS_AT_TYPE|XFS_AT_BLKSIZE|XFS_AT_NBLOCKS|XFS_AT_VCODE|\
> -		XFS_AT_NEXTENTS|XFS_AT_ANEXTENTS|XFS_AT_GENCOUNT)
>  
Cool. Get rid of a whole bunch of stuff.

>  extern void	vn_init(void);
>  extern int	vn_revalidate(bhv_vnode_t *);
> @@ -204,15 +140,6 @@ static inline void vn_atime_to_time_t(bh
>  #define VN_DIRTY(vp)	mapping_tagged(vn_to_inode(vp)->i_mapping, \
>  					PAGECACHE_TAG_DIRTY)
>  
> -/*
> - * Flags to vop_setattr/getattr.
> - */
> -#define	ATTR_UTIME	0x01	/* non-default utime(2) request */
> -#define	ATTR_DMI	0x08	/* invocation from a DMI function */
> -#define	ATTR_LAZY	0x80	/* set/get attributes lazily */
> -#define	ATTR_NONBLOCK	0x100	/* return EAGAIN if operation would block */
> -#define ATTR_NOLOCK	0x200	/* Don't grab any conflicting locks */
> -#define ATTR_NOSIZETOK	0x400	/* Don't get the SIZE token */
>  
So where do these go now?
Looking ahead:
xfs_vnodeops.h: DMI,NONBLOCK,NOLOCK

And UTIME, LAZY and NOSIZETOK are gone.
LAZY doesn't seem to be used anywhere.
NOSIZETOK is presumably for cxfs.
UTIME is done by ATTR_MTIME_SET or ATTR_ATIME_SET now (passed straight thru).



>  /*
>   * Tracking vnode activity.
> Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_vnodeops.c	2008-07-04 14:59:14.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_vnodeops.c	2008-07-04 14:59:21.000000000 +0200
> @@ -75,19 +75,16 @@ xfs_open(
>  	return 0;
>  }
>  
> -/*
> - * xfs_setattr
> - */
>  int
>  xfs_setattr(
> -	xfs_inode_t		*ip,
> -	bhv_vattr_t		*vap,
> +	struct xfs_inode	*ip,
> +	struct iattr		*iattr,
>  	int			flags,
>  	cred_t			*credp)
>  {
>  	xfs_mount_t		*mp = ip->i_mount;
> +	int			mask = iattr->ia_valid;
>  	xfs_trans_t		*tp;
> -	int			mask;
>  	int			code;
>  	uint			lock_flags;
>  	uint			commit_flags=0;
> @@ -103,30 +100,9 @@ xfs_setattr(
>  	if (mp->m_flags & XFS_MOUNT_RDONLY)
>  		return XFS_ERROR(EROFS);
>  
> -	/*
> -	 * Cannot set certain attributes.
> -	 */
> -	mask = vap->va_mask;
> -	if (mask & XFS_AT_NOSET) {
> -		return XFS_ERROR(EINVAL);
> -	}
> -

So we get rid of the test for XFS_AT_NOSET.
where:
#define XFS_AT_NOSET    (XFS_AT_NLINK|XFS_AT_RDEV|XFS_AT_FSID|XFS_AT_NODEID|\
                XFS_AT_TYPE|XFS_AT_BLKSIZE|XFS_AT_NBLOCKS|XFS_AT_VCODE|\
                XFS_AT_NEXTENTS|XFS_AT_ANEXTENTS|XFS_AT_GENCOUNT)

I can't see anywhere we set any of these.
Presumably out of the xattr calls.
Some left over from IRIX I guess.


>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return XFS_ERROR(EIO);
>  
> -	/*
> -	 * Timestamps do not need to be logged and hence do not
> -	 * need to be done within a transaction.
> -	 */
> -	if (mask & XFS_AT_UPDTIMES) {
> -		ASSERT((mask & ~XFS_AT_UPDTIMES) == 0);
> -		timeflags = ((mask & XFS_AT_UPDATIME) ? XFS_ICHGTIME_ACC : 0) |
> -			    ((mask & XFS_AT_UPDCTIME) ? XFS_ICHGTIME_CHG : 0) |
> -			    ((mask & XFS_AT_UPDMTIME) ? XFS_ICHGTIME_MOD : 0);
> -		xfs_ichgtime(ip, timeflags);
> -		return 0;
> -	}
> -

#define XFS_AT_UPDATIME         0x00010000
#define XFS_AT_UPDMTIME         0x00020000
#define XFS_AT_UPDCTIME         0x00040000
3 more not supported by vfs ATTR_* macros.
I can't see where we set any of these.
So no loss there I guess.
Presumably they were just for IRIX.


>  	olddquot1 = olddquot2 = NULL;
>  	udqp = gdqp = NULL;
>  
> @@ -138,17 +114,17 @@ xfs_setattr(
>  	 * If the IDs do change before we take the ilock, we're covered
>  	 * because the i_*dquot fields will get updated anyway.
>  	 */
> -	if (XFS_IS_QUOTA_ON(mp) && (mask & (XFS_AT_UID|XFS_AT_GID))) {
> +	if (XFS_IS_QUOTA_ON(mp) && (mask & (ATTR_UID|ATTR_GID))) {
>  		uint	qflags = 0;
>  
> -		if ((mask & XFS_AT_UID) && XFS_IS_UQUOTA_ON(mp)) {
> -			uid = vap->va_uid;
> +		if ((mask & ATTR_UID) && XFS_IS_UQUOTA_ON(mp)) {
> +			uid = iattr->ia_uid;
>  			qflags |= XFS_QMOPT_UQUOTA;
>  		} else {
>  			uid = ip->i_d.di_uid;
>  		}
> -		if ((mask & XFS_AT_GID) && XFS_IS_GQUOTA_ON(mp)) {
> -			gid = vap->va_gid;
> +		if ((mask & ATTR_GID) && XFS_IS_GQUOTA_ON(mp)) {
> +			gid = iattr->ia_gid;
>  			qflags |= XFS_QMOPT_GQUOTA;
>  		}  else {
>  			gid = ip->i_d.di_gid;
> @@ -173,10 +149,10 @@ xfs_setattr(
>  	 */
>  	tp = NULL;
>  	lock_flags = XFS_ILOCK_EXCL;
> -	if (flags & ATTR_NOLOCK)
> +	if (flags & XFS_ATTR_NOLOCK)
>  		need_iolock = 0;
> -	if (!(mask & XFS_AT_SIZE)) {
> -		if ((mask != (XFS_AT_CTIME|XFS_AT_ATIME|XFS_AT_MTIME)) ||
> +	if (!(mask & ATTR_SIZE)) {
> +		if ((mask != (ATTR_CTIME|ATTR_ATIME|ATTR_MTIME)) ||
>  		    (mp->m_flags & XFS_MOUNT_WSYNC)) {
>  			tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
>  			commit_flags = 0;
> @@ -189,10 +165,10 @@ xfs_setattr(
>  		}
>  	} else {
>  		if (DM_EVENT_ENABLED(ip, DM_EVENT_TRUNCATE) &&
> -		    !(flags & ATTR_DMI)) {
> +		    !(flags & XFS_ATTR_DMI)) {
>  			int dmflags = AT_DELAY_FLAG(flags) | DM_SEM_FLAG_WR;
>  			code = XFS_SEND_DATA(mp, DM_EVENT_TRUNCATE, ip,
> -				vap->va_size, 0, dmflags, NULL);
> +				iattr->ia_size, 0, dmflags, NULL);
>  			if (code) {
>  				lock_flags = 0;
>  				goto error_return;
> @@ -212,7 +188,7 @@ xfs_setattr(
>  	 * Only the owner or users with CAP_FOWNER
>  	 * capability may do these things.
>  	 */
> -	if (mask & (XFS_AT_MODE|XFS_AT_UID|XFS_AT_GID)) {
> +	if (mask & (ATTR_MODE|ATTR_UID|ATTR_GID)) {
>  		/*
>  		 * CAP_FOWNER overrides the following restrictions:
>  		 *
> @@ -236,21 +212,21 @@ xfs_setattr(
>  		 * IDs of the calling process shall match the group owner of
>  		 * the file when setting the set-group-ID bit on that file
>  		 */
> -		if (mask & XFS_AT_MODE) {
> +		if (mask & ATTR_MODE) {
>  			mode_t m = 0;
>  
> -			if ((vap->va_mode & S_ISUID) && !file_owner)
> +			if ((iattr->ia_mode & S_ISUID) && !file_owner)
>  				m |= S_ISUID;
> -			if ((vap->va_mode & S_ISGID) &&
> +			if ((iattr->ia_mode & S_ISGID) &&
>  			    !in_group_p((gid_t)ip->i_d.di_gid))
>  				m |= S_ISGID;
>  #if 0
>  			/* Linux allows this, Irix doesn't. */
> -			if ((vap->va_mode & S_ISVTX) && !S_ISDIR(ip->i_d.di_mode))
> +			if ((iattr->ia_mode & S_ISVTX) && !S_ISDIR(ip->i_d.di_mode))
>  				m |= S_ISVTX;
>  #endif
>  			if (m && !capable(CAP_FSETID))
> -				vap->va_mode &= ~m;
> +				iattr->ia_mode &= ~m;
>  		}
>  	}
>  
> @@ -261,7 +237,7 @@ xfs_setattr(
>  	 * and can change the group id only to a group of which he
>  	 * or she is a member.
>  	 */
> -	if (mask & (XFS_AT_UID|XFS_AT_GID)) {
> +	if (mask & (ATTR_UID|ATTR_GID)) {
>  		/*
>  		 * These IDs could have changed since we last looked at them.
>  		 * But, we're assured that if the ownership did change
> @@ -270,8 +246,8 @@ xfs_setattr(
>  		 */
>  		iuid = ip->i_d.di_uid;
>  		igid = ip->i_d.di_gid;
> -		gid = (mask & XFS_AT_GID) ? vap->va_gid : igid;
> -		uid = (mask & XFS_AT_UID) ? vap->va_uid : iuid;
> +		gid = (mask & ATTR_GID) ? iattr->ia_gid : igid;
> +		uid = (mask & ATTR_UID) ? iattr->ia_uid : iuid;
>  
>  		/*
>  		 * CAP_CHOWN overrides the following restrictions:
> @@ -308,13 +284,13 @@ xfs_setattr(
>  	/*
>  	 * Truncate file.  Must have write permission and not be a directory.
>  	 */
> -	if (mask & XFS_AT_SIZE) {
> +	if (mask & ATTR_SIZE) {
>  		/* Short circuit the truncate case for zero length files */
> -		if ((vap->va_size == 0) &&
> -		   (ip->i_size == 0) && (ip->i_d.di_nextents == 0)) {
> +		if (iattr->ia_size == 0 &&
> +		    ip->i_size == 0 && ip->i_d.di_nextents == 0) {
>  			xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  			lock_flags &= ~XFS_ILOCK_EXCL;
> -			if (mask & XFS_AT_CTIME)
> +			if (mask & ATTR_CTIME)
>  				xfs_ichgtime(ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
>  			code = 0;
>  			goto error_return;
> @@ -337,9 +313,9 @@ xfs_setattr(
>  	/*
>  	 * Change file access or modified times.
>  	 */
> -	if (mask & (XFS_AT_ATIME|XFS_AT_MTIME)) {
> +	if (mask & (ATTR_ATIME|ATTR_MTIME)) {
>  		if (!file_owner) {
> -			if ((flags & ATTR_UTIME) &&
> +			if ((mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)) &&
>  			    !capable(CAP_FOWNER)) {
>  				code = XFS_ERROR(EPERM);
>  				goto error_return;
> @@ -349,23 +325,22 @@ xfs_setattr(
>  
>  	/*
>  	 * Now we can make the changes.  Before we join the inode
> -	 * to the transaction, if XFS_AT_SIZE is set then take care of
> +	 * to the transaction, if ATTR_SIZE is set then take care of
>  	 * the part of the truncation that must be done without the
>  	 * inode lock.  This needs to be done before joining the inode
>  	 * to the transaction, because the inode cannot be unlocked
>  	 * once it is a part of the transaction.
>  	 */
> -	if (mask & XFS_AT_SIZE) {
> +	if (mask & ATTR_SIZE) {
>  		code = 0;
> -		if ((vap->va_size > ip->i_size) &&
> -		    (flags & ATTR_NOSIZETOK) == 0) {
> +		if (iattr->ia_size > ip->i_size) {

Yeah, so we no longer support ATTR_NOSIZETOK (presumably for cxfs).

>  			/*
>  			 * Do the first part of growing a file: zero any data
>  			 * in the last block that is beyond the old EOF.  We
>  			 * need to do this before the inode is joined to the
>  			 * transaction to modify the i_size.
>  			 */
> -			code = xfs_zero_eof(ip, vap->va_size, ip->i_size);
> +			code = xfs_zero_eof(ip, iattr->ia_size, ip->i_size);
>  		}
>  		xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  
> @@ -382,10 +357,10 @@ xfs_setattr(
>  		 * not within the range we care about here.
>  		 */
>  		if (!code &&
> -		    (ip->i_size != ip->i_d.di_size) &&
> -		    (vap->va_size > ip->i_d.di_size)) {
> +		    ip->i_size != ip->i_d.di_size &&
> +		    iattr->ia_size > ip->i_d.di_size) {
>  			code = xfs_flush_pages(ip,
> -					ip->i_d.di_size, vap->va_size,
> +					ip->i_d.di_size, iattr->ia_size,
>  					XFS_B_ASYNC, FI_NONE);
>  		}
>  
> @@ -393,7 +368,7 @@ xfs_setattr(
>  		vn_iowait(ip);
>  
>  		if (!code)
> -			code = xfs_itruncate_data(ip, vap->va_size);
> +			code = xfs_itruncate_data(ip, iattr->ia_size);
>  		if (code) {
>  			ASSERT(tp == NULL);
>  			lock_flags &= ~XFS_ILOCK_EXCL;
> @@ -422,31 +397,30 @@ xfs_setattr(
>  	/*
>  	 * Truncate file.  Must have write permission and not be a directory.
>  	 */
> -	if (mask & XFS_AT_SIZE) {
> +	if (mask & ATTR_SIZE) {
>  		/*
>  		 * Only change the c/mtime if we are changing the size
>  		 * or we are explicitly asked to change it. This handles
>  		 * the semantic difference between truncate() and ftruncate()
>  		 * as implemented in the VFS.
>  		 */
> -		if (vap->va_size != ip->i_size || (mask & XFS_AT_CTIME))
> +		if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
>  			timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
>  
> -		if (vap->va_size > ip->i_size) {
> -			ip->i_d.di_size = vap->va_size;
> -			ip->i_size = vap->va_size;
> -			if (!(flags & ATTR_DMI))
> +		if (iattr->ia_size > ip->i_size) {
> +			ip->i_d.di_size = iattr->ia_size;
> +			ip->i_size = iattr->ia_size;
> +			if (!(flags & XFS_ATTR_DMI))
>  				xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
>  			xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> -		} else if ((vap->va_size <= ip->i_size) ||
> -			   ((vap->va_size == 0) && ip->i_d.di_nextents)) {
> +		} else if (iattr->ia_size <= ip->i_size ||
> +			   (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
>  			/*
>  			 * signal a sync transaction unless
>  			 * we're truncating an already unlinked
>  			 * file on a wsync filesystem
>  			 */
> -			code = xfs_itruncate_finish(&tp, ip,
> -					    (xfs_fsize_t)vap->va_size,
> +			code = xfs_itruncate_finish(&tp, ip, iattr->ia_size,
>  					    XFS_DATA_FORK,
>  					    ((ip->i_d.di_nlink != 0 ||
>  					      !(mp->m_flags & XFS_MOUNT_WSYNC))
> @@ -468,9 +442,9 @@ xfs_setattr(
>  	/*
>  	 * Change file access modes.
>  	 */
> -	if (mask & XFS_AT_MODE) {
> +	if (mask & ATTR_MODE) {
>  		ip->i_d.di_mode &= S_IFMT;
> -		ip->i_d.di_mode |= vap->va_mode & ~S_IFMT;
> +		ip->i_d.di_mode |= iattr->ia_mode & ~S_IFMT;
>  
>  		xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
>  		timeflags |= XFS_ICHGTIME_CHG;
> @@ -483,7 +457,7 @@ xfs_setattr(
>  	 * and can change the group id only to a group of which he
>  	 * or she is a member.
>  	 */
> -	if (mask & (XFS_AT_UID|XFS_AT_GID)) {
> +	if (mask & (ATTR_UID|ATTR_GID)) {
>  		/*
>  		 * CAP_FSETID overrides the following restrictions:
>  		 *
> @@ -501,7 +475,7 @@ xfs_setattr(
>  		 */
>  		if (iuid != uid) {
>  			if (XFS_IS_UQUOTA_ON(mp)) {
> -				ASSERT(mask & XFS_AT_UID);
> +				ASSERT(mask & ATTR_UID);
>  				ASSERT(udqp);
>  				olddquot1 = XFS_QM_DQVOPCHOWN(mp, tp, ip,
>  							&ip->i_udquot, udqp);
> @@ -511,7 +485,7 @@ xfs_setattr(
>  		if (igid != gid) {
>  			if (XFS_IS_GQUOTA_ON(mp)) {
>  				ASSERT(!XFS_IS_PQUOTA_ON(mp));
> -				ASSERT(mask & XFS_AT_GID);
> +				ASSERT(mask & ATTR_GID);
>  				ASSERT(gdqp);
>  				olddquot2 = XFS_QM_DQVOPCHOWN(mp, tp, ip,
>  							&ip->i_gdquot, gdqp);
> @@ -527,31 +501,31 @@ xfs_setattr(
>  	/*
>  	 * Change file access or modified times.
>  	 */
> -	if (mask & (XFS_AT_ATIME|XFS_AT_MTIME)) {
> -		if (mask & XFS_AT_ATIME) {
> -			ip->i_d.di_atime.t_sec = vap->va_atime.tv_sec;
> -			ip->i_d.di_atime.t_nsec = vap->va_atime.tv_nsec;
> +	if (mask & (ATTR_ATIME|ATTR_MTIME)) {
> +		if (mask & ATTR_ATIME) {
> +			ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> +			ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
>  			ip->i_update_core = 1;
>  			timeflags &= ~XFS_ICHGTIME_ACC;
>  		}
> -		if (mask & XFS_AT_MTIME) {
> -			ip->i_d.di_mtime.t_sec = vap->va_mtime.tv_sec;
> -			ip->i_d.di_mtime.t_nsec = vap->va_mtime.tv_nsec;
> +		if (mask & ATTR_MTIME) {
> +			ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> +			ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
>  			timeflags &= ~XFS_ICHGTIME_MOD;
>  			timeflags |= XFS_ICHGTIME_CHG;
>  		}
> -		if (tp && (flags & ATTR_UTIME))
> +		if (tp && (mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)))
>  			xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
>  	}
>  
>  	/*
> -	 * Change file inode change time only if XFS_AT_CTIME set
> +	 * Change file inode change time only if ATTR_CTIME set
>  	 * AND we have been called by a DMI function.
>  	 */
>  
> -	if ( (flags & ATTR_DMI) && (mask & XFS_AT_CTIME) ) {
> -		ip->i_d.di_ctime.t_sec = vap->va_ctime.tv_sec;
> -		ip->i_d.di_ctime.t_nsec = vap->va_ctime.tv_nsec;
> +	if ((flags & XFS_ATTR_DMI) && (mask & ATTR_CTIME)) {
> +		ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
> +		ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
>  		ip->i_update_core = 1;
>  		timeflags &= ~XFS_ICHGTIME_CHG;
>  	}
> @@ -560,7 +534,7 @@ xfs_setattr(
>  	 * Send out timestamp changes that need to be set to the
>  	 * current time.  Not done when called by a DMI function.
>  	 */
> -	if (timeflags && !(flags & ATTR_DMI))
> +	if (timeflags && !(flags & XFS_ATTR_DMI))
>  		xfs_ichgtime(ip, timeflags);
>  
>  	XFS_STATS_INC(xs_ig_attrchg);
> @@ -598,7 +572,7 @@ xfs_setattr(
>  	}
>  
>  	if (DM_EVENT_ENABLED(ip, DM_EVENT_ATTRIBUTE) &&
> -	    !(flags & ATTR_DMI)) {
> +	    !(flags & XFS_ATTR_DMI)) {
>  		(void) XFS_SEND_NAMESP(mp, DM_EVENT_ATTRIBUTE, ip, DM_RIGHT_NULL,
>  					NULL, DM_RIGHT_NULL, NULL, NULL,
>  					0, 0, AT_DELAY_FLAG(flags));
> @@ -3033,7 +3007,7 @@ xfs_alloc_file_space(
>  
>  	/*	Generate a DMAPI event if needed.	*/
>  	if (alloc_type != 0 && offset < ip->i_size &&
> -			(attr_flags&ATTR_DMI) == 0  &&
> +			(attr_flags & XFS_ATTR_DMI) == 0  &&
>  			DM_EVENT_ENABLED(ip, DM_EVENT_WRITE)) {
>  		xfs_off_t           end_dmi_offset;
>  
> @@ -3147,7 +3121,7 @@ retry:
>  		allocatesize_fsb -= allocated_fsb;
>  	}
>  dmapi_enospc_check:
> -	if (error == ENOSPC && (attr_flags & ATTR_DMI) == 0 &&
> +	if (error == ENOSPC && (attr_flags & XFS_ATTR_DMI) == 0 &&
>  	    DM_EVENT_ENABLED(ip, DM_EVENT_NOSPACE)) {
>  		error = XFS_SEND_NAMESP(mp, DM_EVENT_NOSPACE,
>  				ip, DM_RIGHT_NULL,
> @@ -3294,7 +3268,7 @@ xfs_free_file_space(
>  	end_dmi_offset = offset + len;
>  	endoffset_fsb = XFS_B_TO_FSBT(mp, end_dmi_offset);
>  
> -	if (offset < ip->i_size && (attr_flags & ATTR_DMI) == 0 &&
> +	if (offset < ip->i_size && (attr_flags & XFS_ATTR_DMI) == 0 &&
>  	    DM_EVENT_ENABLED(ip, DM_EVENT_WRITE)) {
>  		if (end_dmi_offset > ip->i_size)
>  			end_dmi_offset = ip->i_size;
> @@ -3305,7 +3279,7 @@ xfs_free_file_space(
>  			return error;
>  	}
>  
> -	if (attr_flags & ATTR_NOLOCK)
> +	if (attr_flags & XFS_ATTR_NOLOCK)
>  		need_iolock = 0;
>  	if (need_iolock) {
>  		xfs_ilock(ip, XFS_IOLOCK_EXCL);
> @@ -3482,7 +3456,7 @@ xfs_change_file_space(
>  	xfs_off_t	startoffset;
>  	xfs_off_t	llen;
>  	xfs_trans_t	*tp;
> -	bhv_vattr_t	va;
> +	struct iattr	iattr;
>  
>  	xfs_itrace_entry(ip);
>  
> @@ -3556,10 +3530,10 @@ xfs_change_file_space(
>  				break;
>  		}
>  
> -		va.va_mask = XFS_AT_SIZE;
> -		va.va_size = startoffset;
> +		iattr.ia_valid = ATTR_SIZE;
> +		iattr.ia_size = startoffset;
>  
> -		error = xfs_setattr(ip, &va, attr_flags, credp);
> +		error = xfs_setattr(ip, &iattr, attr_flags, credp);
>  
>  		if (error)
>  			return error;
> @@ -3589,7 +3563,7 @@ xfs_change_file_space(
>  	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ihold(tp, ip);
>  
> -	if ((attr_flags & ATTR_DMI) == 0) {
> +	if ((attr_flags & XFS_ATTR_DMI) == 0) {
>  		ip->i_d.di_mode &= ~S_ISUID;
>  
>  		/*
> Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.h
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_vnodeops.h	2008-07-04 14:58:36.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_vnodeops.h	2008-07-04 14:59:21.000000000 +0200
> @@ -2,9 +2,9 @@
>  #define _XFS_VNODEOPS_H 1
>  
>  struct attrlist_cursor_kern;
> -struct bhv_vattr;
>  struct cred;
>  struct file;
> +struct iattr;
>  struct inode;
>  struct iovec;
>  struct kiocb;
> @@ -15,8 +15,12 @@ struct xfs_iomap;
>  
>  
>  int xfs_open(struct xfs_inode *ip);
> -int xfs_setattr(struct xfs_inode *ip, struct bhv_vattr *vap, int flags,
> +int xfs_setattr(struct xfs_inode *ip, struct iattr *vap, int flags,
>  		struct cred *credp);
> +#define	XFS_ATTR_DMI		0x01	/* invocation from a DMI function */
> +#define	XFS_ATTR_NONBLOCK	0x02	/* return EAGAIN if operation would block */
> +#define XFS_ATTR_NOLOCK		0x04	/* Don't grab any conflicting locks */
> +

So we don't bring thru: ATTR_UTIME, ATTR_LAZY, ATTR_NOSIZETOK


>  int xfs_readlink(struct xfs_inode *ip, char *link);
>  int xfs_fsync(struct xfs_inode *ip);
>  int xfs_release(struct xfs_inode *ip);
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c	2008-07-04 14:59:14.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c	2008-07-04 14:59:21.000000000 +0200
> @@ -685,9 +685,9 @@ xfs_ioc_space(
>  		return -XFS_ERROR(EFAULT);
>  
>  	if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> -		attr_flags |= ATTR_NONBLOCK;
> +		attr_flags |= XFS_ATTR_NONBLOCK;
>  	if (ioflags & IO_INVIS)
> -		attr_flags |= ATTR_DMI;
> +		attr_flags |= XFS_ATTR_DMI;
>  
>  	error = xfs_change_file_space(ip, cmd, &bf, filp->f_pos,
>  					      NULL, attr_flags);
> Index: linux-2.6-xfs/fs/xfs/xfs_dmapi.h
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_dmapi.h	2008-07-04 14:58:36.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_dmapi.h	2008-07-04 14:59:21.000000000 +0200
> @@ -166,6 +166,6 @@ typedef enum {
>  
>  #define FILP_DELAY_FLAG(filp) ((filp->f_flags&(O_NDELAY|O_NONBLOCK)) ? \
>  			DM_FLAGS_NDELAY : 0)
> -#define AT_DELAY_FLAG(f) ((f&ATTR_NONBLOCK) ? DM_FLAGS_NDELAY : 0)
> +#define AT_DELAY_FLAG(f) ((f & XFS_ATTR_NONBLOCK) ? DM_FLAGS_NDELAY : 0)
>  
>  #endif  /* __XFS_DMAPI_H__ */
> Index: linux-2.6-xfs/fs/xfs/xfs_acl.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_acl.c	2008-07-04 15:01:44.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_acl.c	2008-07-04 15:02:47.000000000 +0200
> @@ -720,7 +720,7 @@ xfs_acl_setmode(
>  	xfs_acl_t	*acl,
>  	int		*basicperms)
>  {
> -	bhv_vattr_t	va;
> +	struct iattr	iattr;
>  	xfs_acl_entry_t	*ap;
>  	xfs_acl_entry_t	*gap = NULL;
>  	int		i, nomask = 1;
> @@ -734,25 +734,25 @@ xfs_acl_setmode(
>  	 * Copy the u::, g::, o::, and m:: bits from the ACL into the
>  	 * mode.  The m:: bits take precedence over the g:: bits.
>  	 */
> -	va.va_mask = XFS_AT_MODE;
> -	va.va_mode = xfs_vtoi(vp)->i_d.di_mode;
> -	va.va_mode &= ~(S_IRWXU|S_IRWXG|S_IRWXO);
> +	iattr.ia_mask = XFS_AT_MODE;
> +	iattr.ia_mode = xfs_vtoi(vp)->i_d.di_mode;
> +	iattr.ia_mode &= ~(S_IRWXU|S_IRWXG|S_IRWXO);
>  	ap = acl->acl_entry;
>  	for (i = 0; i < acl->acl_cnt; ++i) {
>  		switch (ap->ae_tag) {
>  		case ACL_USER_OBJ:
> -			va.va_mode |= ap->ae_perm << 6;
> +			iattr.ia_mode |= ap->ae_perm << 6;
>  			break;
>  		case ACL_GROUP_OBJ:
>  			gap = ap;
>  			break;
>  		case ACL_MASK:	/* more than just standard modes */
>  			nomask = 0;
> -			va.va_mode |= ap->ae_perm << 3;
> +			iattr.ia_mode |= ap->ae_perm << 3;
>  			*basicperms = 0;
>  			break;
>  		case ACL_OTHER:
> -			va.va_mode |= ap->ae_perm;
> +			iattr.ia_mode |= ap->ae_perm;
>  			break;
>  		default:	/* more than just standard modes */
>  			*basicperms = 0;
> @@ -763,9 +763,9 @@ xfs_acl_setmode(
>  
>  	/* Set the group bits from ACL_GROUP_OBJ if there's no ACL_MASK */
>  	if (gap && nomask)
> -		va.va_mode |= gap->ae_perm << 3;
> +		iattr.ia_mode |= gap->ae_perm << 3;
>  
> -	return xfs_setattr(xfs_vtoi(vp), &va, 0, sys_cred);
> +	return xfs_setattr(xfs_vtoi(vp), &iattr, 0, sys_cred);
>  }
>  
>  /*
>