On Mon, May 08, 2023 at 12:57:57AM +0530, Ritesh Harjani (IBM) wrote:
> This patch moves up and combine the definitions of two functions
> (iomap_iop_set_range_uptodate() & iomap_set_range_uptodate()) into
> iop_set_range_uptodate() & refactors it's arguments a bit.
>
> No functionality change in this patch.
>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> ---
Hi Ritesh,
I just have a few random and nitty comments/questions on the series..
> fs/iomap/buffered-io.c | 57 ++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index cbd945d96584..e732581dc2d4 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -43,6 +43,27 @@ static inline struct iomap_page *to_iomap_page(struct folio *folio)
>
> static struct bio_set iomap_ioend_bioset;
>
> +static void iop_set_range_uptodate(struct inode *inode, struct folio *folio,
> + size_t off, size_t len)
> +{
Any particular reason this now takes the inode as a param now instead of
continuing to use the folio?
Brian
> + struct iomap_page *iop = to_iomap_page(folio);
> + unsigned int first_blk = off >> inode->i_blkbits;
> + unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
> + unsigned int nr_blks = last_blk - first_blk + 1;
> + unsigned long flags;
> +
> + if (iop) {
> + spin_lock_irqsave(&iop->uptodate_lock, flags);
> + bitmap_set(iop->uptodate, first_blk, nr_blks);
> + if (bitmap_full(iop->uptodate,
> + i_blocks_per_folio(inode, folio)))
> + folio_mark_uptodate(folio);
> + spin_unlock_irqrestore(&iop->uptodate_lock, flags);
> + } else {
> + folio_mark_uptodate(folio);
> + }
> +}
> +
> static struct iomap_page *iop_alloc(struct inode *inode, struct folio *folio,
> unsigned int flags)
> {
> @@ -145,30 +166,6 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
> *lenp = plen;
> }
>
> -static void iomap_iop_set_range_uptodate(struct folio *folio,
> - struct iomap_page *iop, size_t off, size_t len)
> -{
> - struct inode *inode = folio->mapping->host;
> - unsigned first = off >> inode->i_blkbits;
> - unsigned last = (off + len - 1) >> inode->i_blkbits;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&iop->uptodate_lock, flags);
> - bitmap_set(iop->uptodate, first, last - first + 1);
> - if (bitmap_full(iop->uptodate, i_blocks_per_folio(inode, folio)))
> - folio_mark_uptodate(folio);
> - spin_unlock_irqrestore(&iop->uptodate_lock, flags);
> -}
> -
> -static void iomap_set_range_uptodate(struct folio *folio,
> - struct iomap_page *iop, size_t off, size_t len)
> -{
> - if (iop)
> - iomap_iop_set_range_uptodate(folio, iop, off, len);
> - else
> - folio_mark_uptodate(folio);
> -}
> -
> static void iomap_finish_folio_read(struct folio *folio, size_t offset,
> size_t len, int error)
> {
> @@ -178,7 +175,8 @@ static void iomap_finish_folio_read(struct folio *folio, size_t offset,
> folio_clear_uptodate(folio);
> folio_set_error(folio);
> } else {
> - iomap_set_range_uptodate(folio, iop, offset, len);
> + iop_set_range_uptodate(folio->mapping->host, folio, offset,
> + len);
> }
>
> if (!iop || atomic_sub_and_test(len, &iop->read_bytes_pending))
> @@ -240,7 +238,7 @@ static int iomap_read_inline_data(const struct iomap_iter *iter,
> memcpy(addr, iomap->inline_data, size);
> memset(addr + size, 0, PAGE_SIZE - poff - size);
> kunmap_local(addr);
> - iomap_set_range_uptodate(folio, iop, offset, PAGE_SIZE - poff);
> + iop_set_range_uptodate(iter->inode, folio, offset, PAGE_SIZE - poff);
> return 0;
> }
>
> @@ -277,7 +275,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
>
> if (iomap_block_needs_zeroing(iter, pos)) {
> folio_zero_range(folio, poff, plen);
> - iomap_set_range_uptodate(folio, iop, poff, plen);
> + iop_set_range_uptodate(iter->inode, folio, poff, plen);
> goto done;
> }
>
> @@ -598,7 +596,7 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
> if (status)
> return status;
> }
> - iomap_set_range_uptodate(folio, iop, poff, plen);
> + iop_set_range_uptodate(iter->inode, folio, poff, plen);
> } while ((block_start += plen) < block_end);
>
> return 0;
> @@ -705,7 +703,6 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos,
> static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
> size_t copied, struct folio *folio)
> {
> - struct iomap_page *iop = to_iomap_page(folio);
> flush_dcache_folio(folio);
>
> /*
> @@ -721,7 +718,7 @@ static size_t __iomap_write_end(struct inode *inode, loff_t pos, size_t len,
> */
> if (unlikely(copied < len && !folio_test_uptodate(folio)))
> return 0;
> - iomap_set_range_uptodate(folio, iop, offset_in_folio(folio, pos), len);
> + iop_set_range_uptodate(inode, folio, offset_in_folio(folio, pos), len);
> filemap_dirty_folio(inode->i_mapping, folio);
> return copied;
> }
> --
> 2.39.2
>
