xfs
[Top] [All Lists]

Re: [PATCH 09/15] fs: iomap based fiemap implementation

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 09/15] fs: iomap based fiemap implementation
From: Bob Peterson <rpeterso@xxxxxxxxxx>
Date: Mon, 23 May 2016 16:09:26 -0400 (EDT)
Cc: xfs@xxxxxxxxxxx, vishal l verma <vishal.l.verma@xxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1462783638-4968-10-git-send-email-hch@xxxxxx>
References: <1462783638-4968-1-git-send-email-hch@xxxxxx> <1462783638-4968-10-git-send-email-hch@xxxxxx>
Thread-index: 7CzbOvESD2SpIXznwpSx7jvNGFnj4A==
Thread-topic: iomap based fiemap implementation
----- Original Message -----
| Add a simple fiemap implementation based on iomap_ops, partially based
| on a previous implementation from Bob Peterson <rpeterso@xxxxxxxxxx>.
| 
| Signed-off-by: Christoph Hellwig <hch@xxxxxx>
| ---
|  fs/iomap.c            | 90
|  +++++++++++++++++++++++++++++++++++++++++++++++++++
|  include/linux/iomap.h |  3 ++
|  2 files changed, 93 insertions(+)
| 
| diff --git a/fs/iomap.c b/fs/iomap.c
| index f84c6eb..7e639bf 100644
| --- a/fs/iomap.c
| +++ b/fs/iomap.c
| @@ -405,3 +405,93 @@ out_unlock:
|       return ret;
|  }
|  EXPORT_SYMBOL_GPL(iomap_page_mkwrite);
| +
| +struct fiemap_ctx {
| +     struct fiemap_extent_info *fi;
| +     struct iomap prev;
| +};
| +
| +static int iomap_to_fiemap(struct fiemap_extent_info *fi,
| +             struct iomap *iomap, u32 flags)
| +{
| +     switch (iomap->type) {
| +     case IOMAP_HOLE:
| +             /* skip holes */
| +             return 0;
| +     case IOMAP_DELALLOC:
| +             flags |= FIEMAP_EXTENT_DELALLOC | FIEMAP_EXTENT_UNKNOWN;
| +             break;
| +     case IOMAP_UNWRITTEN:
| +             flags |= FIEMAP_EXTENT_UNWRITTEN;
| +             break;
| +     case IOMAP_MAPPED:
| +             break;
| +     }
| +
| +     return fiemap_fill_next_extent(fi, iomap->offset,
| +                     iomap->blkno != IOMAP_NULL_BLOCK ? iomap->blkno << 9: 0,
| +                     iomap->length, flags | FIEMAP_EXTENT_MERGED);
| +
| +}
| +
| +static loff_t
| +iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void
| *data,
| +             struct iomap *iomap)
| +{
| +     struct fiemap_ctx *ctx = data;
| +     loff_t ret = length;
| +
| +     if (iomap->type == IOMAP_HOLE)
| +             return length;
| +
| +     ret = iomap_to_fiemap(ctx->fi, &ctx->prev, 0);
| +     ctx->prev = *iomap;
| +     switch (ret) {
| +     case 0:         /* success */
| +             return length;
| +     case 1:         /* extent array full */
| +             return 0;
| +     default:
| +             return ret;
| +     }
| +}
| +
| +int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
| +             loff_t start, loff_t len, struct iomap_ops *ops)
| +{
| +     struct fiemap_ctx ctx;
| +     loff_t ret;
| +
| +     memset(&ctx, 0, sizeof(ctx));
| +     ctx.fi = fi;
| +     ctx.prev.type = IOMAP_HOLE;
| +
| +     ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC);
| +     if (ret)
| +             return ret;
| +
| +     ret = filemap_write_and_wait(inode->i_mapping);
| +     if (ret)
| +             return ret;
| +
| +     while (len > 0) {
| +             ret = iomap_apply(inode, start, len, 0, ops, &ctx,
| +                             iomap_fiemap_actor);
| +             if (ret < 0)
| +                     return ret;
| +             if (ret == 0)
| +                     break;
| +
| +             start += ret;
| +             len -= ret;
| +     }
| +
| +     if (ctx.prev.type != IOMAP_HOLE) {
| +             ret = iomap_to_fiemap(fi, &ctx.prev, FIEMAP_EXTENT_LAST);
| +             if (ret < 0)
| +                     return ret;
| +     }
| +
| +     return 0;
| +}
| +EXPORT_SYMBOL_GPL(iomap_fiemap);
| diff --git a/include/linux/iomap.h b/include/linux/iomap.h
| index 854766f..b3deee1 100644
| --- a/include/linux/iomap.h
| +++ b/include/linux/iomap.h
| @@ -3,6 +3,7 @@
|  
|  #include <linux/types.h>
|  
| +struct fiemap_extent_info;
|  struct inode;
|  struct iov_iter;
|  struct kiocb;
| @@ -63,5 +64,7 @@ int iomap_truncate_page(struct inode *inode, loff_t pos,
| bool *did_zero,
|               struct iomap_ops *ops);
|  int iomap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
|               struct iomap_ops *ops);
| +int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
| +             loff_t start, loff_t len, struct iomap_ops *ops);
|  
|  #endif /* LINUX_IOMAP_H */
| --
| 2.1.4
| 
| 

Hi Christoph,

I've been looking at this again. Where are the calls to the fs-specific bits
for fiemap? It looks like iomap_fiemap calls iomap_apply, which calls
iomap_fiemap_actor, but that doesn't call any ops->iomap_get_iomap or similar.
It calls the iomap_begin (which BTW has a comment that says "Execute a iomap
write" which is probably wrong and should be more generic, as for cases like
fiemap) and it calls iomap_end. But it never calls an fs-specific actor
anywhere. Am I missing something? My earlier version passed in the actor
function, as per Dave Chinner's request, but yours doesn't.

Regards,

Bob Peterson
Red Hat File Systems

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