Commit 507630b2 authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers

xfs: use shared ilock mode for direct IO writes by default

For the direct IO write path, we only really need the ilock to be taken in
exclusive mode during IO submission if we need to do extent allocation
instead of all the time.

Change the block mapping code to take the ilock in shared mode for the
initial block mapping, and only retake it exclusively when we actually
have to perform extent allocations.  We were already dropping the ilock
for the transaction allocation, so this doesn't introduce new race windows.

Based on an earlier patch from Dave Chinner.
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 193aec10
...@@ -1146,7 +1146,14 @@ __xfs_get_blocks( ...@@ -1146,7 +1146,14 @@ __xfs_get_blocks(
if (!create && direct && offset >= i_size_read(inode)) if (!create && direct && offset >= i_size_read(inode))
return 0; return 0;
if (create) { /*
* Direct I/O is usually done on preallocated files, so try getting
* a block mapping without an exclusive lock first. For buffered
* writes we already have the exclusive iolock anyway, so avoiding
* a lock roundtrip here by taking the ilock exclusive from the
* beginning is a useful micro optimization.
*/
if (create && !direct) {
lockmode = XFS_ILOCK_EXCL; lockmode = XFS_ILOCK_EXCL;
xfs_ilock(ip, lockmode); xfs_ilock(ip, lockmode);
} else { } else {
...@@ -1169,22 +1176,37 @@ __xfs_get_blocks( ...@@ -1169,22 +1176,37 @@ __xfs_get_blocks(
(imap.br_startblock == HOLESTARTBLOCK || (imap.br_startblock == HOLESTARTBLOCK ||
imap.br_startblock == DELAYSTARTBLOCK))) { imap.br_startblock == DELAYSTARTBLOCK))) {
if (direct) { if (direct) {
/*
* Drop the ilock in preparation for starting the block
* allocation transaction. It will be retaken
* exclusively inside xfs_iomap_write_direct for the
* actual allocation.
*/
xfs_iunlock(ip, lockmode);
error = xfs_iomap_write_direct(ip, offset, size, error = xfs_iomap_write_direct(ip, offset, size,
&imap, nimaps); &imap, nimaps);
if (error)
return -error;
} else { } else {
/*
* Delalloc reservations do not require a transaction,
* we can go on without dropping the lock here.
*/
error = xfs_iomap_write_delay(ip, offset, size, &imap); error = xfs_iomap_write_delay(ip, offset, size, &imap);
}
if (error) if (error)
goto out_unlock; goto out_unlock;
xfs_iunlock(ip, lockmode);
}
trace_xfs_get_blocks_alloc(ip, offset, size, 0, &imap); trace_xfs_get_blocks_alloc(ip, offset, size, 0, &imap);
} else if (nimaps) { } else if (nimaps) {
trace_xfs_get_blocks_found(ip, offset, size, 0, &imap); trace_xfs_get_blocks_found(ip, offset, size, 0, &imap);
xfs_iunlock(ip, lockmode);
} else { } else {
trace_xfs_get_blocks_notfound(ip, offset, size); trace_xfs_get_blocks_notfound(ip, offset, size);
goto out_unlock; goto out_unlock;
} }
xfs_iunlock(ip, lockmode);
if (imap.br_startblock != HOLESTARTBLOCK && if (imap.br_startblock != HOLESTARTBLOCK &&
imap.br_startblock != DELAYSTARTBLOCK) { imap.br_startblock != DELAYSTARTBLOCK) {
......
...@@ -142,11 +142,7 @@ xfs_iomap_write_direct( ...@@ -142,11 +142,7 @@ xfs_iomap_write_direct(
int committed; int committed;
int error; int error;
/* error = xfs_qm_dqattach(ip, 0);
* Make sure that the dquots are there. This doesn't hold
* the ilock across a disk read.
*/
error = xfs_qm_dqattach_locked(ip, 0);
if (error) if (error)
return XFS_ERROR(error); return XFS_ERROR(error);
...@@ -158,7 +154,7 @@ xfs_iomap_write_direct( ...@@ -158,7 +154,7 @@ xfs_iomap_write_direct(
if ((offset + count) > XFS_ISIZE(ip)) { if ((offset + count) > XFS_ISIZE(ip)) {
error = xfs_iomap_eof_align_last_fsb(mp, ip, extsz, &last_fsb); error = xfs_iomap_eof_align_last_fsb(mp, ip, extsz, &last_fsb);
if (error) if (error)
goto error_out; return XFS_ERROR(error);
} else { } else {
if (nmaps && (imap->br_startblock == HOLESTARTBLOCK)) if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
last_fsb = MIN(last_fsb, (xfs_fileoff_t) last_fsb = MIN(last_fsb, (xfs_fileoff_t)
...@@ -190,7 +186,6 @@ xfs_iomap_write_direct( ...@@ -190,7 +186,6 @@ xfs_iomap_write_direct(
/* /*
* Allocate and setup the transaction * Allocate and setup the transaction
*/ */
xfs_iunlock(ip, XFS_ILOCK_EXCL);
tp = xfs_trans_alloc(mp, XFS_TRANS_DIOSTRAT); tp = xfs_trans_alloc(mp, XFS_TRANS_DIOSTRAT);
error = xfs_trans_reserve(tp, resblks, error = xfs_trans_reserve(tp, resblks,
XFS_WRITE_LOG_RES(mp), resrtextents, XFS_WRITE_LOG_RES(mp), resrtextents,
...@@ -199,15 +194,16 @@ xfs_iomap_write_direct( ...@@ -199,15 +194,16 @@ xfs_iomap_write_direct(
/* /*
* Check for running out of space, note: need lock to return * Check for running out of space, note: need lock to return
*/ */
if (error) if (error) {
xfs_trans_cancel(tp, 0); xfs_trans_cancel(tp, 0);
return XFS_ERROR(error);
}
xfs_ilock(ip, XFS_ILOCK_EXCL); xfs_ilock(ip, XFS_ILOCK_EXCL);
if (error)
goto error_out;
error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0, quota_flag); error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0, quota_flag);
if (error) if (error)
goto error1; goto out_trans_cancel;
xfs_trans_ijoin(tp, ip, 0); xfs_trans_ijoin(tp, ip, 0);
...@@ -224,42 +220,39 @@ xfs_iomap_write_direct( ...@@ -224,42 +220,39 @@ xfs_iomap_write_direct(
error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb, bmapi_flag, error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb, bmapi_flag,
&firstfsb, 0, imap, &nimaps, &free_list); &firstfsb, 0, imap, &nimaps, &free_list);
if (error) if (error)
goto error0; goto out_bmap_cancel;
/* /*
* Complete the transaction * Complete the transaction
*/ */
error = xfs_bmap_finish(&tp, &free_list, &committed); error = xfs_bmap_finish(&tp, &free_list, &committed);
if (error) if (error)
goto error0; 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)
goto error_out; goto out_unlock;
/* /*
* Copy any maps to caller's array and return any error. * Copy any maps to caller's array and return any error.
*/ */
if (nimaps == 0) { if (nimaps == 0) {
error = ENOSPC; error = XFS_ERROR(ENOSPC);
goto error_out; goto out_unlock;
} }
if (!(imap->br_startblock || XFS_IS_REALTIME_INODE(ip))) { if (!(imap->br_startblock || XFS_IS_REALTIME_INODE(ip)))
error = xfs_alert_fsblock_zero(ip, imap); error = xfs_alert_fsblock_zero(ip, imap);
goto error_out;
}
return 0; out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
error0: /* Cancel bmap, unlock inode, unreserve quota blocks, cancel trans */ out_bmap_cancel:
xfs_bmap_cancel(&free_list); xfs_bmap_cancel(&free_list);
xfs_trans_unreserve_quota_nblks(tp, ip, qblocks, 0, quota_flag); xfs_trans_unreserve_quota_nblks(tp, ip, qblocks, 0, quota_flag);
out_trans_cancel:
error1: /* Just cancel transaction */
xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT); xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
goto out_unlock;
error_out:
return XFS_ERROR(error);
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment