xfs: simplify the xfs_getbmap interface

Instead of passing in a formatter callback allocate the bmap buffer
in the caller and process the entries there.  Additionally replace
the in-kernel buffer with a new much smaller structure, and unify
the implementation of the different ioctls in a single function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 2564b8b..0543423 100644 (file)
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -409,11 +409,11 @@ static int
 xfs_getbmap_report_one(
        struct xfs_inode        *ip,
        struct getbmapx         *bmv,
-       struct getbmapx         *out,
+       struct kgetbmap         *out,
        int64_t                 bmv_end,
        struct xfs_bmbt_irec    *got)
 {
-       struct getbmapx         *p = out + bmv->bmv_entries;
+       struct kgetbmap         *p = out + bmv->bmv_entries;
        bool                    shared = false, trimmed = false;
        int                     error;
 
@@ -460,12 +460,12 @@ static void
 xfs_getbmap_report_hole(
        struct xfs_inode        *ip,
        struct getbmapx         *bmv,
-       struct getbmapx         *out,
+       struct kgetbmap         *out,
        int64_t                 bmv_end,
        xfs_fileoff_t           bno,
        xfs_fileoff_t           end)
 {
-       struct getbmapx         *p = out + bmv->bmv_entries;
+       struct kgetbmap         *p = out + bmv->bmv_entries;
 
        if (bmv->bmv_iflags & BMV_IF_NO_HOLES)
                return;
@@ -513,47 +513,36 @@ xfs_getbmap_next_rec(
  */
 int                                            /* error code */
 xfs_getbmap(
-       xfs_inode_t             *ip,
+       struct xfs_inode        *ip,
        struct getbmapx         *bmv,           /* user bmap structure */
-       xfs_bmap_format_t       formatter,      /* format to user */
-       void                    *arg)           /* formatter arg */
+       struct kgetbmap         *out)
 {
        struct xfs_mount        *mp = ip->i_mount;
        int                     iflags = bmv->bmv_iflags;
-       int                     whichfork, lock, i, error = 0;
+       int                     whichfork, lock, error = 0;
        int64_t                 bmv_end, max_len;
        xfs_fileoff_t           bno, first_bno;
        struct xfs_ifork        *ifp;
-       struct getbmapx         *out;
        struct xfs_bmbt_irec    got, rec;
        xfs_filblks_t           len;
        xfs_extnum_t            idx;
 
+       if (bmv->bmv_iflags & ~BMV_IF_VALID)
+               return -EINVAL;
 #ifndef DEBUG
        /* Only allow CoW fork queries if we're debugging. */
        if (iflags & BMV_IF_COWFORK)
                return -EINVAL;
 #endif
-
        if ((iflags & BMV_IF_ATTRFORK) && (iflags & BMV_IF_COWFORK))
                return -EINVAL;
 
-       if (bmv->bmv_count <= 1)
-               return -EINVAL;
-       if (bmv->bmv_count > ULONG_MAX / sizeof(struct getbmapx))
-               return -ENOMEM;
-
        if (bmv->bmv_length < -1)
                return -EINVAL;
-
        bmv->bmv_entries = 0;
        if (bmv->bmv_length == 0)
                return 0;
 
-       out = kmem_zalloc_large(bmv->bmv_count * sizeof(struct getbmapx), 0);
-       if (!out)
-               return -ENOMEM;
-
        if (iflags & BMV_IF_ATTRFORK)
                whichfork = XFS_ATTR_FORK;
        else if (iflags & BMV_IF_COWFORK)
@@ -700,15 +689,6 @@ out_unlock_ilock:
        xfs_iunlock(ip, lock);
 out_unlock_iolock:
        xfs_iunlock(ip, XFS_IOLOCK_SHARED);
-
-       for (i = 0; i < bmv->bmv_entries; i++) {
-               /* format results & advance arg */
-               error = formatter(&arg, &out[i]);
-               if (error)
-                       break;
-       }
-
-       kmem_free(out);
        return error;
 }
 

diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 7d330b3..4d4ae48 100644 (file)
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -47,10 +47,14 @@ int xfs_bmap_eof(struct xfs_inode *ip, xfs_fileoff_t endoff,
 int    xfs_bmap_punch_delalloc_range(struct xfs_inode *ip,
                xfs_fileoff_t start_fsb, xfs_fileoff_t length);
 
-/* bmap to userspace formatter - copy to user & advance pointer */
-typedef int (*xfs_bmap_format_t)(void **, struct getbmapx *);
+struct kgetbmap {
+       __s64           bmv_offset;     /* file offset of segment in blocks */
+       __s64           bmv_block;      /* starting block (64-bit daddr_t)  */
+       __s64           bmv_length;     /* length of segment, blocks        */
+       __s32           bmv_oflags;     /* output flags */
+};
 int    xfs_getbmap(struct xfs_inode *ip, struct getbmapx *bmv,
-               xfs_bmap_format_t formatter, void *arg);
+               struct kgetbmap *out);
 
 /* functions in xfs_bmap.c that are only needed by xfs_bmap_util.c */
 int    xfs_bmap_extsize_align(struct xfs_mount *mp, struct xfs_bmbt_irec *gotp,

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index aa75389..b01a198 100644 (file)
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1540,17 +1540,26 @@ out_drop_write:
        return error;
 }
 
-STATIC int
-xfs_getbmap_format(void **ap, struct getbmapx *bmv)
+static bool
+xfs_getbmap_format(
+       struct kgetbmap         *p,
+       struct getbmapx __user  *u,
+       size_t                  recsize)
 {
-       struct getbmap __user   *base = (struct getbmap __user *)*ap;
-
-       /* copy only getbmap portion (not getbmapx) */
-       if (copy_to_user(base, bmv, sizeof(struct getbmap)))
-               return -EFAULT;
-
-       *ap += sizeof(struct getbmap);
-       return 0;
+       if (put_user(p->bmv_offset, &u->bmv_offset) ||
+           put_user(p->bmv_block, &u->bmv_block) ||
+           put_user(p->bmv_length, &u->bmv_length) ||
+           put_user(0, &u->bmv_count) ||
+           put_user(0, &u->bmv_entries))
+               return false;
+       if (recsize < sizeof(struct getbmapx))
+               return true;
+       if (put_user(0, &u->bmv_iflags) ||
+           put_user(p->bmv_oflags, &u->bmv_oflags) ||
+           put_user(0, &u->bmv_unused1) ||
+           put_user(0, &u->bmv_unused2))
+               return false;
+       return true;
 }
 
 STATIC int
@@ -1560,68 +1569,57 @@ xfs_ioc_getbmap(
        void                    __user *arg)
 {
        struct getbmapx         bmx = { 0 };
-       int                     error;
+       struct kgetbmap         *buf;
+       size_t                  recsize;
+       int                     error, i;
 
-       /* struct getbmap is a strict subset of struct getbmapx. */
-       if (copy_from_user(&bmx, arg, offsetof(struct getbmapx, bmv_iflags)))
-               return -EFAULT;
-
-       if (bmx.bmv_count < 2)
+       switch (cmd) {
+       case XFS_IOC_GETBMAPA:
+               bmx.bmv_iflags = BMV_IF_ATTRFORK;
+               /*FALLTHRU*/
+       case XFS_IOC_GETBMAP:
+               if (file->f_mode & FMODE_NOCMTIME)
+                       bmx.bmv_iflags |= BMV_IF_NO_DMAPI_READ;
+               /* struct getbmap is a strict subset of struct getbmapx. */
+               recsize = sizeof(struct getbmap);
+               break;
+       case XFS_IOC_GETBMAPX:
+               recsize = sizeof(struct getbmapx);
+               break;
+       default:
                return -EINVAL;
+       }
 
-       bmx.bmv_iflags = (cmd == XFS_IOC_GETBMAPA ? BMV_IF_ATTRFORK : 0);
-       if (file->f_mode & FMODE_NOCMTIME)
-               bmx.bmv_iflags |= BMV_IF_NO_DMAPI_READ;
-
-       error = xfs_getbmap(XFS_I(file_inode(file)), &bmx, xfs_getbmap_format,
-                           (__force struct getbmap *)arg+1);
-       if (error)
-               return error;
-
-       /* copy back header - only size of getbmap */
-       if (copy_to_user(arg, &bmx, sizeof(struct getbmap)))
-               return -EFAULT;
-       return 0;
-}
-
-STATIC int
-xfs_getbmapx_format(void **ap, struct getbmapx *bmv)
-{
-       struct getbmapx __user  *base = (struct getbmapx __user *)*ap;
-
-       if (copy_to_user(base, bmv, sizeof(struct getbmapx)))
-               return -EFAULT;
-
-       *ap += sizeof(struct getbmapx);
-       return 0;
-}
-
-STATIC int
-xfs_ioc_getbmapx(
-       struct xfs_inode        *ip,
-       void                    __user *arg)
-{
-       struct getbmapx         bmx;
-       int                     error;
-
-       if (copy_from_user(&bmx, arg, sizeof(bmx)))
+       if (copy_from_user(&bmx, arg, recsize))
                return -EFAULT;
 
        if (bmx.bmv_count < 2)
                return -EINVAL;
+       if (bmx.bmv_count > ULONG_MAX / recsize)
+               return -ENOMEM;
 
-       if (bmx.bmv_iflags & (~BMV_IF_VALID))
-               return -EINVAL;
+       buf = kmem_zalloc_large(bmx.bmv_count * sizeof(*buf), 0);
+       if (!buf)
+               return -ENOMEM;
 
-       error = xfs_getbmap(ip, &bmx, xfs_getbmapx_format,
-                           (__force struct getbmapx *)arg+1);
+       error = xfs_getbmap(XFS_I(file_inode(file)), &bmx, buf);
        if (error)
-               return error;
+               goto out_free_buf;
 
-       /* copy back header */
-       if (copy_to_user(arg, &bmx, sizeof(struct getbmapx)))
-               return -EFAULT;
+       error = -EFAULT;
+       if (copy_to_user(arg, &bmx, recsize))
+               goto out_free_buf;
+       arg += recsize;
+
+       for (i = 0; i < bmx.bmv_entries; i++) {
+               if (!xfs_getbmap_format(buf + i, arg, recsize))
+                       goto out_free_buf;
+               arg += recsize;
+       }
 
+       error = 0;
+out_free_buf:
+       kmem_free(buf);
        return 0;
 }
 
@@ -1878,10 +1876,8 @@ xfs_file_ioctl(
 
        case XFS_IOC_GETBMAP:
        case XFS_IOC_GETBMAPA:
-               return xfs_ioc_getbmap(filp, cmd, arg);
-
        case XFS_IOC_GETBMAPX:
-               return xfs_ioc_getbmapx(ip, arg);
+               return xfs_ioc_getbmap(filp, cmd, arg);
 
        case FS_IOC_GETFSMAP:
                return xfs_ioc_getfsmap(ip, arg);