Commit 5f8aca8b authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Ben Myers

xfs: always hold the iolock when calling xfs_change_file_space

Currently fallocate always holds the iolock when calling into
xfs_change_file_space, while the ioctl path lets some of the lower level
functions take it, but leave it out in others.

This patch makes sure the ioctl path also always holds the iolock and
thus introduces consistent locking for the preallocation operations while
simplifying the code and allowing to kill the now unused XFS_ATTR_NOLOCK
flag.
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 001a3e73
...@@ -989,8 +989,7 @@ xfs_alloc_file_space( ...@@ -989,8 +989,7 @@ xfs_alloc_file_space(
xfs_inode_t *ip, xfs_inode_t *ip,
xfs_off_t offset, xfs_off_t offset,
xfs_off_t len, xfs_off_t len,
int alloc_type, int alloc_type)
int attr_flags)
{ {
xfs_mount_t *mp = ip->i_mount; xfs_mount_t *mp = ip->i_mount;
xfs_off_t count; xfs_off_t count;
...@@ -1248,8 +1247,7 @@ STATIC int ...@@ -1248,8 +1247,7 @@ STATIC int
xfs_free_file_space( xfs_free_file_space(
xfs_inode_t *ip, xfs_inode_t *ip,
xfs_off_t offset, xfs_off_t offset,
xfs_off_t len, xfs_off_t len)
int attr_flags)
{ {
int committed; int committed;
int done; int done;
...@@ -1267,7 +1265,6 @@ xfs_free_file_space( ...@@ -1267,7 +1265,6 @@ xfs_free_file_space(
int rt; int rt;
xfs_fileoff_t startoffset_fsb; xfs_fileoff_t startoffset_fsb;
xfs_trans_t *tp; xfs_trans_t *tp;
int need_iolock = 1;
mp = ip->i_mount; mp = ip->i_mount;
...@@ -1284,20 +1281,15 @@ xfs_free_file_space( ...@@ -1284,20 +1281,15 @@ xfs_free_file_space(
startoffset_fsb = XFS_B_TO_FSB(mp, offset); startoffset_fsb = XFS_B_TO_FSB(mp, offset);
endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len); endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
if (attr_flags & XFS_ATTR_NOLOCK)
need_iolock = 0;
if (need_iolock) {
xfs_ilock(ip, XFS_IOLOCK_EXCL);
/* wait for the completion of any pending DIOs */ /* wait for the completion of any pending DIOs */
inode_dio_wait(VFS_I(ip)); inode_dio_wait(VFS_I(ip));
}
rounding = max_t(xfs_off_t, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE); rounding = max_t(xfs_off_t, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
ioffset = offset & ~(rounding - 1); ioffset = offset & ~(rounding - 1);
error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping, error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
ioffset, -1); ioffset, -1);
if (error) if (error)
goto out_unlock_iolock; goto out;
truncate_pagecache_range(VFS_I(ip), ioffset, -1); truncate_pagecache_range(VFS_I(ip), ioffset, -1);
/* /*
...@@ -1311,7 +1303,7 @@ xfs_free_file_space( ...@@ -1311,7 +1303,7 @@ xfs_free_file_space(
error = xfs_bmapi_read(ip, startoffset_fsb, 1, error = xfs_bmapi_read(ip, startoffset_fsb, 1,
&imap, &nimap, 0); &imap, &nimap, 0);
if (error) if (error)
goto out_unlock_iolock; goto out;
ASSERT(nimap == 0 || nimap == 1); ASSERT(nimap == 0 || nimap == 1);
if (nimap && imap.br_startblock != HOLESTARTBLOCK) { if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
xfs_daddr_t block; xfs_daddr_t block;
...@@ -1326,7 +1318,7 @@ xfs_free_file_space( ...@@ -1326,7 +1318,7 @@ xfs_free_file_space(
error = xfs_bmapi_read(ip, endoffset_fsb - 1, 1, error = xfs_bmapi_read(ip, endoffset_fsb - 1, 1,
&imap, &nimap, 0); &imap, &nimap, 0);
if (error) if (error)
goto out_unlock_iolock; goto out;
ASSERT(nimap == 0 || nimap == 1); ASSERT(nimap == 0 || nimap == 1);
if (nimap && imap.br_startblock != HOLESTARTBLOCK) { if (nimap && imap.br_startblock != HOLESTARTBLOCK) {
ASSERT(imap.br_startblock != DELAYSTARTBLOCK); ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
...@@ -1412,18 +1404,15 @@ xfs_free_file_space( ...@@ -1412,18 +1404,15 @@ xfs_free_file_space(
xfs_iunlock(ip, XFS_ILOCK_EXCL); xfs_iunlock(ip, XFS_ILOCK_EXCL);
} }
out_unlock_iolock: out:
if (need_iolock)
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
return error; return error;
error0: error0:
xfs_bmap_cancel(&free_list); xfs_bmap_cancel(&free_list);
error1: error1:
xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT); xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
xfs_iunlock(ip, need_iolock ? (XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL) : xfs_iunlock(ip, XFS_ILOCK_EXCL);
XFS_ILOCK_EXCL); goto out;
return error;
} }
...@@ -1431,8 +1420,7 @@ STATIC int ...@@ -1431,8 +1420,7 @@ STATIC int
xfs_zero_file_space( xfs_zero_file_space(
struct xfs_inode *ip, struct xfs_inode *ip,
xfs_off_t offset, xfs_off_t offset,
xfs_off_t len, xfs_off_t len)
int attr_flags)
{ {
struct xfs_mount *mp = ip->i_mount; struct xfs_mount *mp = ip->i_mount;
uint granularity; uint granularity;
...@@ -1453,9 +1441,6 @@ xfs_zero_file_space( ...@@ -1453,9 +1441,6 @@ xfs_zero_file_space(
ASSERT(start_boundary >= offset); ASSERT(start_boundary >= offset);
ASSERT(end_boundary <= offset + len); ASSERT(end_boundary <= offset + len);
if (!(attr_flags & XFS_ATTR_NOLOCK))
xfs_ilock(ip, XFS_IOLOCK_EXCL);
if (start_boundary < end_boundary - 1) { if (start_boundary < end_boundary - 1) {
/* punch out the page cache over the conversion range */ /* punch out the page cache over the conversion range */
truncate_pagecache_range(VFS_I(ip), start_boundary, truncate_pagecache_range(VFS_I(ip), start_boundary,
...@@ -1463,16 +1448,16 @@ xfs_zero_file_space( ...@@ -1463,16 +1448,16 @@ xfs_zero_file_space(
/* convert the blocks */ /* convert the blocks */
error = xfs_alloc_file_space(ip, start_boundary, error = xfs_alloc_file_space(ip, start_boundary,
end_boundary - start_boundary - 1, end_boundary - start_boundary - 1,
XFS_BMAPI_PREALLOC | XFS_BMAPI_CONVERT, XFS_BMAPI_PREALLOC | XFS_BMAPI_CONVERT);
attr_flags);
if (error) if (error)
goto out_unlock; goto out;
/* We've handled the interior of the range, now for the edges */ /* We've handled the interior of the range, now for the edges */
if (start_boundary != offset) if (start_boundary != offset) {
error = xfs_iozero(ip, offset, start_boundary - offset); error = xfs_iozero(ip, offset, start_boundary - offset);
if (error) if (error)
goto out_unlock; goto out;
}
if (end_boundary != offset + len) if (end_boundary != offset + len)
error = xfs_iozero(ip, end_boundary, error = xfs_iozero(ip, end_boundary,
...@@ -1486,9 +1471,7 @@ xfs_zero_file_space( ...@@ -1486,9 +1471,7 @@ xfs_zero_file_space(
error = xfs_iozero(ip, offset, len); error = xfs_iozero(ip, offset, len);
} }
out_unlock: out:
if (!(attr_flags & XFS_ATTR_NOLOCK))
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
return error; return error;
} }
...@@ -1571,8 +1554,7 @@ xfs_change_file_space( ...@@ -1571,8 +1554,7 @@ xfs_change_file_space(
setprealloc = clrprealloc = 0; setprealloc = clrprealloc = 0;
switch (cmd) { switch (cmd) {
case XFS_IOC_ZERO_RANGE: case XFS_IOC_ZERO_RANGE:
error = xfs_zero_file_space(ip, startoffset, bf->l_len, error = xfs_zero_file_space(ip, startoffset, bf->l_len);
attr_flags);
if (error) if (error)
return error; return error;
setprealloc = 1; setprealloc = 1;
...@@ -1581,7 +1563,7 @@ xfs_change_file_space( ...@@ -1581,7 +1563,7 @@ xfs_change_file_space(
case XFS_IOC_RESVSP: case XFS_IOC_RESVSP:
case XFS_IOC_RESVSP64: case XFS_IOC_RESVSP64:
error = xfs_alloc_file_space(ip, startoffset, bf->l_len, error = xfs_alloc_file_space(ip, startoffset, bf->l_len,
XFS_BMAPI_PREALLOC, attr_flags); XFS_BMAPI_PREALLOC);
if (error) if (error)
return error; return error;
setprealloc = 1; setprealloc = 1;
...@@ -1589,8 +1571,8 @@ xfs_change_file_space( ...@@ -1589,8 +1571,8 @@ xfs_change_file_space(
case XFS_IOC_UNRESVSP: case XFS_IOC_UNRESVSP:
case XFS_IOC_UNRESVSP64: case XFS_IOC_UNRESVSP64:
if ((error = xfs_free_file_space(ip, startoffset, bf->l_len, error = xfs_free_file_space(ip, startoffset, bf->l_len);
attr_flags))) if (error)
return error; return error;
break; break;
...@@ -1608,22 +1590,17 @@ xfs_change_file_space( ...@@ -1608,22 +1590,17 @@ xfs_change_file_space(
* truncate, direct IO) from racing against the transient * truncate, direct IO) from racing against the transient
* allocated but not written state we can have here. * allocated but not written state we can have here.
*/ */
xfs_ilock(ip, XFS_IOLOCK_EXCL);
if (startoffset > fsize) { if (startoffset > fsize) {
error = xfs_alloc_file_space(ip, fsize, error = xfs_alloc_file_space(ip, fsize,
startoffset - fsize, 0, startoffset - fsize, 0);
attr_flags | XFS_ATTR_NOLOCK); if (error)
if (error) {
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
break; break;
} }
}
iattr.ia_valid = ATTR_SIZE; iattr.ia_valid = ATTR_SIZE;
iattr.ia_size = startoffset; iattr.ia_size = startoffset;
error = xfs_setattr_size(ip, &iattr); error = xfs_setattr_size(ip, &iattr);
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
if (error) if (error)
return error; return error;
......
...@@ -816,7 +816,7 @@ xfs_file_fallocate( ...@@ -816,7 +816,7 @@ xfs_file_fallocate(
xfs_flock64_t bf; xfs_flock64_t bf;
xfs_inode_t *ip = XFS_I(inode); xfs_inode_t *ip = XFS_I(inode);
int cmd = XFS_IOC_RESVSP; int cmd = XFS_IOC_RESVSP;
int attr_flags = XFS_ATTR_NOLOCK; int attr_flags = 0;
if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE)) if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
return -EOPNOTSUPP; return -EOPNOTSUPP;
......
...@@ -670,7 +670,9 @@ xfs_ioc_space( ...@@ -670,7 +670,9 @@ xfs_ioc_space(
error = mnt_want_write_file(filp); error = mnt_want_write_file(filp);
if (error) if (error)
return error; return error;
xfs_ilock(ip, XFS_IOLOCK_EXCL);
error = xfs_change_file_space(ip, cmd, bf, filp->f_pos, attr_flags); error = xfs_change_file_space(ip, cmd, bf, filp->f_pos, attr_flags);
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
mnt_drop_write_file(filp); mnt_drop_write_file(filp);
return -error; return -error;
} }
......
...@@ -31,7 +31,6 @@ extern void xfs_setup_inode(struct xfs_inode *); ...@@ -31,7 +31,6 @@ extern void xfs_setup_inode(struct xfs_inode *);
* Internal setattr interfaces. * Internal setattr interfaces.
*/ */
#define XFS_ATTR_DMI 0x01 /* invocation from a DMI function */ #define XFS_ATTR_DMI 0x01 /* invocation from a DMI function */
#define XFS_ATTR_NOLOCK 0x04 /* Don't grab any conflicting locks */
#define XFS_ATTR_NOACL 0x08 /* Don't call xfs_acl_chmod */ #define XFS_ATTR_NOACL 0x08 /* Don't call xfs_acl_chmod */
#define XFS_ATTR_SYNC 0x10 /* synchronous operation required */ #define XFS_ATTR_SYNC 0x10 /* synchronous operation required */
......
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