xfs
[Top] [All Lists]

Re: [PATCH 5/8] xfs: implement iomap based buffered write path

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 5/8] xfs: implement iomap based buffered write path
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 14 Apr 2016 08:58:14 -0400
Cc: xfs@xxxxxxxxxxx, rpeterso@xxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1460494382-14547-6-git-send-email-hch@xxxxxx>
References: <1460494382-14547-1-git-send-email-hch@xxxxxx> <1460494382-14547-6-git-send-email-hch@xxxxxx>
User-agent: Mutt/1.5.24 (2015-08-30)
On Tue, Apr 12, 2016 at 01:52:59PM -0700, Christoph Hellwig wrote:
> Convert XFS to use the new iomap based multipage write path. This involves
> implementing the ->iomap_begin and ->iomap_end methods, and switching the
> buffered file write, page_mkwrite and xfs_iozero paths to the new iomap
> helpers.
> 
> With this change __xfs_get_blocks will never be used for buffered writes,
> and the code handling them can be removed.
> 
> Based on earlier code from Dave Chinner.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>  fs/xfs/Kconfig     |   1 +
>  fs/xfs/xfs_aops.c  | 212 
> -----------------------------------------------------
>  fs/xfs/xfs_file.c  |  71 ++++++++----------
>  fs/xfs/xfs_iomap.c | 144 ++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_iomap.h |   5 +-
>  fs/xfs/xfs_iops.c  |   9 ++-
>  fs/xfs/xfs_trace.h |   3 +
>  7 files changed, 187 insertions(+), 258 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 2f37194..73de1ec 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -967,3 +967,147 @@ xfs_bmbt_to_iomap(
...
> +static int
> +xfs_file_iomap_end_delalloc(
> +     struct xfs_inode        *ip,
> +     loff_t                  offset,
> +     loff_t                  length,
> +     ssize_t                 written)
> +{
> +     struct xfs_mount        *mp = ip->i_mount;
> +     xfs_fileoff_t           start_fsb;
> +     xfs_fileoff_t           end_fsb;
> +     int                     error = 0;
> +
> +     start_fsb = XFS_B_TO_FSB(mp, offset + written);
> +     end_fsb = XFS_B_TO_FSB(mp, offset + length - written);
> +

Just skimming over this series... but shouldn't this be offset + length?
Why walk back from the end of the allocated range?

Brian

> +     /*
> +      * Trim back delalloc blocks if we didn't manage to write the whole
> +      * range reserved.
> +      *
> +      * We don't need to care about racing delalloc as we hold i_mutex
> +      * across the reserve/allocate/unreserve calls. If there are delalloc
> +      * blocks in the range, they are ours.
> +      */
> +     if (start_fsb < end_fsb) {
> +             xfs_ilock(ip, XFS_ILOCK_EXCL);
> +             error = xfs_bmap_punch_delalloc_range(ip, start_fsb,
> +                                            end_fsb - start_fsb);
> +             xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> +             if (error && !XFS_FORCED_SHUTDOWN(mp)) {
> +                     xfs_alert(mp, "%s: unable to clean up ino %lld",
> +                             __func__, ip->i_ino);
> +                     return error;
> +             }
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +xfs_file_iomap_end(
> +     struct inode            *inode,
> +     loff_t                  offset,
> +     loff_t                  length,
> +     ssize_t                 written,
> +     unsigned                flags,
> +     struct iomap            *iomap)
> +{
> +     if ((flags & IOMAP_WRITE) && iomap->type == IOMAP_DELALLOC)
> +             return xfs_file_iomap_end_delalloc(XFS_I(inode), offset,
> +                             length, written);
> +     return 0;
> +}
> +
> +struct iomap_ops xfs_iomap_ops = {
> +     .iomap_begin            = xfs_file_iomap_begin,
> +     .iomap_end              = xfs_file_iomap_end,
> +};
> diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
> index 718f07c..e066d04 100644
> --- a/fs/xfs/xfs_iomap.h
> +++ b/fs/xfs/xfs_iomap.h
> @@ -18,7 +18,8 @@
>  #ifndef __XFS_IOMAP_H__
>  #define __XFS_IOMAP_H__
>  
> -struct iomap;
> +#include <linux/iomap.h>
> +
>  struct xfs_inode;
>  struct xfs_bmbt_irec;
>  
> @@ -33,4 +34,6 @@ int xfs_iomap_write_unwritten(struct xfs_inode *, 
> xfs_off_t, xfs_off_t);
>  void xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
>               struct xfs_bmbt_irec *);
>  
> +extern struct iomap_ops xfs_iomap_ops;
> +
>  #endif /* __XFS_IOMAP_H__*/
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 1e2086d..6dfa10c 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -38,6 +38,7 @@
>  #include "xfs_dir2.h"
>  #include "xfs_trans_space.h"
>  #include "xfs_pnfs.h"
> +#include "xfs_iomap.h"
>  
>  #include <linux/capability.h>
>  #include <linux/xattr.h>
> @@ -822,8 +823,8 @@ xfs_setattr_size(
>                       error = dax_truncate_page(inode, newsize,
>                                       xfs_get_blocks_direct);
>               } else {
> -                     error = block_truncate_page(inode->i_mapping, newsize,
> -                                     xfs_get_blocks);
> +                     error = iomap_truncate_page(inode, newsize,
> +                                     &did_zeroing, &xfs_iomap_ops);
>               }
>       }
>  
> @@ -838,8 +839,8 @@ xfs_setattr_size(
>        * problem. Note that this includes any block zeroing we did above;
>        * otherwise those blocks may not be zeroed after a crash.
>        */
> -     if (newsize > ip->i_d.di_size &&
> -         (oldsize != ip->i_d.di_size || did_zeroing)) {
> +     if (did_zeroing ||
> +         (newsize > ip->i_d.di_size && oldsize != ip->i_d.di_size)) {
>               error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
>                                                     ip->i_d.di_size, newsize);
>               if (error)
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 840d52e..86fb345 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1296,6 +1296,9 @@ DEFINE_IOMAP_EVENT(xfs_map_blocks_alloc);
>  DEFINE_IOMAP_EVENT(xfs_get_blocks_found);
>  DEFINE_IOMAP_EVENT(xfs_get_blocks_alloc);
>  DEFINE_IOMAP_EVENT(xfs_get_blocks_map_direct);
> +DEFINE_IOMAP_EVENT(xfs_iomap_alloc);
> +DEFINE_IOMAP_EVENT(xfs_iomap_found);
> +DEFINE_IOMAP_EVENT(xfs_iomap_not_found);
>  
>  DECLARE_EVENT_CLASS(xfs_simple_io_class,
>       TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count),
> -- 
> 2.1.4
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>