Skip to content

Commit

Permalink
xfs: simplify xfs_trans_ijoin* again
Browse files Browse the repository at this point in the history 
There is no reason to keep a reference to the inode even if we unlock
it during transaction commit because we never drop a reference between
the ijoin and commit.  Also use this fact to merge xfs_trans_ijoin_ref
back into xfs_trans_ijoin - the third argument decides if an unlock
is needed now.

I'm actually starting to wonder if allowing inodes to be unlocked
at transaction commit really is worth the effort.  The only real
benefit is that they can be unlocked earlier when commiting a
synchronous transactions, but that could be solved by doing the
log force manually after the unlock, too.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
  • Loading branch information
Christoph Hellwig authored and Alex Elder committed Oct 12, 2011
1 parent 23bb0be commit ddc3415
Show file tree
Hide file tree
Showing 18 changed files with 65 additions and 83 deletions.
28 changes: 14 additions & 14 deletions fs/xfs/xfs_attr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ xfs_attr_set_int(
return (error);
}

xfs_trans_ijoin(args.trans, dp);
xfs_trans_ijoin(args.trans, dp, 0);

/*
* If the attribute list is non-existent or a shortform list,
Expand Down Expand Up @@ -389,7 +389,7 @@ xfs_attr_set_int(
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args.trans, dp);
xfs_trans_ijoin(args.trans, dp, 0);

/*
* Commit the leaf transformation. We'll need another (linked)
Expand Down Expand Up @@ -537,7 +537,7 @@ xfs_attr_remove_int(xfs_inode_t *dp, struct xfs_name *name, int flags)
* No need to make quota reservations here. We expect to release some
* blocks not allocate in the common case.
*/
xfs_trans_ijoin(args.trans, dp);
xfs_trans_ijoin(args.trans, dp, 0);

/*
* Decide on what work routines to call based on the inode size.
Expand Down Expand Up @@ -809,7 +809,7 @@ xfs_attr_inactive(xfs_inode_t *dp)
* No need to make quota reservations here. We expect to release some
* blocks, not allocate, in the common case.
*/
xfs_trans_ijoin(trans, dp);
xfs_trans_ijoin(trans, dp, 0);

/*
* Decide on what work routines to call based on the inode size.
Expand Down Expand Up @@ -961,7 +961,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);

/*
* Commit the current trans (including the inode) and start
Expand Down Expand Up @@ -1063,7 +1063,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
* in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);
} else
xfs_da_buf_done(bp);

Expand Down Expand Up @@ -1137,7 +1137,7 @@ xfs_attr_leaf_removename(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);
} else
xfs_da_buf_done(bp);
return(0);
Expand Down Expand Up @@ -1291,7 +1291,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
* in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);

/*
* Commit the node conversion and start the next
Expand Down Expand Up @@ -1328,7 +1328,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);
} else {
/*
* Addition succeeded, update Btree hashvals.
Expand Down Expand Up @@ -1440,7 +1440,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
* in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);
}

/*
Expand Down Expand Up @@ -1572,7 +1572,7 @@ xfs_attr_node_removename(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);

/*
* Commit the Btree join operation and start a new trans.
Expand Down Expand Up @@ -1623,7 +1623,7 @@ xfs_attr_node_removename(xfs_da_args_t *args)
* in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);
} else
xfs_da_brelse(args->trans, bp);
}
Expand Down Expand Up @@ -2060,7 +2060,7 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, dp);
xfs_trans_ijoin(args->trans, dp, 0);

ASSERT(nmap == 1);
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
Expand Down Expand Up @@ -2207,7 +2207,7 @@ xfs_attr_rmtval_remove(xfs_da_args_t *args)
* a new one. We need the inode to be in all transactions.
*/
if (committed)
xfs_trans_ijoin(args->trans, args->dp);
xfs_trans_ijoin(args->trans, args->dp, 0);

/*
* Close out trans and start the next one in the chain.
Expand Down
4 changes: 2 additions & 2 deletions fs/xfs/xfs_bmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2195,7 +2195,7 @@ xfs_bmap_rtalloc(
* Lock out other modifications to the RT bitmap inode.
*/
xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL);

/*
* If it's an allocation to an empty file at offset 0,
Expand Down Expand Up @@ -3460,7 +3460,7 @@ xfs_bmap_add_attrfork(
}
ASSERT(ip->i_d.di_anextents == 0);

xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);

switch (ip->i_d.di_format) {
Expand Down
4 changes: 2 additions & 2 deletions fs/xfs/xfs_dfrag.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,8 @@ xfs_swap_extents(
}


xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
xfs_trans_ijoin_ref(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
xfs_trans_ijoin(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);

xfs_trans_log_inode(tp, ip, ilf_fields);
xfs_trans_log_inode(tp, tip, tilf_fields);
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_dquot.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ xfs_qm_dqalloc(
return (ESRCH);
}

xfs_trans_ijoin_ref(tp, quotip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
nmaps = 1;
error = xfs_bmapi_write(tp, quotip, offset_fsb,
XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ xfs_file_fsync(
* transaction. So we play it safe and fire off the
* transaction anyway.
*/
xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
error = xfs_trans_commit(tp, 0);

Expand Down
6 changes: 3 additions & 3 deletions fs/xfs/xfs_inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,7 @@ xfs_ialloc(
/*
* Log the new values stuffed into the inode.
*/
xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_log_inode(tp, ip, flags);

/* now that we have an i_mode we can setup inode ops and unlock */
Expand Down Expand Up @@ -1297,7 +1297,7 @@ xfs_itruncate_extents(
*/
error = xfs_bmap_finish(&tp, &free_list, &committed);
if (committed)
xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);
if (error)
goto out_bmap_cancel;

Expand All @@ -1313,7 +1313,7 @@ xfs_itruncate_extents(
error = xfs_trans_commit(tp, 0);
tp = ntp;

xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

if (error)
goto out;
Expand Down
4 changes: 1 addition & 3 deletions fs/xfs/xfs_inode_item.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,8 @@ xfs_inode_item_unlock(

lock_flags = iip->ili_lock_flags;
iip->ili_lock_flags = 0;
if (lock_flags) {
if (lock_flags)
xfs_iunlock(ip, lock_flags);
IRELE(ip);
}
}

/*
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ xfs_ioctl_setattr(
}
}

xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

/*
* Change file ownership. Must be the owner or privileged.
Expand Down
6 changes: 3 additions & 3 deletions fs/xfs/xfs_iomap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ xfs_iomap_write_direct(
if (error)
goto error1;

xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

bmapi_flag = 0;
if (offset < ip->i_size || extsz)
Expand Down Expand Up @@ -528,7 +528,7 @@ xfs_iomap_write_allocate(
return XFS_ERROR(error);
}
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

xfs_bmap_init(&free_list, &first_block);

Expand Down Expand Up @@ -692,7 +692,7 @@ xfs_iomap_write_unwritten(
}

xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

/*
* Modify the unwritten extent state of the buffer.
Expand Down
4 changes: 2 additions & 2 deletions fs/xfs/xfs_iops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ xfs_setattr_nonsize(
}
}

xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

/*
* Change file ownership. Must be the owner or privileged.
Expand Down Expand Up @@ -863,7 +863,7 @@ xfs_setattr_size(

xfs_ilock(ip, XFS_ILOCK_EXCL);

xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

/*
* Only change the c/mtime if we are changing the size or we are
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_qm_syscalls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ xfs_qm_scall_trunc_qfile(
}

xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip);
xfs_trans_ijoin(tp, ip, 0);

error = xfs_itruncate_data(&tp, ip, 0);
if (error) {
Expand Down
8 changes: 4 additions & 4 deletions fs/xfs/xfs_rename.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ xfs_rename(
* we can rely on either trans_commit or trans_cancel to unlock
* them.
*/
xfs_trans_ijoin_ref(tp, src_dp, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, src_dp, XFS_ILOCK_EXCL);
if (new_parent)
xfs_trans_ijoin_ref(tp, target_dp, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, src_ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, target_dp, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, src_ip, XFS_ILOCK_EXCL);
if (target_ip)
xfs_trans_ijoin_ref(tp, target_ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, target_ip, XFS_ILOCK_EXCL);

/*
* If we are using project inheritance, we only allow renames
Expand Down
10 changes: 5 additions & 5 deletions fs/xfs/xfs_rtalloc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ xfs_growfs_rt_alloc(
* Lock the inode.
*/
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);

xfs_bmap_init(&flist, &firstblock);
/*
Expand Down Expand Up @@ -155,7 +155,7 @@ xfs_growfs_rt_alloc(
* Lock the bitmap inode.
*/
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
/*
* Get a buffer for the block.
*/
Expand Down Expand Up @@ -1960,7 +1960,7 @@ xfs_growfs_rt(
* Lock out other callers by grabbing the bitmap inode lock.
*/
xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
/*
* Update the bitmap inode's size.
*/
Expand All @@ -1972,7 +1972,7 @@ xfs_growfs_rt(
* Get the summary inode into the transaction.
*/
xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
/*
* Update the summary inode's size.
*/
Expand Down Expand Up @@ -2143,7 +2143,7 @@ xfs_rtfree_extent(
* Synchronize by locking the bitmap inode.
*/
xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);

#if defined(__KERNEL__) && defined(DEBUG)
/*
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_super.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ xfs_log_inode(
}

xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
return xfs_trans_commit(tp, 0);
}
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_trans.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,6 @@ xfs_trans_roll(
if (error)
return error;

xfs_trans_ijoin(trans, dp);
xfs_trans_ijoin(trans, dp, 0);
return 0;
}
3 changes: 1 addition & 2 deletions fs/xfs/xfs_trans.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,7 @@ 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_inode_alloc_buf(xfs_trans_t *, struct xfs_buf *);
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(struct xfs_trans *, struct xfs_inode *);
void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
void xfs_trans_log_buf(xfs_trans_t *, struct xfs_buf *, uint, uint);
void xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
struct xfs_efi_log_item *xfs_trans_get_efi(xfs_trans_t *, uint);
Expand Down
Loading

0 comments on commit ddc3415

Please sign in to comment.