On 2021/7/22 23:48, Eric Biggers wrote:
On Thu, Jul 22, 2021 at 09:16:17PM +0800, Chao Yu wrote:
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ba120d55e9b1..d0a1ca6ae38e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1720,6 +1720,9 @@ static int __get_data_block(struct inode *inode, sector_t 
iblock,
                map_bh(bh, inode->i_sb, map.m_pblk);
                bh->b_state = (bh->b_state & ~F2FS_MAP_FLAGS) | map.m_flags;
                bh->b_size = blks_to_bytes(inode, map.m_len);
+
+               if (flag == F2FS_GET_BLOCK_DIO)
+                       bh->b_private = (void *)may_write;

Why is this hunk needed?

For passing rw info from get_block() to dio->private.


+static int f2fs_dio_end_io(struct kiocb *iocb, loff_t offset,
+                                       ssize_t bytes, void *private)
  {
-       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);
+       struct inode *inode = file_inode(iocb->ki_filp);
+       bool may_write = private;
- bio_endio(bio);
+       dec_dio_req_count(F2FS_I_SB(inode), may_write ? WRITE : READ);
+       return 0;
  }
static void f2fs_dio_submit_bio(struct bio *bio, struct inode *inode,
                                                        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);
+       inc_dio_req_count(F2FS_I_SB(inode),
+                       op_is_write(bio_op(bio)) ? WRITE : READ);
submit_bio(bio);
-       return;
-out:
-       bio->bi_status = BLK_STS_IOERR;
-       bio_endio(bio);
  }

The inc and dec here aren't correctly paired, since f2fs_dio_submit_bio() is
called once per bio whereas f2fs_dio_end_io() is called when the entire direct
I/O request (which may have consisted of multiple bios) has completed.

Correct, it still needs to hook bio->bi_end_io rather than hooking dio->end_io,
however, w/o one extra allocated memory, we have no way to pass sbi to
f2fs_dio_end_io() for dec_dio_req_count(sbi, ...) invoking.


The correct way to do this would be to do the inc before calling
__blockdev_direct_IO(), and do the dec in end_io or if __blockdev_direct_IO()
returned without actually issuing any I/O.

But I think you shouldn't bother with this part of the change before we switch
to iomap, as it will then need to be changed again anyway.

Agreed.

Thanks,


- Eric



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

Reply via email to