Skip to contentSkip to conversation

kill the v_flag member in struct bhv_vnode

PATCH In review Christoph Hellwig 3 participants 3 messages Aug 23, 2007 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 4/17 kill the v_flag member in struct bhv_vnode

All flags previous handled at the vnode level are not in the xfs_inode where we already have a flags mechanisms and free bits for flags previously in the vnode.

fs/xfs/dmapi/xfs_dm.c +4 -6
Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
===================================================================
@@ -2599,7 +2599,7 @@ xfs_dm_punch_hole(
 	/* Let threads in send_data_event know we punched the file. */
 	ip->i_iocore.io_dmstate++;
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-	VMODIFY(vp);
+	xfs_iflags_set(ip, XFS_IMODIFIED);
 
 up_and_out:
 	up_rw_sems(inode, DM_SEM_FLAG_WR);
@@ -2988,7 +2988,7 @@ xfs_dm_sync_by_handle(
 	dm_right_t	right)
 {
 	int		err, ret;
-	bhv_vnode_t	*vp = vn_from_inode(inode);
+	xfs_inode_t	*ip = XFS_I(inode);
 
 	/* Returns negative errors to DMAPI */
 	if (right < DM_RIGHT_EXCL)
@@ -2997,7 +2997,7 @@ xfs_dm_sync_by_handle(
 	/* We need to protect against concurrent writers.. */
 	ret = filemap_fdatawrite(inode->i_mapping);
 	down_rw_sems(inode, DM_FLAGS_IMUX);
-	err = xfs_fsync(XFS_I(inode), FSYNC_WAIT, 0, -1);
+	err = xfs_fsync(ip, FSYNC_WAIT, 0, -1);
 	if (!ret)
 		ret = err;
 	up_rw_sems(inode, DM_FLAGS_IMUX);
@@ -3005,9 +3005,7 @@ xfs_dm_sync_by_handle(
 	if (!ret)
 		ret = err;
 
-	if (VN_TRUNC(vp))
-		VUNTRUNCATE(vp);
-
+	xfs_iflags_clear(ip, XFS_ITRUNCATED);
 	if (ret > 0)
 		ret = -ret;	/* Return negative errors to DMAPI */
 
fs/xfs/linux-2.6/xfs_aops.c +4 -7
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
@@ -303,13 +303,13 @@ xfs_map_blocks(
 	xfs_iomap_t		*mapp,
 	int			flags)
 {
-	bhv_vnode_t		*vp = vn_from_inode(inode);
+	xfs_inode_t		*ip = XFS_I(inode);
 	int			error, nmaps = 1;
 
-	error = xfs_bmap(xfs_vtoi(vp), offset, count,
+	error = xfs_bmap(ip, offset, count,
 				flags, mapp, &nmaps);
 	if (!error && (flags & (BMAPI_WRITE|BMAPI_ALLOCATE)))
-		VMODIFY(vp);
+		xfs_iflags_set(ip, XFS_IMODIFIED);
 	return -error;
 }
 
@@ -1245,10 +1245,7 @@ xfs_vm_writepages(
 	struct address_space	*mapping,
 	struct writeback_control *wbc)
 {
-	struct bhv_vnode	*vp = vn_from_inode(mapping->host);
-
-	if (VN_TRUNC(vp))
-		VUNTRUNCATE(vp);
+	xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED);
 	return generic_writepages(mapping, wbc);
 }
 
fs/xfs/linux-2.6/xfs_file.c +3 -7
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_file.c
===================================================================
@@ -217,13 +217,11 @@ xfs_file_fsync(
 	struct dentry	*dentry,
 	int		datasync)
 {
-	bhv_vnode_t	*vp = vn_from_inode(dentry->d_inode);
 	int		flags = FSYNC_WAIT;
 
 	if (datasync)
 		flags |= FSYNC_DATA;
-	if (VN_TRUNC(vp))
-		VUNTRUNCATE(vp);
+	xfs_iflags_clear(XFS_I(dentry->d_inode), XFS_ITRUNCATED);
 	return -xfs_fsync(XFS_I(dentry->d_inode), flags,
 			(xfs_off_t)0, (xfs_off_t)-1);
 }
@@ -300,10 +298,9 @@ xfs_file_ioctl(
 {
 	int		error;
 	struct inode	*inode = filp->f_path.dentry->d_inode;
-	bhv_vnode_t	*vp = vn_from_inode(inode);
 
 	error = xfs_ioctl(XFS_I(inode), filp, 0, cmd, (void __user *)p);
-	VMODIFY(vp);
+	xfs_iflags_set(XFS_I(inode), XFS_IMODIFIED);
 
 	/* NOTE:  some of the ioctl's return positive #'s as a
 	 *	  byte count indicating success, such as
@@ -322,10 +319,9 @@ xfs_file_ioctl_invis(
 {
 	int		error;
 	struct inode	*inode = filp->f_path.dentry->d_inode;
-	bhv_vnode_t	*vp = vn_from_inode(inode);
 
 	error = xfs_ioctl(XFS_I(inode), filp, IO_INVIS, cmd, (void __user *)p);
-	VMODIFY(vp);
+	xfs_iflags_set(XFS_I(inode), XFS_IMODIFIED);
 
 	/* NOTE:  some of the ioctl's return positive #'s as a
 	 *	  byte count indicating success, such as
fs/xfs/linux-2.6/xfs_fs_subr.c +2 -4
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_fs_subr.c
===================================================================
@@ -61,8 +61,7 @@ xfs_flushinval_pages(
 	int		ret = 0;
 
 	if (VN_CACHED(vp)) {
-		if (VN_TRUNC(vp))
-			VUNTRUNCATE(vp);
+		xfs_iflags_clear(ip, XFS_ITRUNCATED);
 		ret = filemap_write_and_wait(inode->i_mapping);
 		if (!ret)
 			truncate_inode_pages(inode->i_mapping, first);
@@ -84,8 +83,7 @@ xfs_flush_pages(
 	int		ret2;
 
 	if (VN_DIRTY(vp)) {
-		if (VN_TRUNC(vp))
-			VUNTRUNCATE(vp);
+		xfs_iflags_clear(ip, XFS_ITRUNCATED);
 		ret = filemap_fdatawrite(inode->i_mapping);
 		if (flags & XFS_B_ASYNC)
 			return ret;
fs/xfs/linux-2.6/xfs_ioctl32.c +1 -2
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl32.c
===================================================================
@@ -371,7 +371,6 @@ xfs_compat_ioctl(
 	unsigned long	arg)
 {
 	struct inode	*inode = file->f_path.dentry->d_inode;
-	bhv_vnode_t	*vp = vn_from_inode(inode);
 	int		error;
 
 	switch (cmd) {
@@ -459,7 +458,7 @@ xfs_compat_ioctl(
 	}
 
 	error = xfs_ioctl(XFS_I(inode), file, mode, cmd, (void __user *)arg);
-	VMODIFY(vp);
+	xfs_iflags_set(XFS_I(inode), XFS_IMODIFIED);
 
 	return error;
 }
fs/xfs/linux-2.6/xfs_iops.c +3 -3
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
@@ -221,7 +221,7 @@ xfs_init_security(
 	error = xfs_attr_set(XFS_I(ip), name, value,
 			length, ATTR_SECURE);
 	if (!error)
-		VMODIFY(vp);
+		xfs_iflags_set(XFS_I(ip), XFS_IMODIFIED);
 
 	kfree(name);
 	kfree(value);
@@ -327,7 +327,7 @@ xfs_vn_mknod(
 		if (!error) {
 			error = _ACL_INHERIT(vp, &vattr, default_acl);
 			if (!error)
-				VMODIFY(vp);
+				xfs_iflags_set(XFS_I(&vp->v_inode), XFS_IMODIFIED);
 			else
 				xfs_cleanup_inode(dir, vp, dentry, mode);
 		}
@@ -409,7 +409,7 @@ xfs_vn_link(
 	if (unlikely(error)) {
 		VN_RELE(vp);
 	} else {
-		VMODIFY(vn_from_inode(dir));
+		xfs_iflags_set(XFS_I(dir), XFS_IMODIFIED);
 		xfs_validate_fields(ip, &vattr);
 		d_instantiate(dentry, ip);
 	}
fs/xfs/linux-2.6/xfs_super.c +4 -5
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
===================================================================
@@ -197,7 +197,7 @@ xfs_revalidate_inode(
 		inode->i_flags |= S_NOATIME;
 	else
 		inode->i_flags &= ~S_NOATIME;
-	vp->v_flag &= ~VMODIFIED;
+	xfs_iflags_clear(ip, XFS_IMODIFIED);
 }
 
 void
@@ -448,13 +448,12 @@ xfs_fs_clear_inode(
 	if (XFS_I(inode))
 		xfs_inactive(XFS_I(inode));
 
-	VN_LOCK(vp);
-	vp->v_flag &= ~VMODIFIED;
-	VN_UNLOCK(vp, 0);
 
-	if (XFS_I(inode))
+	if (XFS_I(inode)) {
+		xfs_iflags_clear(XFS_I(inode), XFS_IMODIFIED);
 		if (xfs_reclaim(XFS_I(inode)))
 			panic("%s: cannot reclaim 0x%p\n", __FUNCTION__, vp);
+	}
 
 	ASSERT(XFS_I(inode) == NULL);
 
fs/xfs/linux-2.6/xfs_vnode.c +2 -7
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_vnode.c
===================================================================
@@ -84,9 +84,6 @@ vn_initialize(
 	XFS_STATS_INC(vn_active);
 	XFS_STATS_INC(vn_alloc);
 
-	vp->v_flag = VMODIFIED;
-	spinlock_init(&vp->v_lock, "v_lock");
-
 	spin_lock(&vnumber_lock);
 	if (!++vn_generation)	/* v_number shouldn't be zero */
 		vn_generation++;
@@ -157,7 +154,7 @@ __vn_revalidate(
 	error = xfs_getattr(xfs_vtoi(vp), vattr, 0);
 	if (likely(!error)) {
 		vn_revalidate_core(vp, vattr);
-		VUNMODIFY(vp);
+		xfs_iflags_clear(xfs_vtoi(vp), XFS_IMODIFIED);
 	}
 	return -error;
 }
@@ -182,10 +179,8 @@ vn_hold(
 
 	XFS_STATS_INC(vn_hold);
 
-	VN_LOCK(vp);
 	inode = igrab(vn_to_inode(vp));
 	ASSERT(inode);
-	VN_UNLOCK(vp, 0);
 
 	return vp;
 }
@@ -199,7 +194,7 @@ vn_hold(
 /*  2 */		(void *)(__psint_t) line,		\
 /*  3 */		(void *)(__psint_t)(vn_count(vp)),	\
 /*  4 */		(void *)(ra),				\
-/*  5 */		(void *)(__psunsigned_t)(vp)->v_flag,	\
+/*  5 */		NULL,					\
 /*  6 */		(void *)(__psint_t)current_cpu(),	\
 /*  7 */		(void *)(__psint_t)current_pid(),	\
 /*  8 */		(void *)__return_address,		\
fs/xfs/linux-2.6/xfs_vnode.h +0 -42

62 lines not on this page · raw message

fs/xfs/xfs_iget.c +1 -0
Index: linux-2.6-xfs/fs/xfs/xfs_iget.c
===================================================================
@@ -346,6 +346,7 @@ finish_inode:
 	ASSERT(((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) != 0) ==
 	       ((ip->i_iocore.io_flags & XFS_IOCORE_RT) != 0));
 
+	xfs_iflags_set(ip, XFS_IMODIFIED);
 	*ipp = ip;
 
 	/*
fs/xfs/xfs_inode.h +3 -0
Index: linux-2.6-xfs/fs/xfs/xfs_inode.h
===================================================================
@@ -392,6 +392,9 @@ xfs_iflags_test(xfs_inode_t *ip, unsigne
 #define XFS_IRECLAIMABLE 0x0020 /* inode can be reclaimed */
 #define XFS_INEW	0x0040
 #define XFS_IFILESTREAM	0x0080	/* inode is in a filestream directory */
+#define XFS_IMODIFIED	0x0100	/* XFS inode state possibly differs */
+				/* to the Linux inode state. */
+#define XFS_ITRUNCATED	0x0200	/* truncated down so flush-on-close */
 
 /*
  * Flags for inode locking.
fs/xfs/xfs_vnodeops.c +10 -2
Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
===================================================================
@@ -660,7 +660,7 @@ xfs_setattr(
 			 * vnode and flush it when the file is closed, and
 			 * do not wait the usual (long) time for writeout.
 			 */
-			VTRUNCATE(vp);
+			xfs_iflags_set(ip, XFS_ITRUNCATED);
 		}
 		/*
 		 * Have to do this even if the file's size doesn't change.
@@ -1516,6 +1516,8 @@ xfs_release(
 		return 0;
 
 	if (!XFS_FORCED_SHUTDOWN(mp)) {
+		int truncated;
+
 		/*
 		 * If we are using filestreams, and we have an unlinked
 		 * file that we are processing the last close on, then nothing
@@ -1536,7 +1538,13 @@ xfs_release(
 		 * significantly reducing the time window where we'd otherwise
 		 * be exposed to that problem.
 		 */
-		if (VUNTRUNCATE(vp) && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
+		spin_lock(&ip->i_flags_lock);
+		truncated = __xfs_iflags_test(ip, XFS_ITRUNCATED);
+		if (truncated)
+			ip->i_flags &= ~XFS_ITRUNCATED;
+		spin_unlock(&ip->i_flags_lock);
+
+		if (truncated && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
 			xfs_flush_pages(ip, 0, -1, XFS_B_ASYNC, FI_NONE);
 	}
 
fs/xfs/xfsidbg.c +2 -44

70 lines not on this page · raw message

#1 lore raw
Quoted 2 lines · Christoph Hellwig

All flags previous handled at the vnode level are not in the xfs_inode

                                                    ^^^

typo :)

Quoted 2 lines

where we already have a flags mechanisms and free bits for flags previously in the vnode.

Yeah, didn't notice anything else...

Jeff.

Signature · 1 line
The box said "Windows XP or better required". So I installed Linux.
#2 lore raw
In reply to Christoph Hellwig
Quoted 7 lines · Christoph Hellwig

All flags previous handled at the vnode level are not in the xfs_inode where we already have a flags mechanisms and free bits for flags previously in the vnode.

....

Quoted 4 lines

Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
===================================================================

fs/xfs/xfs_vnodeops.c +0 -0

....

Quoted 4 lines
fs/xfs/xfs_vnodeops.c +0 -0
@@ -1536,7 +1538,13 @@ xfs_release(
 		 * significantly reducing the time window where we'd otherwise
 		 * be exposed to that problem.
 		 */

fs/xfs/xfs_vnodeops.c xfs_release( · line 1549

Quoted 10 lines
fs/xfs/xfs_vnodeops.c +7 -1
-		if (VUNTRUNCATE(vp) && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
+		spin_lock(&ip->i_flags_lock);
+		truncated = __xfs_iflags_test(ip, XFS_ITRUNCATED);
+		if (truncated)
+			ip->i_flags &= ~XFS_ITRUNCATED;
+		spin_unlock(&ip->i_flags_lock);
+
+		if (truncated && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
 			xfs_flush_pages(ip, 0, -1, XFS_B_ASYNC, FI_NONE);
 	}

This is kind of icky - doing an open coded flag clear instead of wrapping it in a xfs_iflags_test_and_clear() type operation. Something like:

static inline int xfs_iflags_test_and_clear(xfs_inode_t *ip, unsigned short flags)

{
	int	ret;

	spin_lock(&ip->i_flags_lock);
	ret = ip->i_flags & flags;
	if (ret)
		ip->i_flags &= ~flags;
	spin_unlock(&ip->i_flags_lock);
	return ret;
}

And then the code can become:

	if (xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED) &&
	    VN_DIRTY(vp) && ip->i_delayed_blks > 0))
		xfs_flush_pages(ip, 0, -1, XFS_B_ASYNC, FI_NONE);

FWIW, for changes to this series I think incremental patches would probably be the easiest way to handle it.

Cheers,

Dave.

Signature · 3 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group