From: "Darrick J. Wong" <djwong@kernel.org>
To: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
Cc: linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
Matthew Wilcox <willy@infradead.org>,
Ojaswin Mujoo <ojaswin@linux.ibm.com>, Jan Kara <jack@suse.cz>
Subject: Re: [RFCv3 2/7] ext2: Convert ext2 regular file buffered I/O to use iomap
Date: Fri, 26 Apr 2024 08:29:49 -0700 [thread overview]
Message-ID: <20240426152949.GJ360919@frogsfrogsfrogs> (raw)
In-Reply-To: <54d3fdabeb82e494fab83204cd49e75b58ef298e.1714046808.git.ritesh.list@gmail.com>
On Thu, Apr 25, 2024 at 06:58:46PM +0530, Ritesh Harjani (IBM) wrote:
> This patch converts ext2 regular file's buffered-io path to use iomap.
> - buffered-io path using iomap_file_buffered_write
> - DIO fallback to buffered-io now uses iomap_file_buffered_write
> - writeback path now uses a new aops - ext2_file_aops
> - truncate now uses iomap_truncate_page
> - mmap path of ext2 continues to use generic_file_vm_ops
>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
> fs/ext2/file.c | 20 ++++++++++++--
> fs/ext2/inode.c | 69 ++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 81 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index 4ddc36f4dbd4..ee5cd4a2f24f 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -252,7 +252,7 @@ static ssize_t ext2_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
>
> iocb->ki_flags &= ~IOCB_DIRECT;
> pos = iocb->ki_pos;
> - status = generic_perform_write(iocb, from);
> + status = iomap_file_buffered_write(iocb, from, &ext2_iomap_ops);
> if (unlikely(status < 0)) {
> ret = status;
> goto out_unlock;
> @@ -278,6 +278,22 @@ static ssize_t ext2_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
> return ret;
> }
>
> +static ssize_t ext2_buffered_write_iter(struct kiocb *iocb,
> + struct iov_iter *from)
> +{
> + ssize_t ret = 0;
> + struct inode *inode = file_inode(iocb->ki_filp);
> +
> + inode_lock(inode);
> + ret = generic_write_checks(iocb, from);
> + if (ret > 0)
> + ret = iomap_file_buffered_write(iocb, from, &ext2_iomap_ops);
> + inode_unlock(inode);
> + if (ret > 0)
> + ret = generic_write_sync(iocb, ret);
> + return ret;
> +}
> +
> static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> {
> #ifdef CONFIG_FS_DAX
> @@ -299,7 +315,7 @@ static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> if (iocb->ki_flags & IOCB_DIRECT)
> return ext2_dio_write_iter(iocb, from);
>
> - return generic_file_write_iter(iocb, from);
> + return ext2_buffered_write_iter(iocb, from);
> }
>
> const struct file_operations ext2_file_operations = {
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index c4de3a94c4b2..f90d280025d9 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -877,10 +877,14 @@ ext2_iomap_end(struct inode *inode, loff_t offset, loff_t length,
> if ((flags & IOMAP_DIRECT) && (flags & IOMAP_WRITE) && written == 0)
> return -ENOTBLK;
>
> - if (iomap->type == IOMAP_MAPPED &&
> - written < length &&
> - (flags & IOMAP_WRITE))
> + if (iomap->type == IOMAP_MAPPED && written < length &&
> + (flags & IOMAP_WRITE)) {
> ext2_write_failed(inode->i_mapping, offset + length);
> + return 0;
> + }
> +
> + if (iomap->flags & IOMAP_F_SIZE_CHANGED)
> + mark_inode_dirty(inode);
> return 0;
> }
>
> @@ -912,6 +916,16 @@ static void ext2_readahead(struct readahead_control *rac)
> mpage_readahead(rac, ext2_get_block);
> }
>
> +static int ext2_file_read_folio(struct file *file, struct folio *folio)
> +{
> + return iomap_read_folio(folio, &ext2_iomap_ops);
> +}
> +
> +static void ext2_file_readahead(struct readahead_control *rac)
> +{
> + iomap_readahead(rac, &ext2_iomap_ops);
> +}
> +
> static int
> ext2_write_begin(struct file *file, struct address_space *mapping,
> loff_t pos, unsigned len, struct page **pagep, void **fsdata)
> @@ -941,12 +955,41 @@ static sector_t ext2_bmap(struct address_space *mapping, sector_t block)
> return generic_block_bmap(mapping,block,ext2_get_block);
> }
>
> +static sector_t ext2_file_bmap(struct address_space *mapping, sector_t block)
> +{
> + return iomap_bmap(mapping, block, &ext2_iomap_ops);
> +}
> +
> static int
> ext2_writepages(struct address_space *mapping, struct writeback_control *wbc)
> {
> return mpage_writepages(mapping, wbc, ext2_get_block);
> }
>
> +static int ext2_write_map_blocks(struct iomap_writepage_ctx *wpc,
> + struct inode *inode, loff_t offset,
> + unsigned len)
> +{
> + if (offset >= wpc->iomap.offset &&
> + offset < wpc->iomap.offset + wpc->iomap.length)
> + return 0;
> +
> + return ext2_iomap_begin(inode, offset, inode->i_sb->s_blocksize,
> + IOMAP_WRITE, &wpc->iomap, NULL);
> +}
Soooo... this is almost a directio write of the pagecache? ;)
> +
> +static const struct iomap_writeback_ops ext2_writeback_ops = {
> + .map_blocks = ext2_write_map_blocks,
> +};
> +
> +static int ext2_file_writepages(struct address_space *mapping,
> + struct writeback_control *wbc)
> +{
> + struct iomap_writepage_ctx wpc = { };
> +
> + return iomap_writepages(mapping, wbc, &wpc, &ext2_writeback_ops);
> +}
> +
> static int
> ext2_dax_writepages(struct address_space *mapping, struct writeback_control *wbc)
> {
> @@ -955,6 +998,20 @@ ext2_dax_writepages(struct address_space *mapping, struct writeback_control *wbc
> return dax_writeback_mapping_range(mapping, sbi->s_daxdev, wbc);
> }
>
> +const struct address_space_operations ext2_file_aops = {
> + .dirty_folio = iomap_dirty_folio,
> + .release_folio = iomap_release_folio,
trailing space here ^
> + .invalidate_folio = iomap_invalidate_folio,
> + .read_folio = ext2_file_read_folio,
> + .readahead = ext2_file_readahead,
> + .bmap = ext2_file_bmap,
> + .direct_IO = noop_direct_IO,
Nowadays, it is preferred to set FMODE_CAN_ODIRECT and skip setting
->direct_IO. But I see that ext2 hasn't been converted, so this is a
minor point.
> + .writepages = ext2_file_writepages,
> + .migrate_folio = filemap_migrate_folio,
> + .is_partially_uptodate = iomap_is_partially_uptodate,
> + .error_remove_folio = generic_error_remove_folio,
> +};
> +
> const struct address_space_operations ext2_aops = {
I wonder, could directories and symlinks get converted to iomap at some
point? (It's ok if that is not in scope for this series.)
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> .dirty_folio = block_dirty_folio,
> .invalidate_folio = block_invalidate_folio,
> @@ -1279,8 +1336,8 @@ static int ext2_setsize(struct inode *inode, loff_t newsize)
> error = dax_truncate_page(inode, newsize, NULL,
> &ext2_iomap_ops);
> else
> - error = block_truncate_page(inode->i_mapping,
> - newsize, ext2_get_block);
> + error = iomap_truncate_page(inode, newsize, NULL,
> + &ext2_iomap_ops);
> if (error)
> return error;
>
> @@ -1370,7 +1427,7 @@ void ext2_set_file_ops(struct inode *inode)
> if (IS_DAX(inode))
> inode->i_mapping->a_ops = &ext2_dax_aops;
> else
> - inode->i_mapping->a_ops = &ext2_aops;
> + inode->i_mapping->a_ops = &ext2_file_aops;
> }
>
> struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
> --
> 2.44.0
>
>
next prev parent reply other threads:[~2024-04-26 15:29 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-25 13:28 [RFCv3 0/7] ext2 iomap changes and iomap improvements Ritesh Harjani (IBM)
2024-04-25 13:28 ` [RFCv3 1/7] ext2: Remove comment related to journal handle Ritesh Harjani (IBM)
2024-04-26 15:21 ` Darrick J. Wong
2024-04-25 13:28 ` [RFCv3 2/7] ext2: Convert ext2 regular file buffered I/O to use iomap Ritesh Harjani (IBM)
2024-04-26 15:29 ` Darrick J. Wong [this message]
2024-04-25 13:28 ` [RFCv3 3/7] ext2: Enable large folio support Ritesh Harjani (IBM)
2024-04-26 15:30 ` Darrick J. Wong
2024-04-25 13:28 ` [RFCv3 4/7] ext2: Implement seq counter for validating cached iomap Ritesh Harjani (IBM)
2024-04-26 15:41 ` Darrick J. Wong
2024-05-12 13:20 ` Jan Kara
2024-04-25 13:28 ` [RFCv3 5/7] iomap: Fix iomap_adjust_read_range for plen calculation Ritesh Harjani (IBM)
2024-04-26 6:49 ` Christoph Hellwig
2024-04-26 8:52 ` Ritesh Harjani
2024-04-26 15:43 ` Darrick J. Wong
2024-04-25 13:28 ` [RFCv3 6/7] iomap: Optimize iomap_read_folio Ritesh Harjani (IBM)
2024-04-25 13:53 ` Matthew Wilcox
2024-04-26 6:53 ` Christoph Hellwig
2024-04-26 8:50 ` Ritesh Harjani
2024-04-27 4:44 ` Christoph Hellwig
2024-04-25 13:28 ` [RFCv3 7/7] iomap: Optimize data access patterns for filesystems with indirect mappings Ritesh Harjani (IBM)
2024-04-26 16:24 ` Darrick J. Wong
2024-04-26 17:17 ` Ritesh Harjani
2024-04-26 17:25 ` Ritesh Harjani
2024-04-26 17:37 ` Matthew Wilcox
2024-04-26 17:55 ` Ritesh Harjani
2024-04-26 18:13 ` Matthew Wilcox
2024-04-26 18:57 ` Ritesh Harjani
2024-04-26 19:19 ` Matthew Wilcox
2024-04-27 4:54 ` Christoph Hellwig
2024-04-27 6:03 ` Ritesh Harjani
2024-04-27 4:47 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240426152949.GJ360919@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=ojaswin@linux.ibm.com \
--cc=ritesh.list@gmail.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.