All of lore.kernel.org
 help / color / mirror / Atom feed
From: Goldwyn Rodrigues <rgoldwyn@suse.de>
To: linux-fsdevel@vger.kernel.org
Cc: linux-btrfs@vger.kernel.org, david@fromorbit.com, hch@lst.de,
	johannes.thumshirn@wdc.com, dsterba@suse.com,
	darrick.wong@oracle.com, josef@toxicpanda.com,
	Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: [PATCH 14/15] btrfs: Revert 09745ff88d93 ("btrfs: dio iomap DSYNC workaround")
Date: Mon, 21 Sep 2020 09:43:52 -0500	[thread overview]
Message-ID: <20200921144353.31319-15-rgoldwyn@suse.de> (raw)
In-Reply-To: <20200921144353.31319-1-rgoldwyn@suse.de>

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

iomap_dio_complete()->generic_write_sync() is not called under i_rwsem
anymore, revert the workaround.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/ctree.h       |  1 -
 fs/btrfs/file.c        | 38 ++------------------------------
 fs/btrfs/inode.c       | 50 ------------------------------------------
 fs/btrfs/transaction.h |  1 -
 4 files changed, 2 insertions(+), 88 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index ea15771bf3da..bc96c52021b2 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3051,7 +3051,6 @@ void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
 extern const struct dentry_operations btrfs_dentry_operations;
 extern const struct iomap_ops btrfs_dio_iomap_ops;
 extern const struct iomap_dio_ops btrfs_dio_ops;
-extern const struct iomap_dio_ops btrfs_sync_dops;
 
 /* ilock flags definition */
 #define BTRFS_ILOCK_SHARED	(1 << 0)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 9c7a2d4b4148..1d3363e7b09d 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2094,44 +2094,10 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 	if (sync)
 		atomic_inc(&BTRFS_I(inode)->sync_writers);
 
-	if (iocb->ki_flags & IOCB_DIRECT) {
-		/*
-		 * 1. We must always clear IOCB_DSYNC in order to not deadlock
-		 *    in iomap, as it calls generic_write_sync() in this case.
-		 * 2. If we are async, we can call iomap_dio_complete() either
-		 *    in
-		 *
-		 *    2.1. A worker thread from the last bio completed.  In this
-		 *	   case we need to mark the btrfs_dio_data that it is
-		 *	   async in order to call generic_write_sync() properly.
-		 *	   This is handled by setting BTRFS_DIO_SYNC_STUB in the
-		 *	   current->journal_info.
-		 *    2.2  The submitter context, because all IO completed
-		 *         before we exited iomap_dio_rw().  In this case we can
-		 *         just re-set the IOCB_DSYNC on the iocb and we'll do
-		 *         the sync below.  If our ->end_io() gets called and
-		 *         current->journal_info is set, then we know we're in
-		 *         our current context and we will clear
-		 *         current->journal_info to indicate that we need to
-		 *         sync below.
-		 */
-		if (sync) {
-			ASSERT(current->journal_info == NULL);
-			iocb->ki_flags &= ~IOCB_DSYNC;
-			current->journal_info = BTRFS_DIO_SYNC_STUB;
-		}
+	if (iocb->ki_flags & IOCB_DIRECT)
 		num_written = btrfs_direct_write(iocb, from);
-
-		/*
-		 * As stated above, we cleared journal_info, so we need to do
-		 * the sync ourselves.
-		 */
-		if (sync && current->journal_info == NULL)
-			iocb->ki_flags |= IOCB_DSYNC;
-		current->journal_info = NULL;
-	} else {
+	else
 		num_written = btrfs_buffered_write(iocb, from);
-	}
 
 	/*
 	 * We also have to set last_sub_trans to the current log transid,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 96ff8e4a3b45..41e32ef6b6ef 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -62,7 +62,6 @@ struct btrfs_dio_data {
 	loff_t length;
 	ssize_t submitted;
 	struct extent_changeset *data_reserved;
-	bool sync;
 };
 
 static const struct inode_operations btrfs_dir_inode_operations;
@@ -7362,17 +7361,6 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start,
 	int ret = 0;
 	u64 len = length;
 	bool unlock_extents = false;
-	bool sync = (current->journal_info == BTRFS_DIO_SYNC_STUB);
-
-	/*
-	 * We used current->journal_info here to see if we were sync, but
-	 * there's a lot of tests in the enospc machinery to not do flushing if
-	 * we have a journal_info set, so we need to clear this out and re-set
-	 * it in iomap_end.
-	 */
-	ASSERT(current->journal_info == NULL ||
-	       current->journal_info == BTRFS_DIO_SYNC_STUB);
-	current->journal_info = NULL;
 
 	if (!write)
 		len = min_t(u64, len, fs_info->sectorsize);
@@ -7398,7 +7386,6 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start,
 	if (!dio_data)
 		return -ENOMEM;
 
-	dio_data->sync = sync;
 	dio_data->length = length;
 	if (write) {
 		dio_data->reserve = round_up(length, fs_info->sectorsize);
@@ -7546,14 +7533,6 @@ static int btrfs_dio_iomap_end(struct inode *inode, loff_t pos, loff_t length,
 		extent_changeset_free(dio_data->data_reserved);
 	}
 out:
-	/*
-	 * We're all done, we can re-set the current->journal_info now safely
-	 * for our endio.
-	 */
-	if (dio_data->sync) {
-		ASSERT(current->journal_info == NULL);
-		current->journal_info = BTRFS_DIO_SYNC_STUB;
-	}
 	kfree(dio_data);
 	iomap->private = NULL;
 
@@ -7929,30 +7908,6 @@ static blk_qc_t btrfs_submit_direct(struct inode *inode, struct iomap *iomap,
 	return BLK_QC_T_NONE;
 }
 
-static inline int btrfs_maybe_fsync_end_io(struct kiocb *iocb, ssize_t size,
-					   int error, unsigned flags)
-{
-	/*
-	 * Now if we're still in the context of our submitter we know we can't
-	 * safely run generic_write_sync(), so clear our flag here so that the
-	 * caller knows to follow up with a sync.
-	 */
-	if (current->journal_info == BTRFS_DIO_SYNC_STUB) {
-		current->journal_info = NULL;
-		return error;
-	}
-
-	if (error)
-		return error;
-
-	if (size) {
-		iocb->ki_flags |= IOCB_DSYNC;
-		return generic_write_sync(iocb, size);
-	}
-
-	return 0;
-}
-
 const struct iomap_ops btrfs_dio_iomap_ops = {
 	.iomap_begin            = btrfs_dio_iomap_begin,
 	.iomap_end              = btrfs_dio_iomap_end,
@@ -7962,11 +7917,6 @@ const struct iomap_dio_ops btrfs_dio_ops = {
 	.submit_io		= btrfs_submit_direct,
 };
 
-const struct iomap_dio_ops btrfs_sync_dops = {
-	.submit_io		= btrfs_submit_direct,
-	.end_io			= btrfs_maybe_fsync_end_io,
-};
-
 static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 			u64 start, u64 len)
 {
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 858d9153a1cd..8241c050ba71 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -112,7 +112,6 @@ struct btrfs_transaction {
 #define TRANS_EXTWRITERS	(__TRANS_START | __TRANS_ATTACH)
 
 #define BTRFS_SEND_TRANS_STUB	((void *)1)
-#define BTRFS_DIO_SYNC_STUB	((void *)2)
 
 struct btrfs_trans_handle {
 	u64 transid;
-- 
2.26.2


  parent reply	other threads:[~2020-09-21 14:44 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-21 14:43 [PATCH 0/15 v2] BTRFS DIO inode locking/D_SYNC fix Goldwyn Rodrigues
2020-09-21 14:43 ` [PATCH 01/15] fs: remove dio_end_io() Goldwyn Rodrigues
2020-09-22 14:17   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 02/15] btrfs: remove BTRFS_INODE_READDIO_NEED_LOCK Goldwyn Rodrigues
2020-09-22 13:18   ` Christoph Hellwig
2020-09-22 14:17   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 03/15] iomap: Allow filesystem to call iomap_dio_complete without i_rwsem Goldwyn Rodrigues
2020-09-21 15:09   ` Johannes Thumshirn
2020-09-22 13:19     ` hch
2020-09-22  9:24   ` Dan Carpenter
2020-09-22  9:24     ` Dan Carpenter
2020-09-22 14:16     ` Goldwyn Rodrigues
2020-09-22 14:58       ` Dan Carpenter
2020-09-22 14:58         ` Dan Carpenter
2020-09-22 16:06         ` Goldwyn Rodrigues
2020-09-22 14:17   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 04/15] iomap: Call inode_dio_end() before generic_write_sync() Goldwyn Rodrigues
2020-09-21 15:11   ` Johannes Thumshirn
2020-09-22 13:21   ` Christoph Hellwig
2020-09-22 14:20   ` Josef Bacik
2020-09-22 16:31     ` Darrick J. Wong
2020-09-22 17:25       ` Goldwyn Rodrigues
2020-09-22 21:49       ` Dave Chinner
2020-09-23  5:16         ` Christoph Hellwig
2020-09-23  5:31           ` Darrick J. Wong
2020-09-23  5:49             ` Christoph Hellwig
2020-09-23  5:59               ` Dave Chinner
2020-09-21 14:43 ` [PATCH 05/15] btrfs: split btrfs_direct_IO to read and write Goldwyn Rodrigues
2020-09-22 13:22   ` Christoph Hellwig
2020-09-22 14:27   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 06/15] btrfs: Move pos increment and pagecache extension to btrfs_buffered_write() Goldwyn Rodrigues
2020-09-22 13:22   ` Christoph Hellwig
2020-09-22 14:30   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 07/15] btrfs: Move FS error state bit early during write Goldwyn Rodrigues
2020-09-22 14:38   ` Josef Bacik
2020-09-23  9:10   ` Nikolay Borisov
2020-09-23 14:07     ` Goldwyn Rodrigues
2020-09-21 14:43 ` [PATCH 08/15] btrfs: Introduce btrfs_write_check() Goldwyn Rodrigues
2020-09-22 13:26   ` Christoph Hellwig
2020-09-22 14:42   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 09/15] btrfs: Introduce btrfs_inode_lock()/unlock() Goldwyn Rodrigues
2020-09-22 14:45   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 10/15] btrfs: Push inode locking and unlocking into buffered/direct write Goldwyn Rodrigues
2020-09-22  9:26   ` Dan Carpenter
2020-09-22  9:26     ` Dan Carpenter
2020-09-22 14:48   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 11/15] btrfs: Use inode_lock_shared() for direct writes within EOF Goldwyn Rodrigues
2020-09-22 14:52   ` Josef Bacik
2020-09-22 17:33     ` Goldwyn Rodrigues
2020-09-21 14:43 ` [PATCH 12/15] btrfs: Remove dio_sem Goldwyn Rodrigues
2020-09-22 14:52   ` Josef Bacik
2020-09-21 14:43 ` [PATCH 13/15] btrfs: Call iomap_dio_complete() without inode_lock Goldwyn Rodrigues
2020-09-22 15:11   ` Josef Bacik
2020-09-21 14:43 ` Goldwyn Rodrigues [this message]
2020-09-22 15:12   ` [PATCH 14/15] btrfs: Revert 09745ff88d93 ("btrfs: dio iomap DSYNC workaround") Josef Bacik
2020-09-21 14:43 ` [PATCH 15/15] iomap: Reinstate lockdep_assert_held in iomap_dio_rw() Goldwyn Rodrigues
2020-09-22 13:26   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200921144353.31319-15-rgoldwyn@suse.de \
    --to=rgoldwyn@suse.de \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=dsterba@suse.com \
    --cc=hch@lst.de \
    --cc=johannes.thumshirn@wdc.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=rgoldwyn@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.