On Fri, Jun 23, 2017 at 03:34:39PM +0200, Andreas Gruenbacher wrote:
> Filesystems can use this for implementing lseek SEEK_HOLE / SEEK_DATA
> support via iomap.
> 
> Signed-off-by: Andreas Gruenbacher <[email protected]>
> ---
>  fs/iomap.c            | 89 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iomap.h |  3 ++
>  2 files changed, 92 insertions(+)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 4b10892..781f0a0 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -584,6 +584,95 @@ int iomap_fiemap(struct inode *inode, struct 
> fiemap_extent_info *fi,
>  }
>  EXPORT_SYMBOL_GPL(iomap_fiemap);
>  
> +static loff_t
> +iomap_seek_hole_actor(struct inode *inode, loff_t offset, loff_t length,
> +                   void *data, struct iomap *iomap)
> +{
> +     if (iomap->type == IOMAP_HOLE)
> +             goto found;
> +     length = iomap->offset + iomap->length - offset;

What is the problem with the passed in length value?

> +     if (iomap->type != IOMAP_UNWRITTEN)
> +             return length;
> +     offset = page_cache_seek_hole_data(inode, offset, length, SEEK_HOLE);
> +     if (offset < 0)
> +             return length;
> +found:
> +     *(loff_t *)data = offset;
> +     return 0;

What about using a switch statement?

        switch (iomap->type) {
        case IOMAP_UNWRITTEN:
                offset = page_cache_seek_hole_data(inode, offset, length,
                                SEEK_HOLE);
                if (offset < 0)
                        return length;
                /*FALLTHRU*/
        case IOMAP_HOLE:
                *(loff_t *)data = offset;
                return 0;
        default:
                return length;
        }

> +static loff_t
> +iomap_seek_data_actor(struct inode *inode, loff_t offset, loff_t length,
> +                   void *data, struct iomap *iomap)
> +{
> +     if (iomap->type != IOMAP_HOLE && iomap->type != IOMAP_UNWRITTEN)
> +             goto found;
> +     length = iomap->offset + iomap->length - offset;
> +     if (iomap->type != IOMAP_UNWRITTEN)
> +             return length;
> +     offset = page_cache_seek_hole_data(inode, offset, length, SEEK_DATA);
> +     if (offset < 0)
> +             return length;
> +found:
> +     *(loff_t *)data = offset;
> +     return 0;

> +loff_t
> +iomap_seek_hole_data(struct inode *inode, loff_t offset,
> +                  int whence, const struct iomap_ops *ops)
> +{
> +     static loff_t (*actor)(struct inode *, loff_t, loff_t, void *,
> +                            struct iomap *);

I wonder (but I'm not sure) if it would be simpler and easier to
understand to just have to different functions for SEEK_HOLE
vs SEEK_DATA here.

Reply via email to