Linux-f2fs-devel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Satya Tangirala <satyaprateek2357@gmail.com>,
	linux-xfs@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Christoph Hellwig <hch@infradead.org>,
	Matthew Bobrowski <mbobrowski@mbobrowski.org>,
	Changheun Lee <nanich.lee@samsung.com>,
	linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>
Subject: Re: [f2fs-dev] [PATCH 6/9] f2fs: implement iomap operations
Date: Fri, 23 Jul 2021 06:00:03 +0100	[thread overview]
Message-ID: <YPpM09DLTB28obqQ@infradead.org> (raw)
In-Reply-To: <YPog4SDY3nNC78sK@sol.localdomain>

On Thu, Jul 22, 2021 at 06:52:33PM -0700, Eric Biggers wrote:
> I am trying to do this, but unfortunately I don't see a way to make it work
> correctly in all cases.
> 
> The main problem is that when iomap_dio_rw() returns an error (other than
> -EIOCBQUEUED), there is no way to know whether ->end_io() has been called or
> not.  This is because iomap_dio_rw() can fail either early, before "starting"
> the I/O (in which case ->end_io() won't have been called), or later, after
> "starting" the I/O (in which case ->end_io() will have been called).  Note that
> this can't be worked around by checking whether the iov_iter has been advanced
> or not, since a failure could occur between "starting" the I/O and the iov_iter
> being advanced for the first time.
> 
> Would you be receptive to adding a ->begin_io() callback to struct iomap_dio_ops
> in order to allow filesystems to maintain counters like this?

I think we can triviall fix this by using the slightly lower level
__iomap_dio_rw API.  Incremental patch to my previous one below:

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 4fed90cc1462..11844bd0cb7a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4243,6 +4243,7 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	struct f2fs_inode_info *fi = F2FS_I(inode);
 	const loff_t pos = iocb->ki_pos;
 	const size_t count = iov_iter_count(to);
+	struct iomap_dio *dio;
 	ssize_t ret;
 
 	if (count == 0)
@@ -4260,8 +4261,13 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	}
 
 	inc_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
-	ret = iomap_dio_rw(iocb, to, &f2fs_iomap_ops, &f2fs_iomap_dio_ops, 0);
-
+	dio = __iomap_dio_rw(iocb, to, &f2fs_iomap_ops, &f2fs_iomap_dio_ops, 0);
+	if (IS_ERR_OR_NULL(dio)) {
+		dec_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
+		ret = PTR_ERR_OR_ZERO(dio);
+	} else {
+		ret = iomap_dio_complete(dio);
+	}
 	up_read(&fi->i_gc_rwsem[READ]);
 
 	file_accessed(file);
@@ -4271,8 +4277,6 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	else if (ret == -EIOCBQUEUED)
 		f2fs_update_iostat(F2FS_I_SB(inode), APP_DIRECT_READ_IO,
 				   count - iov_iter_count(to));
-	else
-		dec_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
 out:
 	trace_f2fs_direct_IO_exit(inode, pos, count, READ, ret);
 	return ret;


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2021-07-23  5:01 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16 14:39 [f2fs-dev] [PATCH 0/9] f2fs: use iomap for direct I/O Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 1/9] f2fs: make f2fs_write_failed() take struct inode Eric Biggers
2021-07-25 10:00   ` Chao Yu
2021-07-16 14:39 ` [f2fs-dev] [PATCH 2/9] f2fs: remove allow_outplace_dio() Eric Biggers
2021-07-19  8:41   ` Christoph Hellwig
2021-07-16 14:39 ` [f2fs-dev] [PATCH 3/9] f2fs: rework write preallocations Eric Biggers
2021-07-25 10:50   ` Chao Yu
2021-07-25 17:57     ` Eric Biggers
2021-07-27  2:00       ` Jaegeuk Kim
2021-07-27  3:23         ` Chao Yu
2021-07-27  7:38           ` Eric Biggers
2021-07-27  8:30             ` Chao Yu
2021-07-27 15:33               ` Darrick J. Wong
2021-07-29  0:26                 ` Chao Yu
2021-07-28  2:29         ` Jaegeuk Kim
2021-07-25 15:35   ` Jaegeuk Kim
2021-07-25 15:47     ` Jaegeuk Kim
2021-07-25 18:01       ` Eric Biggers
2021-07-26 19:04         ` Jaegeuk Kim
2021-07-16 14:39 ` [f2fs-dev] [PATCH 4/9] f2fs: reduce indentation in f2fs_file_write_iter() Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 5/9] f2fs: fix the f2fs_file_write_iter tracepoint Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 6/9] f2fs: implement iomap operations Eric Biggers
2021-07-19  8:59   ` Christoph Hellwig
2021-07-22 20:47     ` Jaegeuk Kim
2021-07-22 20:49       ` Jaegeuk Kim
2021-07-22 20:54       ` Eric Biggers
2021-07-22 21:57         ` Jaegeuk Kim
2021-07-23  1:52     ` Eric Biggers
2021-07-23  5:00       ` Christoph Hellwig [this message]
2021-07-23  8:05         ` Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 7/9] f2fs: use iomap for direct I/O reads Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 8/9] f2fs: use iomap for direct I/O writes Eric Biggers
2021-07-16 14:39 ` [f2fs-dev] [PATCH 9/9] f2fs: remove f2fs_direct_IO() Eric Biggers

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=YPpM09DLTB28obqQ@infradead.org \
    --to=hch@infradead.org \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    --cc=nanich.lee@samsung.com \
    --cc=satyaprateek2357@gmail.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 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).