* [PATCH] xfs: avoid direct I/O write vs buffered I/O race
@ 2011-08-27 14:42 Christoph Hellwig
2011-08-30 6:29 ` Dave Chinner
2011-09-12 18:43 ` Alex Elder
0 siblings, 2 replies; 3+ messages in thread
From: Christoph Hellwig @ 2011-08-27 14:42 UTC (permalink / raw)
To: xfs
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.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_file.c
===================================================================
--- xfs.orig/fs/xfs/xfs_file.c 2011-08-27 08:22:18.537779834 +0200
+++ xfs/fs/xfs/xfs_file.c 2011-08-27 08:31:43.296278506 +0200
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: avoid direct I/O write vs buffered I/O race
2011-08-27 14:42 [PATCH] xfs: avoid direct I/O write vs buffered I/O race Christoph Hellwig
@ 2011-08-30 6:29 ` Dave Chinner
2011-09-12 18:43 ` Alex Elder
1 sibling, 0 replies; 3+ messages in thread
From: Dave Chinner @ 2011-08-30 6:29 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Sat, Aug 27, 2011 at 10:42:53AM -0400, Christoph Hellwig wrote:
> 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.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks sane. Pushing the ILOCK completely into
xfs_file_aio_write_checks() is a nice segregation of locking
responsibilities.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: avoid direct I/O write vs buffered I/O race
2011-08-27 14:42 [PATCH] xfs: avoid direct I/O write vs buffered I/O race Christoph Hellwig
2011-08-30 6:29 ` Dave Chinner
@ 2011-09-12 18:43 ` Alex Elder
1 sibling, 0 replies; 3+ messages in thread
From: Alex Elder @ 2011-09-12 18:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Sat, 2011-08-27 at 10:42 -0400, Christoph Hellwig wrote:
> 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.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
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.
Reviewed-by: Alex Elder <aelder@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-12 18:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-27 14:42 [PATCH] xfs: avoid direct I/O write vs buffered I/O race Christoph Hellwig
2011-08-30 6:29 ` Dave Chinner
2011-09-12 18:43 ` Alex Elder
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.