Christoph Hellwig <hch@xxxxxx> writes:
> The calculation in iomap_sector is pretty trivial and most of the time
> iomap_add_to_ioend only callers either iomap_can_add_to_ioend or
> iomap_alloc_ioend from a single invocation.
>
> Calculate the sector in the two lower level functions and stop passing it
> from iomap_add_to_ioend and update the iomap_alloc_ioend argument passing
> order to match that of iomap_add_to_ioend.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/iomap/buffered-io.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
Straight forward change. Looks good to me, please feel free to add -
(small nit below on naming style convention)
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 7f86d2f90e3863..329e2c342f1c64 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1666,9 +1666,8 @@ iomap_submit_ioend(struct iomap_writepage_ctx *wpc, struct iomap_ioend *ioend,
> return 0;
> }
>
> -static struct iomap_ioend *
> -iomap_alloc_ioend(struct inode *inode, struct iomap_writepage_ctx *wpc,
> - loff_t offset, sector_t sector, struct writeback_control *wbc)
> +static struct iomap_ioend *iomap_alloc_ioend(struct iomap_writepage_ctx *wpc,
> + struct writeback_control *wbc, struct inode *inode, loff_t pos)
> {
> struct iomap_ioend *ioend;
> struct bio *bio;
> @@ -1676,7 +1675,7 @@ iomap_alloc_ioend(struct inode *inode, struct iomap_writepage_ctx *wpc,
> bio = bio_alloc_bioset(wpc->iomap.bdev, BIO_MAX_VECS,
> REQ_OP_WRITE | wbc_to_write_flags(wbc),
> GFP_NOFS, &iomap_ioend_bioset);
> - bio->bi_iter.bi_sector = sector;
> + bio->bi_iter.bi_sector = iomap_sector(&wpc->iomap, pos);
> wbc_init_bio(wbc, bio);
>
> ioend = container_of(bio, struct iomap_ioend, io_inline_bio);
> @@ -1685,9 +1684,9 @@ iomap_alloc_ioend(struct inode *inode, struct iomap_writepage_ctx *wpc,
> ioend->io_flags = wpc->iomap.flags;
> ioend->io_inode = inode;
> ioend->io_size = 0;
> - ioend->io_offset = offset;
> + ioend->io_offset = pos;
> ioend->io_bio = bio;
> - ioend->io_sector = sector;
> + ioend->io_sector = bio->bi_iter.bi_sector;
>
> wpc->nr_folios = 0;
> return ioend;
> @@ -1716,8 +1715,7 @@ iomap_chain_bio(struct bio *prev)
> }
>
> static bool
> -iomap_can_add_to_ioend(struct iomap_writepage_ctx *wpc, loff_t offset,
> - sector_t sector)
> +iomap_can_add_to_ioend(struct iomap_writepage_ctx *wpc, loff_t offset)
Not sure which style you would like to keep in fs/iomap/.
Should the function name be in the same line as "static bool" or in the next line?
For previous function you made the function name definition in the same
line. Or is the naming style irrelevant for fs/iomap/?
-ritesh
