Skip to content

Commit

Permalink
xfs: remove xfs_trans_unlocked_item
Browse files Browse the repository at this point in the history
There is no reason to wake up log space waiters when unlocking inodes or
dquots, and the commit log has no explanation for this function either.

Given that we now have exact log space wakeups everywhere we can assume
the reason for this function was to paper over log space races in earlier
XFS versions.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Ben Myers <bpm@sgi.com>
  • Loading branch information
Christoph Hellwig authored and Ben Myers committed Feb 23, 2012
1 parent 3af1de7 commit 5b03ff1
Show file tree
Hide file tree
Showing 8 changed files with 5 additions and 104 deletions.
11 changes: 0 additions & 11 deletions fs/xfs/xfs_dquot.c
Original file line number Diff line number Diff line change
Expand Up @@ -1076,17 +1076,6 @@ xfs_qm_dqflush(

}

void
xfs_dqunlock(
xfs_dquot_t *dqp)
{
xfs_dqunlock_nonotify(dqp);
if (dqp->q_logitem.qli_dquot == dqp) {
xfs_trans_unlocked_item(dqp->q_logitem.qli_item.li_ailp,
&dqp->q_logitem.qli_item);
}
}

/*
* Lock two xfs_dquot structures.
*
Expand Down
3 changes: 1 addition & 2 deletions fs/xfs/xfs_dquot.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ static inline void xfs_dqlock(struct xfs_dquot *dqp)
mutex_lock(&dqp->q_qlock);
}

static inline void xfs_dqunlock_nonotify(struct xfs_dquot *dqp)
static inline void xfs_dqunlock(struct xfs_dquot *dqp)
{
mutex_unlock(&dqp->q_qlock);
}
Expand Down Expand Up @@ -166,7 +166,6 @@ extern int xfs_qm_dqget(xfs_mount_t *, xfs_inode_t *,
extern void xfs_qm_dqput(xfs_dquot_t *);

extern void xfs_dqlock2(struct xfs_dquot *, struct xfs_dquot *);
extern void xfs_dqunlock(struct xfs_dquot *);
extern void xfs_dqflock_pushbuf_wait(struct xfs_dquot *dqp);

static inline struct xfs_dquot *xfs_qm_dqhold(struct xfs_dquot *dqp)
Expand Down
13 changes: 1 addition & 12 deletions fs/xfs/xfs_iget.c
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,7 @@ xfs_iunlock(
(XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
(XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_IUNLOCK_NONOTIFY |
XFS_LOCK_DEP_MASK)) == 0);
ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_LOCK_DEP_MASK)) == 0);
ASSERT(lock_flags != 0);

if (lock_flags & XFS_IOLOCK_EXCL)
Expand All @@ -656,16 +655,6 @@ xfs_iunlock(
else if (lock_flags & XFS_ILOCK_SHARED)
mrunlock_shared(&ip->i_lock);

if ((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) &&
!(lock_flags & XFS_IUNLOCK_NONOTIFY) && ip->i_itemp) {
/*
* Let the AIL know that this item has been unlocked in case
* it is in the AIL and anyone is waiting on it. Don't do
* this if the caller has asked us not to.
*/
xfs_trans_unlocked_item(ip->i_itemp->ili_item.li_ailp,
(xfs_log_item_t*)(ip->i_itemp));
}
trace_xfs_iunlock(ip, lock_flags, _RET_IP_);
}

Expand Down
4 changes: 1 addition & 3 deletions fs/xfs/xfs_inode.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,6 @@ static inline int xfs_isiflocked(struct xfs_inode *ip)
#define XFS_IOLOCK_SHARED (1<<1)
#define XFS_ILOCK_EXCL (1<<2)
#define XFS_ILOCK_SHARED (1<<3)
#define XFS_IUNLOCK_NONOTIFY (1<<4)

#define XFS_LOCK_MASK (XFS_IOLOCK_EXCL | XFS_IOLOCK_SHARED \
| XFS_ILOCK_EXCL | XFS_ILOCK_SHARED)
Expand All @@ -431,8 +430,7 @@ static inline int xfs_isiflocked(struct xfs_inode *ip)
{ XFS_IOLOCK_EXCL, "IOLOCK_EXCL" }, \
{ XFS_IOLOCK_SHARED, "IOLOCK_SHARED" }, \
{ XFS_ILOCK_EXCL, "ILOCK_EXCL" }, \
{ XFS_ILOCK_SHARED, "ILOCK_SHARED" }, \
{ XFS_IUNLOCK_NONOTIFY, "IUNLOCK_NONOTIFY" }
{ XFS_ILOCK_SHARED, "ILOCK_SHARED" }


/*
Expand Down
6 changes: 1 addition & 5 deletions fs/xfs/xfs_inode_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,7 @@ xfs_inode_item_trylock(
/* Stale items should force out the iclog */
if (ip->i_flags & XFS_ISTALE) {
xfs_ifunlock(ip);
/*
* we hold the AIL lock - notify the unlock routine of this
* so it doesn't try to get the lock again.
*/
xfs_iunlock(ip, XFS_ILOCK_SHARED|XFS_IUNLOCK_NONOTIFY);
xfs_iunlock(ip, XFS_ILOCK_SHARED);
return XFS_ITEM_PINNED;
}

Expand Down
44 changes: 0 additions & 44 deletions fs/xfs/xfs_trans_ail.c
Original file line number Diff line number Diff line change
Expand Up @@ -610,50 +610,6 @@ xfs_ail_push_all(
xfs_ail_push(ailp, threshold_lsn);
}

/*
* This is to be called when an item is unlocked that may have
* been in the AIL. It will wake up the first member of the AIL
* wait list if this item's unlocking might allow it to progress.
* If the item is in the AIL, then we need to get the AIL lock
* while doing our checking so we don't race with someone going
* to sleep waiting for this event in xfs_trans_push_ail().
*/
void
xfs_trans_unlocked_item(
struct xfs_ail *ailp,
xfs_log_item_t *lip)
{
xfs_log_item_t *min_lip;

/*
* If we're forcibly shutting down, we may have
* unlocked log items arbitrarily. The last thing
* we want to do is to move the tail of the log
* over some potentially valid data.
*/
if (!(lip->li_flags & XFS_LI_IN_AIL) ||
XFS_FORCED_SHUTDOWN(ailp->xa_mount)) {
return;
}

/*
* This is the one case where we can call into xfs_ail_min()
* without holding the AIL lock because we only care about the
* case where we are at the tail of the AIL. If the object isn't
* at the tail, it doesn't matter what result we get back. This
* is slightly racy because since we were just unlocked, we could
* go to sleep between the call to xfs_ail_min and the call to
* xfs_log_space_wake, have someone else lock us, commit to us disk,
* move us out of the tail of the AIL, and then we wake up. However,
* the call to xfs_log_space_wake() doesn't do anything if there's
* not enough free space to wake people up so we're safe calling it.
*/
min_lip = xfs_ail_min(ailp);

if (min_lip == lip)
xfs_log_space_wake(ailp->xa_mount, true);
} /* xfs_trans_unlocked_item */

/*
* xfs_trans_ail_update - bulk AIL insertion operation.
*
Expand Down
25 changes: 1 addition & 24 deletions fs/xfs/xfs_trans_buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,19 +463,7 @@ xfs_trans_brelse(xfs_trans_t *tp,
* Default to a normal brelse() call if the tp is NULL.
*/
if (tp == NULL) {
struct xfs_log_item *lip = bp->b_fspriv;

ASSERT(bp->b_transp == NULL);

/*
* If there's a buf log item attached to the buffer,
* then let the AIL know that the buffer is being
* unlocked.
*/
if (lip != NULL && lip->li_type == XFS_LI_BUF) {
bip = bp->b_fspriv;
xfs_trans_unlocked_item(bip->bli_item.li_ailp, lip);
}
xfs_buf_relse(bp);
return;
}
Expand Down Expand Up @@ -550,21 +538,10 @@ xfs_trans_brelse(xfs_trans_t *tp,
ASSERT(!(bip->bli_item.li_flags & XFS_LI_IN_AIL));
ASSERT(!(bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF));
xfs_buf_item_relse(bp);
bip = NULL;
}
bp->b_transp = NULL;

/*
* If we've still got a buf log item on the buffer, then
* tell the AIL that the buffer is being unlocked.
*/
if (bip != NULL) {
xfs_trans_unlocked_item(bip->bli_item.li_ailp,
(xfs_log_item_t*)bip);
}

bp->b_transp = NULL;
xfs_buf_relse(bp);
return;
}

/*
Expand Down
3 changes: 0 additions & 3 deletions fs/xfs/xfs_trans_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ void xfs_ail_push(struct xfs_ail *, xfs_lsn_t);
void xfs_ail_push_all(struct xfs_ail *);
xfs_lsn_t xfs_ail_min_lsn(struct xfs_ail *ailp);

void xfs_trans_unlocked_item(struct xfs_ail *,
xfs_log_item_t *);

struct xfs_log_item * xfs_trans_ail_cursor_first(struct xfs_ail *ailp,
struct xfs_ail_cursor *cur,
xfs_lsn_t lsn);
Expand Down

0 comments on commit 5b03ff1

Please sign in to comment.