linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ritesh Harjani <riteshh@linux.ibm.com>
To: jack@suse.cz, tytso@mit.edu, linux-ext4@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org, mbobrowski@mbobrowski.org,
	riteshh@linux.ibm.com, joseph.qi@linux.alibaba.com
Subject: [PATCHv4 3/3] ext4: Move to shared i_rwsem even without dioread_nolock mount opt
Date: Thu,  5 Dec 2019 12:16:24 +0530	[thread overview]
Message-ID: <20191205064624.13419-4-riteshh@linux.ibm.com> (raw)
In-Reply-To: <20191205064624.13419-1-riteshh@linux.ibm.com>

We were using shared locking only in case of dioread_nolock mount option in case
of DIO overwrites. This mount condition is not needed anymore with current code,
since:-

1. No race between buffered writes & DIO overwrites. Since buffIO writes takes
exclusive lock & DIO overwrites will take shared locking. Also DIO path will
make sure to flush and wait for any dirty page cache data.

2. No race between buffered reads & DIO overwrites, since there is no block
allocation that is possible with DIO overwrites. So no stale data exposure
should happen. Same is the case between DIO reads & DIO overwrites.

3. Also other paths like truncate is protected, since we wait there for any DIO
in flight to be over.

4. In case of buffIO writes followed by DIO reads:- since here also we take
exclusive lock in ext4_write_begin/end(). There is no risk of exposing any
stale data in this case. Since after ext4_write_end, iomap_dio_rw() will flush &
wait for any dirty page cache data to be written.

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 fs/ext4/file.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index cbafaec9e4fc..682ed956eb02 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -392,8 +392,8 @@ static const struct iomap_dio_ops ext4_dio_write_ops = {
  * - For extending writes case we don't take the shared lock, since it requires
  *   updating inode i_disksize and/or orphan handling with exclusive lock.
  *
- * - shared locking will only be true mostly with overwrites in dioread_nolock
- *   mode. Otherwise we will switch to exclusive i_rwsem lock.
+ * - shared locking will only be true mostly with overwrites. Otherwise we will
+ *   switch to exclusive i_rwsem lock.
  */
 static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
 				     bool *ilock_shared, bool *extend)
@@ -415,14 +415,11 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
 		*extend = true;
 	/*
 	 * Determine whether the IO operation will overwrite allocated
-	 * and initialized blocks. If so, check to see whether it is
-	 * possible to take the dioread_nolock path.
-	 *
+	 * and initialized blocks.
 	 * We need exclusive i_rwsem for changing security info
 	 * in file_modified().
 	 */
 	if (*ilock_shared && (!IS_NOSEC(inode) || *extend ||
-	     !ext4_should_dioread_nolock(inode) ||
 	     !ext4_overwrite_io(inode, offset, count))) {
 		inode_unlock_shared(inode);
 		*ilock_shared = false;
-- 
2.21.0


  parent reply	other threads:[~2019-12-05  6:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05  6:46 [PATCHv4 0/3] Fix inode_lock sequence to scale performance of DIO mixed R/W workload Ritesh Harjani
2019-12-05  6:46 ` [PATCHv4 1/3] ext4: fix ext4_dax_read/write inode locking sequence for IOCB_NOWAIT Ritesh Harjani
2019-12-05  6:46 ` [PATCHv4 2/3] ext4: Start with shared i_rwsem in case of DIO instead of exclusive Ritesh Harjani
2019-12-05 12:03   ` Jan Kara
2019-12-05 13:40     ` Ritesh Harjani
2019-12-05  6:46 ` Ritesh Harjani [this message]
2019-12-05 12:05   ` [PATCHv4 3/3] ext4: Move to shared i_rwsem even without dioread_nolock mount opt Jan Kara
2019-12-05 13:41     ` Ritesh Harjani
2019-12-05 13:46     ` Ritesh Harjani
2019-12-06  8:46 ` [PATCHv4 0/3] Fix inode_lock sequence to scale performance of DIO mixed R/W workload Joseph Qi
2019-12-06  8:49   ` Ritesh Harjani

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=20191205064624.13419-4-riteshh@linux.ibm.com \
    --to=riteshh@linux.ibm.com \
    --cc=jack@suse.cz \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    --cc=tytso@mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).