Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec3ba85f authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Alex Elder
Browse files

xfs: more sensible inode refcounting for ialloc 



Currently we return iodes from xfs_ialloc with just a single reference held.
But we need two references, as one is dropped during transaction commit and
the second needs to be transfered to the VFS.  Change xfs_ialloc to use
xfs_iget plus xfs_trans_ijoin_ref to grab two references to the inode,
and remove the now superflous IHOLD calls from all callers.  This also
greatly simplifies the error handling in xfs_create and also allow to remove
xfs_trans_iget as no other callers are left.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Signed-off-by: default avatarAlex Elder <aelder@sgi.com>
parent 1050c71e

fs/xfs/quota/xfs_qm.c

+0 −7
Original line number Original line Diff line number Diff line
@@ -1229,13 +1229,6 @@ xfs_qm_qino_alloc( 
		return error;

		return error;

	}

	}





	/*

	 * Keep an extra reference to this quota inode. This inode is

	 * locked exclusively and joined to the transaction already.

	 */

	ASSERT(xfs_isilocked(*ip, XFS_ILOCK_EXCL));

	IHOLD(*ip);



	/*

	/*

	 * Make the changes in the superblock, and log those too.

	 * Make the changes in the superblock, and log those too.

	 * sbfields arg may contain fields other than *QUOTINO;

	 * sbfields arg may contain fields other than *QUOTINO;

fs/xfs/xfs_inode.c

+3 −2
Original line number Original line Diff line number Diff line
@@ -1016,8 +1016,8 @@ xfs_ialloc( 
	 * This is because we're setting fields here we need

	 * This is because we're setting fields here we need

	 * to prevent others from looking at until we're done.

	 * to prevent others from looking at until we're done.

	 */

	 */

	error = xfs_trans_iget(tp->t_mountp, tp, ino,

	error = xfs_iget(tp->t_mountp, tp, ino, XFS_IGET_CREATE,

				XFS_IGET_CREATE, XFS_ILOCK_EXCL, &ip);

			 XFS_ILOCK_EXCL, &ip);

	if (error)

	if (error)

		return error;

		return error;

	ASSERT(ip != NULL);

	ASSERT(ip != NULL);
@@ -1166,6 +1166,7 @@ xfs_ialloc( 
	/*

	/*

	 * Log the new values stuffed into the inode.

	 * Log the new values stuffed into the inode.

	 */

	 */

	xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);

	xfs_trans_log_inode(tp, ip, flags);

	xfs_trans_log_inode(tp, ip, flags);





	/* now that we have an i_mode we can setup inode ops and unlock */

	/* now that we have an i_mode we can setup inode ops and unlock */

fs/xfs/xfs_trans.h

+0 −2
Original line number Original line Diff line number Diff line
@@ -469,8 +469,6 @@ void xfs_trans_inode_buf(xfs_trans_t *, struct xfs_buf *); 
void		xfs_trans_stale_inode_buf(xfs_trans_t *, struct xfs_buf *);

void		xfs_trans_stale_inode_buf(xfs_trans_t *, struct xfs_buf *);

void		xfs_trans_dquot_buf(xfs_trans_t *, struct xfs_buf *, uint);

void		xfs_trans_dquot_buf(xfs_trans_t *, struct xfs_buf *, uint);

void		xfs_trans_inode_alloc_buf(xfs_trans_t *, struct xfs_buf *);

void		xfs_trans_inode_alloc_buf(xfs_trans_t *, struct xfs_buf *);

int		xfs_trans_iget(struct xfs_mount *, xfs_trans_t *,

			       xfs_ino_t , uint, uint, struct xfs_inode **);

void		xfs_trans_ichgtime(struct xfs_trans *, struct xfs_inode *, int);

void		xfs_trans_ichgtime(struct xfs_trans *, struct xfs_inode *, int);

void		xfs_trans_ijoin_ref(struct xfs_trans *, struct xfs_inode *, uint);

void		xfs_trans_ijoin_ref(struct xfs_trans *, struct xfs_inode *, uint);

void		xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *);

void		xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *);

fs/xfs/xfs_trans_inode.c

+0 −22
Original line number Original line Diff line number Diff line
@@ -43,28 +43,6 @@ xfs_trans_inode_broot_debug( 
#define	xfs_trans_inode_broot_debug(ip)

#define	xfs_trans_inode_broot_debug(ip)

#endif

#endif





/*

 * Get an inode and join it to the transaction.

 */

int

xfs_trans_iget(

	xfs_mount_t	*mp,

	xfs_trans_t	*tp,

	xfs_ino_t	ino,

	uint		flags,

	uint		lock_flags,

	xfs_inode_t	**ipp)

{

	int			error;



	error = xfs_iget(mp, tp, ino, flags, lock_flags, ipp);

	if (!error && tp) {

		xfs_trans_ijoin(tp, *ipp);

		(*ipp)->i_itemp->ili_lock_flags = lock_flags;

	}

	return error;

}



/*

/*

 * Add a locked inode to the transaction.

 * Add a locked inode to the transaction.

 *

 *

fs/xfs/xfs_vnodeops.c

+16 −45
Original line number Original line Diff line number Diff line
@@ -1310,7 +1310,7 @@ xfs_create( 
	error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,

	error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,

			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);

			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);

	if (error)

	if (error)

		goto std_return;

		return error;





	if (is_dir) {

	if (is_dir) {

		rdev = 0;

		rdev = 0;
@@ -1389,12 +1389,6 @@ xfs_create( 
		goto out_trans_abort;

		goto out_trans_abort;

	}

	}





	/*

	 * At this point, we've gotten a newly allocated inode.

	 * It is locked (and joined to the transaction).

	 */

	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));



	/*

	/*

	 * Now we join the directory inode to the transaction.  We do not do it

	 * Now we join the directory inode to the transaction.  We do not do it

	 * earlier because xfs_dir_ialloc might commit the previous transaction

	 * earlier because xfs_dir_ialloc might commit the previous transaction
@@ -1440,22 +1434,13 @@ xfs_create( 
	 */

	 */

	xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp);

	xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp);





	/*

	 * xfs_trans_commit normally decrements the vnode ref count

	 * when it unlocks the inode. Since we want to return the

	 * vnode to the caller, we bump the vnode ref count now.

	 */

	IHOLD(ip);



	error = xfs_bmap_finish(&tp, &free_list, &committed);

	error = xfs_bmap_finish(&tp, &free_list, &committed);

	if (error)

	if (error)

		goto out_abort_rele;

		goto out_bmap_cancel;





	error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);

	error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);

	if (error) {

	if (error)

		IRELE(ip);

		goto out_release_inode;

		goto out_dqrele;

	}





	xfs_qm_dqrele(udqp);

	xfs_qm_dqrele(udqp);

	xfs_qm_dqrele(gdqp);

	xfs_qm_dqrele(gdqp);
@@ -1469,27 +1454,21 @@ xfs_create( 
	cancel_flags |= XFS_TRANS_ABORT;

	cancel_flags |= XFS_TRANS_ABORT;

 out_trans_cancel:

 out_trans_cancel:

	xfs_trans_cancel(tp, cancel_flags);

	xfs_trans_cancel(tp, cancel_flags);

 out_dqrele:

 out_release_inode:

	xfs_qm_dqrele(udqp);

	xfs_qm_dqrele(gdqp);



	if (unlock_dp_on_error)

		xfs_iunlock(dp, XFS_ILOCK_EXCL);

 std_return:

	return error;



 out_abort_rele:

	/*

	/*

	 * Wait until after the current transaction is aborted to

	 * Wait until after the current transaction is aborted to

	 * release the inode.  This prevents recursive transactions

	 * release the inode.  This prevents recursive transactions

	 * and deadlocks from xfs_inactive.

	 * and deadlocks from xfs_inactive.

	 */

	 */

	xfs_bmap_cancel(&free_list);

	if (ip)

	cancel_flags |= XFS_TRANS_ABORT;

	xfs_trans_cancel(tp, cancel_flags);

		IRELE(ip);

		IRELE(ip);

	unlock_dp_on_error = B_FALSE;



	goto out_dqrele;

	xfs_qm_dqrele(udqp);

	xfs_qm_dqrele(gdqp);



	if (unlock_dp_on_error)

		xfs_iunlock(dp, XFS_ILOCK_EXCL);

	return error;

}

}





#ifdef DEBUG

#ifdef DEBUG
@@ -2114,9 +2093,8 @@ xfs_symlink( 
				  XFS_BMAPI_WRITE | XFS_BMAPI_METADATA,

				  XFS_BMAPI_WRITE | XFS_BMAPI_METADATA,

				  &first_block, resblks, mval, &nmaps,

				  &first_block, resblks, mval, &nmaps,

				  &free_list);

				  &free_list);

		if (error) {

		if (error)

			goto error1;

			goto error2;

		}





		if (resblks)

		if (resblks)

			resblks -= fs_blocks;

			resblks -= fs_blocks;
@@ -2148,7 +2126,7 @@ xfs_symlink( 
	error = xfs_dir_createname(tp, dp, link_name, ip->i_ino,

	error = xfs_dir_createname(tp, dp, link_name, ip->i_ino,

					&first_block, &free_list, resblks);

					&first_block, &free_list, resblks);

	if (error)

	if (error)

		goto error1;

		goto error2;

	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);

	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);

	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);

	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
@@ -2161,13 +2139,6 @@ xfs_symlink( 
		xfs_trans_set_sync(tp);

		xfs_trans_set_sync(tp);

	}

	}





	/*

	 * xfs_trans_commit normally decrements the vnode ref count

	 * when it unlocks the inode. Since we want to return the

	 * vnode to the caller, we bump the vnode ref count now.

	 */

	IHOLD(ip);



	error = xfs_bmap_finish(&tp, &free_list, &committed);

	error = xfs_bmap_finish(&tp, &free_list, &committed);

	if (error) {

	if (error) {

		goto error2;

		goto error2;