Commit bc4010ec authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers
Browse files

xfs: use iolock on XFS_IOC_ALLOCSP calls



fsstress has a particular effective way of stopping debug XFS
kernels. We keep seeing assert failures due finding delayed
allocation extents where there should be none. This shows up when
extracting extent maps and we are holding all the locks we should be
to prevent races, so this really makes no sense to see these errors.

After checking that fsstress does not use mmap, it occurred to me
that fsstress uses something that no sane application uses - the
XFS_IOC_ALLOCSP ioctl interfaces for preallocation. These interfaces
do allocation of blocks beyond EOF without using preallocation, and
then call setattr to extend and zero the allocated blocks.

THe problem here is this is a buffered write, and hence the
allocation is a delayed allocation. Unlike the buffered IO path, the
allocation and zeroing are not serialised using the IOLOCK. Hence
the ALLOCSP operation can race with operations holding the iolock to
prevent buffered IO operations from occurring.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent aa5c158e
...@@ -2315,17 +2315,33 @@ xfs_change_file_space( ...@@ -2315,17 +2315,33 @@ xfs_change_file_space(
case XFS_IOC_ALLOCSP64: case XFS_IOC_ALLOCSP64:
case XFS_IOC_FREESP: case XFS_IOC_FREESP:
case XFS_IOC_FREESP64: case XFS_IOC_FREESP64:
/*
* These operations actually do IO when extending the file, but
* the allocation is done seperately to the zeroing that is
* done. This set of operations need to be serialised against
* other IO operations, such as truncate and buffered IO. We
* need to take the IOLOCK here to serialise the allocation and
* zeroing IO to prevent other IOLOCK holders (e.g. getbmap,
* truncate, direct IO) from racing against the transient
* 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, attr_flags); startoffset - fsize, 0,
if (error) attr_flags | XFS_ATTR_NOLOCK);
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, attr_flags); error = xfs_setattr_size(ip, &iattr,
attr_flags | XFS_ATTR_NOLOCK);
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
if (error) if (error)
return error; return error;
......
Supports Markdown
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