On 07/22, Eric Biggers wrote:
> 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.
Yup, I dropped it in dev branch, and wait for Chao's next patch on top of
iomap.
>
> - Eric
