From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Eric Biggers <ebiggers@kernel.org>,
linux-f2fs-devel@lists.sourceforge.net, Chao Yu <chao@kernel.org>,
linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
Satya Tangirala <satyaprateek2357@gmail.com>,
Changheun Lee <nanich.lee@samsung.com>,
Matthew Bobrowski <mbobrowski@mbobrowski.org>
Subject: Re: [PATCH 6/9] f2fs: implement iomap operations
Date: Thu, 22 Jul 2021 13:47:39 -0700 [thread overview]
Message-ID: <YPnZa5dFVP7vtB9q@google.com> (raw)
In-Reply-To: <YPU+3inGclUtcSpJ@infradead.org>
On 07/19, Christoph Hellwig wrote:
> On Fri, Jul 16, 2021 at 09:39:16AM -0500, Eric Biggers wrote:
> > +static blk_qc_t f2fs_dio_submit_bio(struct inode *inode, struct iomap *iomap,
> > + struct bio *bio, loff_t file_offset)
> > +{
> > + struct f2fs_private_dio *dio;
> > + bool write = (bio_op(bio) == REQ_OP_WRITE);
> > +
> > + dio = f2fs_kzalloc(F2FS_I_SB(inode),
> > + sizeof(struct f2fs_private_dio), GFP_NOFS);
> > + if (!dio)
> > + goto out;
> > +
> > + dio->inode = inode;
> > + dio->orig_end_io = bio->bi_end_io;
> > + dio->orig_private = bio->bi_private;
> > + dio->write = write;
> > +
> > + bio->bi_end_io = f2fs_dio_end_io;
> > + bio->bi_private = dio;
> > +
> > + inc_page_count(F2FS_I_SB(inode),
> > + write ? F2FS_DIO_WRITE : F2FS_DIO_READ);
> > +
> > + return submit_bio(bio);
>
> I don't think there is any need for this mess. The F2FS_DIO_WRITE /
> F2FS_DIO_READ counts are only used to check if there is any inflight
> I/O at all. So instead we can increment them once before calling
> iomap_dio_rw, and decrement them in ->end_io or for a failure/noop
> exit from iomap_dio_rw. Untested patch below. Note that all this
> would be much simpler to review if the last three patches were folded
> into a single one.
Eric, wdyt?
I've merged v1 to v5, including Christoph's comment in v2.
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 4fbf28f5aaab..9f9cc49fbe94 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3369,50 +3369,6 @@ static int f2fs_write_end(struct file *file,
> return copied;
> }
>
> -static void f2fs_dio_end_io(struct bio *bio)
> -{
> - struct f2fs_private_dio *dio = bio->bi_private;
> -
> - dec_page_count(F2FS_I_SB(dio->inode),
> - dio->write ? F2FS_DIO_WRITE : F2FS_DIO_READ);
> -
> - bio->bi_private = dio->orig_private;
> - bio->bi_end_io = dio->orig_end_io;
> -
> - kfree(dio);
> -
> - bio_endio(bio);
> -}
> -
> -static blk_qc_t f2fs_dio_submit_bio(struct inode *inode, struct iomap *iomap,
> - struct bio *bio, loff_t file_offset)
> -{
> - struct f2fs_private_dio *dio;
> - bool write = (bio_op(bio) == REQ_OP_WRITE);
> -
> - dio = f2fs_kzalloc(F2FS_I_SB(inode),
> - sizeof(struct f2fs_private_dio), GFP_NOFS);
> - if (!dio)
> - goto out;
> -
> - dio->inode = inode;
> - dio->orig_end_io = bio->bi_end_io;
> - dio->orig_private = bio->bi_private;
> - dio->write = write;
> -
> - bio->bi_end_io = f2fs_dio_end_io;
> - bio->bi_private = dio;
> -
> - inc_page_count(F2FS_I_SB(inode),
> - write ? F2FS_DIO_WRITE : F2FS_DIO_READ);
> -
> - return submit_bio(bio);
> -out:
> - bio->bi_status = BLK_STS_IOERR;
> - bio_endio(bio);
> - return BLK_QC_T_NONE;
> -}
> -
> void f2fs_invalidate_page(struct page *page, unsigned int offset,
> unsigned int length)
> {
> @@ -4006,6 +3962,18 @@ const struct iomap_ops f2fs_iomap_ops = {
> .iomap_begin = f2fs_iomap_begin,
> };
>
> +static int f2fs_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
> + unsigned flags)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(file_inode(iocb->ki_filp));
> +
> + if (iocb->ki_flags & IOCB_WRITE)
> + dec_page_count(sbi, F2FS_DIO_WRITE);
> + else
> + dec_page_count(sbi, F2FS_DIO_READ);
> + return 0;
> +}
> +
> const struct iomap_dio_ops f2fs_iomap_dio_ops = {
> - .submit_io = f2fs_dio_submit_bio,
> + .end_io = f2fs_dio_end_io,
> };
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 6dbbac05a15c..abd521dc504a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1750,13 +1750,6 @@ struct f2fs_sb_info {
> #endif
> };
>
> -struct f2fs_private_dio {
> - struct inode *inode;
> - void *orig_private;
> - bio_end_io_t *orig_end_io;
> - bool write;
> -};
> -
> #ifdef CONFIG_F2FS_FAULT_INJECTION
> #define f2fs_show_injection_info(sbi, type) \
> printk_ratelimited("%sF2FS-fs (%s) : inject %s in %s of %pS\n", \
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6b8eac6b25d4..4fed90cc1462 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4259,6 +4259,7 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
> down_read(&fi->i_gc_rwsem[READ]);
> }
>
> + inc_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
> ret = iomap_dio_rw(iocb, to, &f2fs_iomap_ops, &f2fs_iomap_dio_ops, 0);
>
> up_read(&fi->i_gc_rwsem[READ]);
> @@ -4270,6 +4271,8 @@ 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;
> @@ -4446,6 +4449,7 @@ static ssize_t f2fs_dio_write_iter(struct kiocb *iocb, struct iov_iter *from,
>
> if (pos + count > inode->i_size)
> dio_flags |= IOMAP_DIO_FORCE_WAIT;
> + inc_page_count(F2FS_I_SB(inode), F2FS_DIO_WRITE);
> ret = iomap_dio_rw(iocb, from, &f2fs_iomap_ops, &f2fs_iomap_dio_ops,
> dio_flags);
> if (ret == -ENOTBLK)
> @@ -4459,6 +4463,9 @@ static ssize_t f2fs_dio_write_iter(struct kiocb *iocb, struct iov_iter *from,
>
> up_read(&fi->i_gc_rwsem[WRITE]);
>
> + if (ret <= 0 && ret != -EIOCBQUEUED)
> + dec_page_count(F2FS_I_SB(inode), F2FS_DIO_WRITE);
> +
> if (ret < 0) {
> if (ret == -EIOCBQUEUED)
> f2fs_update_iostat(sbi, APP_DIRECT_IO,
next prev parent reply other threads:[~2021-07-22 20:47 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-16 14:39 [PATCH 0/9] f2fs: use iomap for direct I/O Eric Biggers
2021-07-16 14:39 ` [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 ` [PATCH 2/9] f2fs: remove allow_outplace_dio() Eric Biggers
2021-07-19 8:41 ` Christoph Hellwig
2021-07-16 14:39 ` [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 ` [PATCH 4/9] f2fs: reduce indentation in f2fs_file_write_iter() Eric Biggers
2021-07-16 14:39 ` [PATCH 5/9] f2fs: fix the f2fs_file_write_iter tracepoint Eric Biggers
2021-07-16 14:39 ` [PATCH 6/9] f2fs: implement iomap operations Eric Biggers
2021-07-19 8:59 ` Christoph Hellwig
2021-07-22 20:47 ` Jaegeuk Kim [this message]
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
2021-07-23 8:05 ` Eric Biggers
2021-07-16 14:39 ` [PATCH 7/9] f2fs: use iomap for direct I/O reads Eric Biggers
2021-07-16 14:39 ` [PATCH 8/9] f2fs: use iomap for direct I/O writes Eric Biggers
2021-07-16 14:39 ` [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=YPnZa5dFVP7vtB9q@google.com \
--to=jaegeuk@kernel.org \
--cc=chao@kernel.org \
--cc=ebiggers@kernel.org \
--cc=hch@infradead.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).