On Tue, Nov 9, 2021 at 12:34 AM Christoph Hellwig <hch@xxxxxx> wrote:
>
> Remove the last user of ->bdev in dax.c by requiring the file system to
> pass in an address that already includes the DAX offset. As part of the
> only set ->bdev or ->daxdev when actually required in the ->iomap_begin
> methods.
Changes look good except for what looks like an argument position
fixup needed for an xfs_bmbt_to_iomap() caller below...
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/dax.c | 6 +-----
> fs/erofs/data.c | 11 ++++++++--
> fs/erofs/internal.h | 1 +
> fs/ext2/inode.c | 8 +++++--
> fs/ext4/inode.c | 16 +++++++++-----
> fs/xfs/libxfs/xfs_bmap.c | 4 ++--
> fs/xfs/xfs_aops.c | 2 +-
> fs/xfs/xfs_iomap.c | 45 +++++++++++++++++++++++++---------------
> fs/xfs/xfs_iomap.h | 5 +++--
> fs/xfs/xfs_pnfs.c | 2 +-
> 10 files changed, 63 insertions(+), 37 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index 0bd6cdcbacfc4..2c13c681edf09 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -711,11 +711,7 @@ int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
>
> static pgoff_t dax_iomap_pgoff(const struct iomap *iomap, loff_t pos)
> {
> - phys_addr_t paddr = iomap->addr + (pos & PAGE_MASK) - iomap->offset;
> -
> - if (iomap->bdev)
> - paddr += (get_start_sect(iomap->bdev) << SECTOR_SHIFT);
> - return PHYS_PFN(paddr);
> + return PHYS_PFN(iomap->addr + (pos & PAGE_MASK) - iomap->offset);
> }
>
> static int copy_cow_page_dax(struct vm_fault *vmf, const struct iomap_iter *iter)
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 0e35ef3f9f3d7..9b1bb177ce303 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
[..]
}
> @@ -215,9 +218,13 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> if (ret)
> return ret;
>
> - iomap->bdev = mdev.m_bdev;
> - iomap->dax_dev = mdev.m_daxdev;
> iomap->offset = map.m_la;
> + if (flags & IOMAP_DAX) {
> + iomap->dax_dev = mdev.m_daxdev;
> + iomap->offset += mdev.m_dax_part_off;
> + } else {
> + iomap->bdev = mdev.m_bdev;
> + }
Ah, that's what IOMAP_DAX is for, to stop making iomap carry bdev
details unnecessarily.
[..]
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 704292c6ce0c7..74dbf1fd99d39 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -54,7 +54,8 @@ xfs_bmbt_to_iomap(
> struct xfs_inode *ip,
> struct iomap *iomap,
> struct xfs_bmbt_irec *imap,
> - u16 flags)
> + unsigned int flags,
> + u16 iomap_flags)
It would be nice if the compiler could help with making sure that
right 'flags' values are passed to the right 'flags' parameter, but I
can't think of
[..]
> @@ -1053,23 +1061,24 @@ xfs_buffered_write_iomap_begin(
> */
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> trace_xfs_iomap_alloc(ip, offset, count, allocfork, &imap);
> - return xfs_bmbt_to_iomap(ip, iomap, &imap, IOMAP_F_NEW);
> + return xfs_bmbt_to_iomap(ip, iomap, &imap, flags, IOMAP_F_NEW);
>
> found_imap:
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> - return xfs_bmbt_to_iomap(ip, iomap, &imap, 0);
> + return xfs_bmbt_to_iomap(ip, iomap, &imap, flags, 0);
The iomap flags are supposed to be the last argument, right?
