Skip to contentSkip to conversation

cleanup fix handling

PATCH In review Christoph Hellwig 2 participants 3 messages Dec 14, 2007 – Dec 17, 2007 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 cleanup fix handling

Cleanup various fid related bits:

- merge xfs_fid2 into it's only caller xfs_dm_inode_to_fh.
- remove xfs_vget and opencode it in the two callers, simplifying both of them by avoiding the awkward calling convetion.
- assign directly to the dm_fid_t members in various places in the dmapi code instead of casting them to xfs_fid_t first (which is identical to dm_fid_t)

fs/xfs/dmapi/xfs_dm.c +50 -27
Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
===================================================================
@@ -586,14 +586,12 @@ dm_dip_to_handle(
 	dm_handle_t	*handlep)
 {
 	dm_fid_t	fid;
-	struct xfs_fid	*xfid;
 	int		hsize;
 
-	xfid = (struct xfs_fid *)&fid;
-	xfid->fid_len = sizeof(struct xfs_fid) - sizeof(xfid->fid_len);
-	xfid->fid_pad = 0;
-	xfid->fid_ino = ino;
-        xfid->fid_gen = be32_to_cpu(dip->di_core.di_gen);
+	fid.dm_fid_len = sizeof(struct dm_fid) - sizeof(fid.dm_fid_len);
+	fid.dm_fid_pad = 0;
+	fid.dm_fid_ino = ino;
+	fid.dm_fid_gen = be32_to_cpu(dip->di_core.di_gen);
 
 	memcpy(&handlep->ha_fsid, fsid, sizeof(*fsid));
 	memcpy(&handlep->ha_fid, &fid, fid.dm_fid_len + sizeof(fid.dm_fid_len));
@@ -3273,27 +3271,49 @@ xfs_dm_get_invis_ops(
 STATIC int
 xfs_dm_fh_to_inode(
 	struct super_block	*sb,
-	struct inode		**ip,
+	struct inode		**inode,
 	dm_fid_t		*dmfid)
 {
-	bhv_vnode_t	*vp = NULL;
-	xfs_mount_t	*mp = XFS_M(sb);
-	int		error;
-	struct xfs_fid	xfid;
+	xfs_mount_t		*mp = XFS_M(sb);
+	xfs_inode_t		*ip;
+	xfs_ino_t		ino;
+	unsigned int		igen;
+	int			error;
 
-	/* Returns negative errors to DMAPI */
+	*inode = NULL;
 
-	*ip = NULL;
-	memcpy(&xfid, dmfid, sizeof(*dmfid));
-	if (xfid.fid_len) {	/* file object handle */
-		error = xfs_vget(mp, &vp, &xfid);
+	if (!dmfid->dm_fid_len) {
+		/* filesystem handle */
+		*inode = igrab(mp->m_rootip->i_vnode);
+		if (!*inode)
+			return -ENOENT;
+		return 0;
 	}
-	else {			/* filesystem handle */
-		error = xfs_root(mp, &vp);
+
+	if (dmfid->dm_fid_len != sizeof(*dmfid) - sizeof(dmfid->dm_fid_len))
+		return -EINVAL;
+
+	ino  = dmfid->dm_fid_ino;
+	igen = dmfid->dm_fid_gen;
+
+	/* fail requests for ino 0 gracefully. */
+	if (ino == 0)
+		return -ESTALE;
+
+	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
+	if (error)
+		return -error;
+	if (!ip)
+		return -EIO;
+
+	if (!ip->i_d.di_mode || ip->i_d.di_gen != igen) {
+		xfs_iput_new(ip, XFS_ILOCK_SHARED);
+		return -ENOENT;
 	}
-	if (vp && (error == 0))
-		*ip = vn_to_inode(vp);
-	return -error; /* Return negative error to DMAPI */
+
+	*inode = ip->i_vnode;
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	return 0;
 }
 
 STATIC int
@@ -3303,18 +3323,21 @@ xfs_dm_inode_to_fh(
 	dm_fsid_t		*dmfsid)
 {
 	xfs_inode_t		*ip = XFS_I(inode);
-	int			error;
-	struct xfs_fid		xfid;
 
 	/* Returns negative errors to DMAPI */
 
 	if (ip->i_mount->m_fixedfsid == NULL)
 		return -EINVAL;
-	error = xfs_fid2(ip, &xfid);
-	if (error)
-		return -error; /* Return negative error to DMAPI */
 
-	memcpy(dmfid, &xfid, sizeof(*dmfid));
+	dmfid->dm_fid_len = sizeof(dm_fid_t) - sizeof(dmfid->dm_fid_len);
+	dmfid->dm_fid_pad = 0;
+	/*
+	 * use memcpy because the inode is a long long and there's no
+	 * assurance that dmfid->dm_fid_ino is properly aligned.
+	 */
+	memcpy(&dmfid->dm_fid_ino, &ip->i_ino, sizeof(dmfid->dm_fid_ino));
+	dmfid->dm_fid_gen = ip->i_d.di_gen;
+
 	memcpy(dmfsid, ip->i_mount->m_fixedfsid, sizeof(*dmfsid));
 	return 0;
 }
fs/xfs/linux-2.6/xfs_export.c +17 -8
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_export.c
===================================================================
@@ -118,20 +118,29 @@ xfs_nfs_get_inode(
 	u64			ino,
 	u32			generation)
  {
-	xfs_fid_t		xfid;
-	bhv_vnode_t		*vp;
+ 	xfs_mount_t		*mp = XFS_M(sb);
+	xfs_inode_t		*ip;
 	int			error;
 
-	xfid.fid_len = sizeof(xfs_fid_t) - sizeof(xfid.fid_len);
-	xfid.fid_pad = 0;
-	xfid.fid_ino = ino;
-	xfid.fid_gen = generation;
+	/*
+	 * NFS can sometimes send requests for ino 0.  Fail them gracefully.
+	 */
+	if (ino == 0)
+		return ERR_PTR(-ESTALE);
 
-	error = xfs_vget(XFS_M(sb), &vp, &xfid);
+	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
 	if (error)
 		return ERR_PTR(-error);
+	if (!ip)
+		return ERR_PTR(-EIO);
 
-	return vp ? vn_to_inode(vp) : NULL;
+	if (!ip->i_d.di_mode || ip->i_d.di_gen != generation) {
+		xfs_iput_new(ip, XFS_ILOCK_SHARED);
+		return ERR_PTR(-ENOENT);
+	}
+
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	return ip->i_vnode;
 }
 
 STATIC struct dentry *
fs/xfs/linux-2.6/xfs_ksyms.c +0 -2
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ksyms.c
===================================================================
@@ -267,7 +267,6 @@ EXPORT_SYMBOL(xfs_read_buf);
 EXPORT_SYMBOL(xfs_rwlock);
 EXPORT_SYMBOL(xfs_rwunlock);
 EXPORT_SYMBOL(xfs_setattr);
-EXPORT_SYMBOL(xfs_fid2);
 EXPORT_SYMBOL(xfs_attr_get);
 EXPORT_SYMBOL(xfs_attr_set);
 EXPORT_SYMBOL(xfs_fsync);
@@ -310,5 +309,4 @@ EXPORT_SYMBOL(xfs_ichgtime_fast);
 EXPORT_SYMBOL(xfs_free_eofblocks);
 
 EXPORT_SYMBOL(xfs_do_force_shutdown);
-EXPORT_SYMBOL(xfs_vget);
 EXPORT_SYMBOL(xfs_root);
fs/xfs/xfs_vfsops.c +0 -53

56 lines not on this page · raw message

fs/xfs/xfs_vfsops.h +0 -1
Index: linux-2.6-xfs/fs/xfs/xfs_vfsops.h
===================================================================
@@ -15,7 +15,6 @@ int xfs_mntupdate(struct xfs_mount *mp, 
 		struct xfs_mount_args *args);
 int xfs_root(struct xfs_mount *mp, bhv_vnode_t **vpp);
 int xfs_sync(struct xfs_mount *mp, int flags);
-int xfs_vget(struct xfs_mount *mp, bhv_vnode_t **vpp, struct xfs_fid *xfid);
 void xfs_do_force_shutdown(struct xfs_mount *mp, int flags, char *fname,
 		int lnnum);
 void xfs_attr_quiesce(struct xfs_mount *mp);
fs/xfs/xfs_vnodeops.c +0 -21
Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
===================================================================
@@ -3457,27 +3457,6 @@ std_return:
 	goto std_return;
 }
 
-
-int
-xfs_fid2(
-	xfs_inode_t	*ip,
-	xfs_fid_t	*xfid)
-{
-	xfs_itrace_entry(ip);
-
-	xfid->fid_len = sizeof(xfs_fid_t) - sizeof(xfid->fid_len);
-	xfid->fid_pad = 0;
-	/*
-	 * use memcpy because the inode is a long long and there's no
-	 * assurance that xfid->fid_ino is properly aligned.
-	 */
-	memcpy(&xfid->fid_ino, &ip->i_ino, sizeof(xfid->fid_ino));
-	xfid->fid_gen = ip->i_d.di_gen;
-
-	return 0;
-}
-
-
 int
 xfs_rwlock(
 	xfs_inode_t	*ip,
fs/xfs/xfs_vnodeops.h +0 -1
Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.h
===================================================================
@@ -39,7 +39,6 @@ int xfs_readdir(struct xfs_inode	*dp, vo
 int xfs_symlink(struct xfs_inode *dp, bhv_vname_t *dentry,
 		char *target_path, mode_t mode, bhv_vnode_t **vpp,
 		struct cred *credp);
-int xfs_fid2(struct xfs_inode *ip, struct xfs_fid *xfid);
 int xfs_rwlock(struct xfs_inode *ip, bhv_vrwlock_t locktype);
 void xfs_rwunlock(struct xfs_inode *ip, bhv_vrwlock_t locktype);
 int xfs_inode_flush(struct xfs_inode *ip, int flags);
#1 lore raw

It is looking good Christoph,

I will check in the patch in the xfs dev tree.

Regards,
Vlad

Quoted 307 lines · Christoph Hellwig

Cleanup various fid related bits:

- merge xfs_fid2 into it's only caller xfs_dm_inode_to_fh.
- remove xfs_vget and opencode it in the two callers, simplifying both of them by avoiding the awkward calling convetion.
- assign directly to the dm_fid_t members in various places in the dmapi code instead of casting them to xfs_fid_t first (which is identical to dm_fid_t)

Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
===================================================================

fs/xfs/dmapi/xfs_dm.c +50 -27
@@ -586,14 +586,12 @@ dm_dip_to_handle(
 	dm_handle_t	*handlep)
 {
 	dm_fid_t	fid;
-	struct xfs_fid	*xfid;
 	int		hsize;
 
-	xfid = (struct xfs_fid *)&fid;
-	xfid->fid_len = sizeof(struct xfs_fid) - sizeof(xfid->fid_len);
-	xfid->fid_pad = 0;
-	xfid->fid_ino = ino;
-        xfid->fid_gen = be32_to_cpu(dip->di_core.di_gen);
+	fid.dm_fid_len = sizeof(struct dm_fid) - sizeof(fid.dm_fid_len);
+	fid.dm_fid_pad = 0;
+	fid.dm_fid_ino = ino;
+	fid.dm_fid_gen = be32_to_cpu(dip->di_core.di_gen);
 
 	memcpy(&handlep->ha_fsid, fsid, sizeof(*fsid));
 	memcpy(&handlep->ha_fid, &fid, fid.dm_fid_len + sizeof(fid.dm_fid_len));
@@ -3273,27 +3271,49 @@ xfs_dm_get_invis_ops(
 STATIC int
 xfs_dm_fh_to_inode(
 	struct super_block	*sb,
-	struct inode		**ip,
+	struct inode		**inode,
 	dm_fid_t		*dmfid)
 {
-	bhv_vnode_t	*vp = NULL;
-	xfs_mount_t	*mp = XFS_M(sb);
-	int		error;
-	struct xfs_fid	xfid;
+	xfs_mount_t		*mp = XFS_M(sb);
+	xfs_inode_t		*ip;
+	xfs_ino_t		ino;
+	unsigned int		igen;
+	int			error;
 
-	/* Returns negative errors to DMAPI */
+	*inode = NULL;
 
-	*ip = NULL;
-	memcpy(&xfid, dmfid, sizeof(*dmfid));
-	if (xfid.fid_len) {	/* file object handle */
-		error = xfs_vget(mp, &vp, &xfid);
+	if (!dmfid->dm_fid_len) {
+		/* filesystem handle */
+		*inode = igrab(mp->m_rootip->i_vnode);
+		if (!*inode)
+			return -ENOENT;
+		return 0;
 	}
-	else {			/* filesystem handle */
-		error = xfs_root(mp, &vp);
+
+	if (dmfid->dm_fid_len != sizeof(*dmfid) - sizeof(dmfid->dm_fid_len))
+		return -EINVAL;
+
+	ino  = dmfid->dm_fid_ino;
+	igen = dmfid->dm_fid_gen;
+
+	/* fail requests for ino 0 gracefully. */
+	if (ino == 0)
+		return -ESTALE;
+
+	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
+	if (error)
+		return -error;
+	if (!ip)
+		return -EIO;
+
+	if (!ip->i_d.di_mode || ip->i_d.di_gen != igen) {
+		xfs_iput_new(ip, XFS_ILOCK_SHARED);
+		return -ENOENT;
 	}
-	if (vp && (error == 0))
-		*ip = vn_to_inode(vp);
-	return -error; /* Return negative error to DMAPI */
+
+	*inode = ip->i_vnode;
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	return 0;
 }
 
 STATIC int
@@ -3303,18 +3323,21 @@ xfs_dm_inode_to_fh(
 	dm_fsid_t		*dmfsid)
 {
 	xfs_inode_t		*ip = XFS_I(inode);
-	int			error;
-	struct xfs_fid		xfid;
 
 	/* Returns negative errors to DMAPI */
 
 	if (ip->i_mount->m_fixedfsid == NULL)
 		return -EINVAL;
-	error = xfs_fid2(ip, &xfid);
-	if (error)
-		return -error; /* Return negative error to DMAPI */
 
-	memcpy(dmfid, &xfid, sizeof(*dmfid));
+	dmfid->dm_fid_len = sizeof(dm_fid_t) - sizeof(dmfid->dm_fid_len);
+	dmfid->dm_fid_pad = 0;
+	/*
+	 * use memcpy because the inode is a long long and there's no
+	 * assurance that dmfid->dm_fid_ino is properly aligned.
+	 */
+	memcpy(&dmfid->dm_fid_ino, &ip->i_ino, sizeof(dmfid->dm_fid_ino));
+	dmfid->dm_fid_gen = ip->i_d.di_gen;
+
 	memcpy(dmfsid, ip->i_mount->m_fixedfsid, sizeof(*dmfsid));
 	return 0;
 }

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

fs/xfs/linux-2.6/xfs_export.c +17 -8
@@ -118,20 +118,29 @@ xfs_nfs_get_inode(
 	u64			ino,
 	u32			generation)
  {
-	xfs_fid_t		xfid;
-	bhv_vnode_t		*vp;
+ 	xfs_mount_t		*mp = XFS_M(sb);
+	xfs_inode_t		*ip;
 	int			error;
 
-	xfid.fid_len = sizeof(xfs_fid_t) - sizeof(xfid.fid_len);
-	xfid.fid_pad = 0;
-	xfid.fid_ino = ino;
-	xfid.fid_gen = generation;
+	/*
+	 * NFS can sometimes send requests for ino 0.  Fail them gracefully.
+	 */
+	if (ino == 0)
+		return ERR_PTR(-ESTALE);
 
-	error = xfs_vget(XFS_M(sb), &vp, &xfid);
+	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
 	if (error)
 		return ERR_PTR(-error);
+	if (!ip)
+		return ERR_PTR(-EIO);
 
-	return vp ? vn_to_inode(vp) : NULL;
+	if (!ip->i_d.di_mode || ip->i_d.di_gen != generation) {
+		xfs_iput_new(ip, XFS_ILOCK_SHARED);
+		return ERR_PTR(-ENOENT);
+	}
+
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	return ip->i_vnode;
 }
 
 STATIC struct dentry *

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

fs/xfs/linux-2.6/xfs_ksyms.c +0 -2
@@ -267,7 +267,6 @@ EXPORT_SYMBOL(xfs_read_buf);
 EXPORT_SYMBOL(xfs_rwlock);
 EXPORT_SYMBOL(xfs_rwunlock);
 EXPORT_SYMBOL(xfs_setattr);
-EXPORT_SYMBOL(xfs_fid2);
 EXPORT_SYMBOL(xfs_attr_get);
 EXPORT_SYMBOL(xfs_attr_set);
 EXPORT_SYMBOL(xfs_fsync);
@@ -310,5 +309,4 @@ EXPORT_SYMBOL(xfs_ichgtime_fast);
 EXPORT_SYMBOL(xfs_free_eofblocks);
 
 EXPORT_SYMBOL(xfs_do_force_shutdown);
-EXPORT_SYMBOL(xfs_vget);
 EXPORT_SYMBOL(xfs_root);

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

fs/xfs/xfs_vfsops.c +0 -53
@@ -1408,56 +1408,3 @@ xfs_syncsub(
 
 	return XFS_ERROR(last_error);
 }
-
-/*
- * xfs_vget - called by DMAPI and NFSD to get vnode from file handle
- */
-int
-xfs_vget(
-	xfs_mount_t	*mp,
-	bhv_vnode_t	**vpp,
-	xfs_fid_t	*xfid)
-{
-	xfs_inode_t	*ip;
-	int		error;
-	xfs_ino_t	ino;
-	unsigned int	igen;
-
-	/*
-	 * Invalid.  Since handles can be created in user space and passed in
-	 * via gethandle(), this is not cause for a panic.
-	 */
-	if (xfid->fid_len != sizeof(*xfid) - sizeof(xfid->fid_len))
-		return XFS_ERROR(EINVAL);
-
-	ino  = xfid->fid_ino;
-	igen = xfid->fid_gen;
-
-	/*
-	 * NFS can sometimes send requests for ino 0.  Fail them gracefully.
-	 */
-	if (ino == 0)
-		return XFS_ERROR(ESTALE);
-
-	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
-	if (error) {
-		*vpp = NULL;
-		return error;
-	}
-
-	if (ip == NULL) {
-		*vpp = NULL;
-		return XFS_ERROR(EIO);
-	}
-
-	if (ip->i_d.di_mode == 0 || ip->i_d.di_gen != igen) {
-		xfs_iput_new(ip, XFS_ILOCK_SHARED);
-		*vpp = NULL;
-		return XFS_ERROR(ENOENT);
-	}
-
-	*vpp = XFS_ITOV(ip);
-	xfs_iunlock(ip, XFS_ILOCK_SHARED);
-	return 0;
-}
-

Index: linux-2.6-xfs/fs/xfs/xfs_vfsops.h
===================================================================

fs/xfs/xfs_vfsops.h +0 -1
@@ -15,7 +15,6 @@ int xfs_mntupdate(struct xfs_mount *mp, 
 		struct xfs_mount_args *args);
 int xfs_root(struct xfs_mount *mp, bhv_vnode_t **vpp);
 int xfs_sync(struct xfs_mount *mp, int flags);
-int xfs_vget(struct xfs_mount *mp, bhv_vnode_t **vpp, struct xfs_fid *xfid);
 void xfs_do_force_shutdown(struct xfs_mount *mp, int flags, char *fname,
 		int lnnum);
 void xfs_attr_quiesce(struct xfs_mount *mp);

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

fs/xfs/xfs_vnodeops.c +0 -21
@@ -3457,27 +3457,6 @@ std_return:
 	goto std_return;
 }
 
-
-int
-xfs_fid2(
-	xfs_inode_t	*ip,
-	xfs_fid_t	*xfid)
-{
-	xfs_itrace_entry(ip);
-
-	xfid->fid_len = sizeof(xfs_fid_t) - sizeof(xfid->fid_len);
-	xfid->fid_pad = 0;
-	/*
-	 * use memcpy because the inode is a long long and there's no
-	 * assurance that xfid->fid_ino is properly aligned.
-	 */
-	memcpy(&xfid->fid_ino, &ip->i_ino, sizeof(xfid->fid_ino));
-	xfid->fid_gen = ip->i_d.di_gen;
-
-	return 0;
-}
-
-
 int
 xfs_rwlock(
 	xfs_inode_t	*ip,

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

fs/xfs/xfs_vnodeops.h +0 -1
@@ -39,7 +39,6 @@ int xfs_readdir(struct xfs_inode	*dp, vo
 int xfs_symlink(struct xfs_inode *dp, bhv_vname_t *dentry,
 		char *target_path, mode_t mode, bhv_vnode_t **vpp,
 		struct cred *credp);
-int xfs_fid2(struct xfs_inode *ip, struct xfs_fid *xfid);
 int xfs_rwlock(struct xfs_inode *ip, bhv_vrwlock_t locktype);
 void xfs_rwunlock(struct xfs_inode *ip, bhv_vrwlock_t locktype);
 int xfs_inode_flush(struct xfs_inode *ip, int flags);
#2 lore raw
Quoted 3 lines · Vlad Apostolov

It is looking good Christoph,

I will check in the patch in the xfs dev tree.

Btw, I typoed the subject and it should be "cleanup fid handling" of course. If you don't mind please check it in with the correct subject.