On Thu, Jul 22, 2021 at 01:47:39PM -0700, Jaegeuk Kim wrote:
> 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.
>
I am planning to do this, but I got caught up by the patch
"f2fs: fix wrong inflight page stats for directIO" that was recently added to
f2fs.git#dev, which makes this suggestion no longer viable. Hence my review
comment on that patch
(https://lkml.kernel.org/r/YPjNGoFzQojO5Amr@sol.localdomain)
and Chao's new version of that patch
(https://lkml.kernel.org/r/20210722131617.749204-1-chao@xxxxxxxxxx),
although the new version has some issues too as I commented.
If you could just revert "f2fs: fix wrong inflight page stats for directIO"
for now, that would be helpful, as I don't think we want it.
- Eric
