xfs: avoid direct I/O write vs buffered I/O race
PATCH 1/1 xfs: avoid direct I/O write vs buffered I/O race
Currently a buffered reader or writer can add pages to the pagecache while we are waiting for the iolock in xfs_file_dio_aio_write. Prevent this by re-checking mapping->nrpages after we got the iolock, and if nessecary upgrade the lock to exclusive mode. To simplify this a bit only take the ilock inside of xfs_file_aio_write_checks.
fs/xfs/xfs_file.c +14 -3
@@ -680,6 +680,7 @@ xfs_file_aio_write_checks(xfs_fsize_t new_size;int error =0 ; +xfs_rw_ilock (ip,XFS_ILOCK_EXCL ); *new_sizep =0 ; restart: error =generic_write_checks (file, pos, count,S_ISBLK (inode->i_mode));
@@ -802,14 +803,24 @@ xfs_file_dio_aio_write( *iolock =XFS_IOLOCK_EXCL ; else *iolock =XFS_IOLOCK_SHARED ; -xfs_rw_ilock (ip,XFS_ILOCK_EXCL | *iolock); +xfs_rw_ilock (ip, *iolock); + +/* +* Recheck if there are cached pages that need invalidate after we got +* the iolock to protect against other threads adding new pages while +* we were waiting for the iolock. +*/ +if (mapping->nrpages && *iolock ==XFS_IOLOCK_SHARED ) { +xfs_rw_iunlock (ip, *iolock); + *iolock =XFS_IOLOCK_EXCL ; +xfs_rw_ilock (ip, *iolock); + } ret =xfs_file_aio_write_checks (file, &pos, &count, new_size, iolock);if (ret)return ret;if (mapping->nrpages) { -WARN_ON (*iolock !=XFS_IOLOCK_EXCL ); ret = -xfs_flushinval_pages (ip, (pos &PAGE_CACHE_MASK ),-1 ,FI_REMAPF_LOCKED );if (ret)
@@ -855,7 +866,7 @@ xfs_file_buffered_aio_write(size_t count = ocount; *iolock =XFS_IOLOCK_EXCL ; -xfs_rw_ilock (ip,XFS_ILOCK_EXCL | *iolock); +xfs_rw_ilock (ip, *iolock); ret =xfs_file_aio_write_checks (file, &pos, &count, new_size, iolock);if (ret)
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Christoph Hellwig
Currently a buffered reader or writer can add pages to the pagecache while we are waiting for the iolock in
xfs_file_dio_aio_write. Prevent this by re-checkingmapping->nrpagesafter we got the iolock, and if nessecary upgrade the lock to exclusive mode. To simplify this a bit only take the ilock inside ofxfs_file_aio_write_checks.Signed-off-by Christoph Hellwig <[email protected]>
Looks sane. Pushing the ILOCK completely into xfs_file_aio_write_checks() is a nice segregation of locking responsibilities.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Christoph Hellwig
Currently a buffered reader or writer can add pages to the pagecache while we are waiting for the iolock in
xfs_file_dio_aio_write. Prevent this by re-checkingmapping->nrpagesafter we got the iolock, and if nessecary upgrade the lock to exclusive mode. To simplify this a bit only take the ilock inside ofxfs_file_aio_write_checks.Signed-off-by Christoph Hellwig <[email protected]>
I agree with Dave, moving the ILOCK acquisition into xfs_file_aio_write_checks() is very nice.
In any case the core change looks good.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs