From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: with ECARTIS (v1.0.0; list xfs); Wed, 09 Jul 2008 01:57:35 -0700 (PDT) Received: from larry.melbourne.sgi.com (larry.melbourne.sgi.com [134.14.52.130]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with SMTP id m698vQEb004104 for ; Wed, 9 Jul 2008 01:57:28 -0700 Message-ID: <48747DAD.7060501@sgi.com> Date: Wed, 09 Jul 2008 18:58:21 +1000 From: Timothy Shimmin MIME-Version: 1.0 Subject: Re: [PATCH 2/3] simplify xfs_setattr References: <20080627154557.GB31476@lst.de> <20080705172021.GA7177@lst.de> In-Reply-To: <20080705172021.GA7177@lst.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: xfs-bounce@oss.sgi.com Errors-to: xfs-bounce@oss.sgi.com List-Id: xfs To: Christoph Hellwig Cc: xfs@oss.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 > > 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); > } > > /* >