Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] fs/direct-io: avoid data race on ->s_dio_done_wq
@ 2020-07-13  3:33 Eric Biggers
  2020-07-15  1:30 ` Dave Chinner
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2020-07-13  3:33 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-xfs, linux-ext4

From: Eric Biggers <ebiggers@google.com>

Fix the preliminary checks for ->s_dio_done_wq to use READ_ONCE(), since
it's a data race, and technically the behavior is undefined without
READ_ONCE().  Also, on one CPU architecture (Alpha), the data read
dependency barrier included in READ_ONCE() is needed to guarantee that
the pointed-to struct is seen as fully initialized.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/direct-io.c       | 8 +++-----
 fs/internal.h        | 9 ++++++++-
 fs/iomap/direct-io.c | 3 +--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 6d5370eac2a8..26221ae24156 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -590,7 +590,7 @@ static inline int dio_bio_reap(struct dio *dio, struct dio_submit *sdio)
  * filesystems that don't need it and also allows us to create the workqueue
  * late enough so the we can include s_id in the name of the workqueue.
  */
-int sb_init_dio_done_wq(struct super_block *sb)
+int __sb_init_dio_done_wq(struct super_block *sb)
 {
 	struct workqueue_struct *old;
 	struct workqueue_struct *wq = alloc_workqueue("dio/%s",
@@ -615,9 +615,7 @@ static int dio_set_defer_completion(struct dio *dio)
 	if (dio->defer_completion)
 		return 0;
 	dio->defer_completion = true;
-	if (!sb->s_dio_done_wq)
-		return sb_init_dio_done_wq(sb);
-	return 0;
+	return sb_init_dio_done_wq(sb);
 }
 
 /*
@@ -1250,7 +1248,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 		retval = 0;
 		if (iocb->ki_flags & IOCB_DSYNC)
 			retval = dio_set_defer_completion(dio);
-		else if (!dio->inode->i_sb->s_dio_done_wq) {
+		else {
 			/*
 			 * In case of AIO write racing with buffered read we
 			 * need to defer completion. We can't decide this now,
diff --git a/fs/internal.h b/fs/internal.h
index 9b863a7bd708..6736c9eee978 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -178,7 +178,14 @@ extern void mnt_pin_kill(struct mount *m);
 extern const struct dentry_operations ns_dentry_operations;
 
 /* direct-io.c: */
-int sb_init_dio_done_wq(struct super_block *sb);
+int __sb_init_dio_done_wq(struct super_block *sb);
+static inline int sb_init_dio_done_wq(struct super_block *sb)
+{
+	/* pairs with cmpxchg() in __sb_init_dio_done_wq() */
+	if (likely(READ_ONCE(sb->s_dio_done_wq)))
+		return 0;
+	return __sb_init_dio_done_wq(sb);
+}
 
 /*
  * fs/stat.c:
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ec7b78e6feca..dc7fe898dab8 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -487,8 +487,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		dio_warn_stale_pagecache(iocb->ki_filp);
 	ret = 0;
 
-	if (iov_iter_rw(iter) == WRITE && !wait_for_completion &&
-	    !inode->i_sb->s_dio_done_wq) {
+	if (iov_iter_rw(iter) == WRITE && !wait_for_completion) {
 		ret = sb_init_dio_done_wq(inode->i_sb);
 		if (ret < 0)
 			goto out_free_dio;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13  3:33 [PATCH] fs/direct-io: avoid data race on ->s_dio_done_wq Eric Biggers
2020-07-15  1:30 ` Dave Chinner
2020-07-15  2:37   ` Eric Biggers
2020-07-15  8:01     ` Dave Chinner
2020-07-15 16:13       ` Eric Biggers
2020-07-15 16:41         ` Darrick J. Wong
2020-07-16  1:46         ` Dave Chinner
2020-07-16  2:39           ` Eric Biggers
2020-07-16  2:47           ` Matthew Wilcox
2020-07-16  3:19             ` Eric Biggers
2020-07-16  5:33             ` Eric Biggers
2020-07-16  8:16               ` Dave Chinner
2020-07-17  1:36                 ` Darrick J. Wong

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lkml.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git