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/[email protected])
and Chao's new version of that patch
(https://lkml.kernel.org/r/[email protected]),
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


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to