lkml.org 
[lkml]   [2021]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [f2fs-dev] [PATCH] f2fs: simplify accounting inflight directIO request
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?

> +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.

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.

- Eric

\
 
 \ /
  Last update: 2021-07-22 19:35    [W:0.356 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site