* [PATCH 1/9] xfs: remove various bulk request typedef usage
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
@ 2019-05-29 22:27 ` Darrick J. Wong
2019-06-05 22:29 ` Allison Collins
2019-05-29 22:27 ` [PATCH 2/9] xfs: rename bulkstat functions Darrick J. Wong
` (7 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:27 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove xfs_bstat_t, xfs_fsop_bulkreq_t, xfs_inogrp_t, and similarly
named compat typedefs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 16 ++++++++--------
fs/xfs/xfs_ioctl.c | 2 +-
fs/xfs/xfs_ioctl32.c | 11 +++++++----
fs/xfs/xfs_ioctl32.h | 14 +++++++-------
4 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index e7382c780ed7..ef0dce229fa4 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -97,7 +97,7 @@ struct getbmapx {
* For use by backup and restore programs to set the XFS on-disk inode
* fields di_dmevmask and di_dmstate. These must be set to exactly and
* only values previously obtained via xfs_bulkstat! (Specifically the
- * xfs_bstat_t fields bs_dmevmask and bs_dmstate.)
+ * struct xfs_bstat fields bs_dmevmask and bs_dmstate.)
*/
#ifndef HAVE_FSDMIDATA
struct fsdmidata {
@@ -328,7 +328,7 @@ typedef struct xfs_bstime {
__s32 tv_nsec; /* and nanoseconds */
} xfs_bstime_t;
-typedef struct xfs_bstat {
+struct xfs_bstat {
__u64 bs_ino; /* inode number */
__u16 bs_mode; /* type and mode */
__u16 bs_nlink; /* number of links */
@@ -356,7 +356,7 @@ typedef struct xfs_bstat {
__u32 bs_dmevmask; /* DMIG event mask */
__u16 bs_dmstate; /* DMIG state info */
__u16 bs_aextents; /* attribute number of extents */
-} xfs_bstat_t;
+};
/* bs_sick flags */
#define XFS_BS_SICK_INODE (1 << 0) /* inode core */
@@ -382,22 +382,22 @@ bstat_get_projid(struct xfs_bstat *bs)
/*
* The user-level BulkStat Request interface structure.
*/
-typedef struct xfs_fsop_bulkreq {
+struct xfs_fsop_bulkreq {
__u64 __user *lastip; /* last inode # pointer */
__s32 icount; /* count of entries in buffer */
void __user *ubuffer;/* user buffer for inode desc. */
__s32 __user *ocount; /* output count pointer */
-} xfs_fsop_bulkreq_t;
+};
/*
* Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
*/
-typedef struct xfs_inogrp {
+struct xfs_inogrp {
__u64 xi_startino; /* starting inode number */
__s32 xi_alloccount; /* # bits set in allocmask */
__u64 xi_allocmask; /* mask of allocated inodes */
-} xfs_inogrp_t;
+};
/*
@@ -529,7 +529,7 @@ typedef struct xfs_swapext
xfs_off_t sx_offset; /* offset into file */
xfs_off_t sx_length; /* leng from offset */
char sx_pad[16]; /* pad space, unused */
- xfs_bstat_t sx_stat; /* stat of target b4 copy */
+ struct xfs_bstat sx_stat; /* stat of target b4 copy */
} xfs_swapext_t;
/*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 4fa9a2c8b029..456a0e132d6d 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -764,7 +764,7 @@ xfs_ioc_bulkstat(
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
- if (copy_from_user(&bulkreq, arg, sizeof(xfs_fsop_bulkreq_t)))
+ if (copy_from_user(&bulkreq, arg, sizeof(struct xfs_fsop_bulkreq)))
return -EFAULT;
if (copy_from_user(&lastino, bulkreq.lastip, sizeof(__s64)))
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index dd53a9692e68..8dcb7046ed15 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -116,11 +116,14 @@ xfs_ioctl32_bstime_copyin(
return 0;
}
-/* xfs_bstat_t has differing alignment on intel, & bstime_t sizes everywhere */
+/*
+ * struct xfs_bstat has differing alignment on intel, & bstime_t sizes
+ * everywhere
+ */
STATIC int
xfs_ioctl32_bstat_copyin(
- xfs_bstat_t *bstat,
- compat_xfs_bstat_t __user *bstat32)
+ struct xfs_bstat *bstat,
+ struct compat_xfs_bstat __user *bstat32)
{
if (get_user(bstat->bs_ino, &bstat32->bs_ino) ||
get_user(bstat->bs_mode, &bstat32->bs_mode) ||
@@ -204,7 +207,7 @@ STATIC int
xfs_compat_ioc_bulkstat(
xfs_mount_t *mp,
unsigned int cmd,
- compat_xfs_fsop_bulkreq_t __user *p32)
+ struct compat_xfs_fsop_bulkreq __user *p32)
{
u32 addr;
struct xfs_fsop_bulkreq bulkreq;
diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h
index d28fa824284a..7985344d3aa6 100644
--- a/fs/xfs/xfs_ioctl32.h
+++ b/fs/xfs/xfs_ioctl32.h
@@ -36,7 +36,7 @@ typedef struct compat_xfs_bstime {
__s32 tv_nsec; /* and nanoseconds */
} compat_xfs_bstime_t;
-typedef struct compat_xfs_bstat {
+struct compat_xfs_bstat {
__u64 bs_ino; /* inode number */
__u16 bs_mode; /* type and mode */
__u16 bs_nlink; /* number of links */
@@ -61,14 +61,14 @@ typedef struct compat_xfs_bstat {
__u32 bs_dmevmask; /* DMIG event mask */
__u16 bs_dmstate; /* DMIG state info */
__u16 bs_aextents; /* attribute number of extents */
-} __compat_packed compat_xfs_bstat_t;
+} __compat_packed;
-typedef struct compat_xfs_fsop_bulkreq {
+struct compat_xfs_fsop_bulkreq {
compat_uptr_t lastip; /* last inode # pointer */
__s32 icount; /* count of entries in buffer */
compat_uptr_t ubuffer; /* user buffer for inode desc. */
compat_uptr_t ocount; /* output count pointer */
-} compat_xfs_fsop_bulkreq_t;
+};
#define XFS_IOC_FSBULKSTAT_32 \
_IOWR('X', 101, struct compat_xfs_fsop_bulkreq)
@@ -106,7 +106,7 @@ typedef struct compat_xfs_swapext {
xfs_off_t sx_offset; /* offset into file */
xfs_off_t sx_length; /* leng from offset */
char sx_pad[16]; /* pad space, unused */
- compat_xfs_bstat_t sx_stat; /* stat of target b4 copy */
+ struct compat_xfs_bstat sx_stat; /* stat of target b4 copy */
} __compat_packed compat_xfs_swapext_t;
#define XFS_IOC_SWAPEXT_32 _IOWR('X', 109, struct compat_xfs_swapext)
@@ -201,11 +201,11 @@ typedef struct compat_xfs_fsop_geom_v1 {
#define XFS_IOC_FSGEOMETRY_V1_32 \
_IOR('X', 100, struct compat_xfs_fsop_geom_v1)
-typedef struct compat_xfs_inogrp {
+struct compat_xfs_inogrp {
__u64 xi_startino; /* starting inode number */
__s32 xi_alloccount; /* # bits set in allocmask */
__u64 xi_allocmask; /* mask of allocated inodes */
-} __attribute__((packed)) compat_xfs_inogrp_t;
+} __attribute__((packed));
/* These growfs input structures have padding on the end, so must translate */
typedef struct compat_xfs_growfs_data {
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/9] xfs: remove various bulk request typedef usage
2019-05-29 22:27 ` [PATCH 1/9] xfs: remove various bulk request typedef usage Darrick J. Wong
@ 2019-06-05 22:29 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:29 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Remove xfs_bstat_t, xfs_fsop_bulkreq_t, xfs_inogrp_t, and similarly
> named compat typedefs.
>
Looks good to me. Thanks!
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 16 ++++++++--------
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_ioctl32.c | 11 +++++++----
> fs/xfs/xfs_ioctl32.h | 14 +++++++-------
> 4 files changed, 23 insertions(+), 20 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index e7382c780ed7..ef0dce229fa4 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -97,7 +97,7 @@ struct getbmapx {
> * For use by backup and restore programs to set the XFS on-disk inode
> * fields di_dmevmask and di_dmstate. These must be set to exactly and
> * only values previously obtained via xfs_bulkstat! (Specifically the
> - * xfs_bstat_t fields bs_dmevmask and bs_dmstate.)
> + * struct xfs_bstat fields bs_dmevmask and bs_dmstate.)
> */
> #ifndef HAVE_FSDMIDATA
> struct fsdmidata {
> @@ -328,7 +328,7 @@ typedef struct xfs_bstime {
> __s32 tv_nsec; /* and nanoseconds */
> } xfs_bstime_t;
>
> -typedef struct xfs_bstat {
> +struct xfs_bstat {
> __u64 bs_ino; /* inode number */
> __u16 bs_mode; /* type and mode */
> __u16 bs_nlink; /* number of links */
> @@ -356,7 +356,7 @@ typedef struct xfs_bstat {
> __u32 bs_dmevmask; /* DMIG event mask */
> __u16 bs_dmstate; /* DMIG state info */
> __u16 bs_aextents; /* attribute number of extents */
> -} xfs_bstat_t;
> +};
>
> /* bs_sick flags */
> #define XFS_BS_SICK_INODE (1 << 0) /* inode core */
> @@ -382,22 +382,22 @@ bstat_get_projid(struct xfs_bstat *bs)
> /*
> * The user-level BulkStat Request interface structure.
> */
> -typedef struct xfs_fsop_bulkreq {
> +struct xfs_fsop_bulkreq {
> __u64 __user *lastip; /* last inode # pointer */
> __s32 icount; /* count of entries in buffer */
> void __user *ubuffer;/* user buffer for inode desc. */
> __s32 __user *ocount; /* output count pointer */
> -} xfs_fsop_bulkreq_t;
> +};
>
>
> /*
> * Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
> */
> -typedef struct xfs_inogrp {
> +struct xfs_inogrp {
> __u64 xi_startino; /* starting inode number */
> __s32 xi_alloccount; /* # bits set in allocmask */
> __u64 xi_allocmask; /* mask of allocated inodes */
> -} xfs_inogrp_t;
> +};
>
>
> /*
> @@ -529,7 +529,7 @@ typedef struct xfs_swapext
> xfs_off_t sx_offset; /* offset into file */
> xfs_off_t sx_length; /* leng from offset */
> char sx_pad[16]; /* pad space, unused */
> - xfs_bstat_t sx_stat; /* stat of target b4 copy */
> + struct xfs_bstat sx_stat; /* stat of target b4 copy */
> } xfs_swapext_t;
>
> /*
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 4fa9a2c8b029..456a0e132d6d 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -764,7 +764,7 @@ xfs_ioc_bulkstat(
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> - if (copy_from_user(&bulkreq, arg, sizeof(xfs_fsop_bulkreq_t)))
> + if (copy_from_user(&bulkreq, arg, sizeof(struct xfs_fsop_bulkreq)))
> return -EFAULT;
>
> if (copy_from_user(&lastino, bulkreq.lastip, sizeof(__s64)))
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index dd53a9692e68..8dcb7046ed15 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -116,11 +116,14 @@ xfs_ioctl32_bstime_copyin(
> return 0;
> }
>
> -/* xfs_bstat_t has differing alignment on intel, & bstime_t sizes everywhere */
> +/*
> + * struct xfs_bstat has differing alignment on intel, & bstime_t sizes
> + * everywhere
> + */
> STATIC int
> xfs_ioctl32_bstat_copyin(
> - xfs_bstat_t *bstat,
> - compat_xfs_bstat_t __user *bstat32)
> + struct xfs_bstat *bstat,
> + struct compat_xfs_bstat __user *bstat32)
> {
> if (get_user(bstat->bs_ino, &bstat32->bs_ino) ||
> get_user(bstat->bs_mode, &bstat32->bs_mode) ||
> @@ -204,7 +207,7 @@ STATIC int
> xfs_compat_ioc_bulkstat(
> xfs_mount_t *mp,
> unsigned int cmd,
> - compat_xfs_fsop_bulkreq_t __user *p32)
> + struct compat_xfs_fsop_bulkreq __user *p32)
> {
> u32 addr;
> struct xfs_fsop_bulkreq bulkreq;
> diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h
> index d28fa824284a..7985344d3aa6 100644
> --- a/fs/xfs/xfs_ioctl32.h
> +++ b/fs/xfs/xfs_ioctl32.h
> @@ -36,7 +36,7 @@ typedef struct compat_xfs_bstime {
> __s32 tv_nsec; /* and nanoseconds */
> } compat_xfs_bstime_t;
>
> -typedef struct compat_xfs_bstat {
> +struct compat_xfs_bstat {
> __u64 bs_ino; /* inode number */
> __u16 bs_mode; /* type and mode */
> __u16 bs_nlink; /* number of links */
> @@ -61,14 +61,14 @@ typedef struct compat_xfs_bstat {
> __u32 bs_dmevmask; /* DMIG event mask */
> __u16 bs_dmstate; /* DMIG state info */
> __u16 bs_aextents; /* attribute number of extents */
> -} __compat_packed compat_xfs_bstat_t;
> +} __compat_packed;
>
> -typedef struct compat_xfs_fsop_bulkreq {
> +struct compat_xfs_fsop_bulkreq {
> compat_uptr_t lastip; /* last inode # pointer */
> __s32 icount; /* count of entries in buffer */
> compat_uptr_t ubuffer; /* user buffer for inode desc. */
> compat_uptr_t ocount; /* output count pointer */
> -} compat_xfs_fsop_bulkreq_t;
> +};
>
> #define XFS_IOC_FSBULKSTAT_32 \
> _IOWR('X', 101, struct compat_xfs_fsop_bulkreq)
> @@ -106,7 +106,7 @@ typedef struct compat_xfs_swapext {
> xfs_off_t sx_offset; /* offset into file */
> xfs_off_t sx_length; /* leng from offset */
> char sx_pad[16]; /* pad space, unused */
> - compat_xfs_bstat_t sx_stat; /* stat of target b4 copy */
> + struct compat_xfs_bstat sx_stat; /* stat of target b4 copy */
> } __compat_packed compat_xfs_swapext_t;
>
> #define XFS_IOC_SWAPEXT_32 _IOWR('X', 109, struct compat_xfs_swapext)
> @@ -201,11 +201,11 @@ typedef struct compat_xfs_fsop_geom_v1 {
> #define XFS_IOC_FSGEOMETRY_V1_32 \
> _IOR('X', 100, struct compat_xfs_fsop_geom_v1)
>
> -typedef struct compat_xfs_inogrp {
> +struct compat_xfs_inogrp {
> __u64 xi_startino; /* starting inode number */
> __s32 xi_alloccount; /* # bits set in allocmask */
> __u64 xi_allocmask; /* mask of allocated inodes */
> -} __attribute__((packed)) compat_xfs_inogrp_t;
> +} __attribute__((packed));
>
> /* These growfs input structures have padding on the end, so must translate */
> typedef struct compat_xfs_growfs_data {
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/9] xfs: rename bulkstat functions
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
2019-05-29 22:27 ` [PATCH 1/9] xfs: remove various bulk request typedef usage Darrick J. Wong
@ 2019-05-29 22:27 ` Darrick J. Wong
2019-06-05 22:29 ` Allison Collins
2019-05-29 22:27 ` [PATCH 3/9] xfs: introduce new v5 bulkstat structure Darrick J. Wong
` (6 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:27 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Rename the bulkstat functions to 'fsbulkstat' so that they match the
ioctl names. We will be introducing a new set of bulkstat/inumbers
ioctls soon, and it will be important to keep the names straight.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_ioctl.c | 14 +++++++-------
fs/xfs/xfs_ioctl.h | 5 +++--
fs/xfs/xfs_ioctl32.c | 18 +++++++++---------
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 456a0e132d6d..f02a9bd757ad 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -723,7 +723,7 @@ xfs_ioc_space(
/* Return 0 on success or positive error */
int
-xfs_bulkstat_one_fmt(
+xfs_fsbulkstat_one_fmt(
struct xfs_ibulk *breq,
const struct xfs_bstat *bstat)
{
@@ -733,7 +733,7 @@ xfs_bulkstat_one_fmt(
}
int
-xfs_inumbers_fmt(
+xfs_fsinumbers_fmt(
struct xfs_ibulk *breq,
const struct xfs_inogrp *igrp)
{
@@ -743,7 +743,7 @@ xfs_inumbers_fmt(
}
STATIC int
-xfs_ioc_bulkstat(
+xfs_ioc_fsbulkstat(
xfs_mount_t *mp,
unsigned int cmd,
void __user *arg)
@@ -790,15 +790,15 @@ xfs_ioc_bulkstat(
*/
if (cmd == XFS_IOC_FSINUMBERS) {
breq.startino = lastino + 1;
- error = xfs_inumbers(&breq, xfs_inumbers_fmt);
+ error = xfs_inumbers(&breq, xfs_fsinumbers_fmt);
lastino = breq.startino - 1;
} else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE) {
breq.startino = lastino;
- error = xfs_bulkstat_one(&breq, xfs_bulkstat_one_fmt);
+ error = xfs_bulkstat_one(&breq, xfs_fsbulkstat_one_fmt);
lastino = breq.startino;
} else { /* XFS_IOC_FSBULKSTAT */
breq.startino = lastino + 1;
- error = xfs_bulkstat(&breq, xfs_bulkstat_one_fmt);
+ error = xfs_bulkstat(&breq, xfs_fsbulkstat_one_fmt);
lastino = breq.startino - 1;
}
@@ -1978,7 +1978,7 @@ xfs_file_ioctl(
case XFS_IOC_FSBULKSTAT_SINGLE:
case XFS_IOC_FSBULKSTAT:
case XFS_IOC_FSINUMBERS:
- return xfs_ioc_bulkstat(mp, cmd, arg);
+ return xfs_ioc_fsbulkstat(mp, cmd, arg);
case XFS_IOC_FSGEOMETRY_V1:
return xfs_ioc_fsgeometry(mp, arg, 3);
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index fb303eaa8863..cb34bc821201 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -81,7 +81,8 @@ struct xfs_ibulk;
struct xfs_bstat;
struct xfs_inogrp;
-int xfs_bulkstat_one_fmt(struct xfs_ibulk *breq, const struct xfs_bstat *bstat);
-int xfs_inumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
+int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
+ const struct xfs_bstat *bstat);
+int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
#endif
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 8dcb7046ed15..af753f2708e8 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -84,7 +84,7 @@ xfs_compat_growfs_rt_copyin(
}
STATIC int
-xfs_inumbers_fmt_compat(
+xfs_fsinumbers_fmt_compat(
struct xfs_ibulk *breq,
const struct xfs_inogrp *igrp)
{
@@ -99,7 +99,7 @@ xfs_inumbers_fmt_compat(
}
#else
-#define xfs_inumbers_fmt_compat xfs_inumbers_fmt
+#define xfs_fsinumbers_fmt_compat xfs_fsinumbers_fmt
#endif /* BROKEN_X86_ALIGNMENT */
STATIC int
@@ -169,7 +169,7 @@ xfs_bstime_store_compat(
/* Return 0 on success or positive error (to xfs_bulkstat()) */
STATIC int
-xfs_bulkstat_one_fmt_compat(
+xfs_fsbulkstat_one_fmt_compat(
struct xfs_ibulk *breq,
const struct xfs_bstat *buffer)
{
@@ -204,7 +204,7 @@ xfs_bulkstat_one_fmt_compat(
/* copied from xfs_ioctl.c */
STATIC int
-xfs_compat_ioc_bulkstat(
+xfs_compat_ioc_fsbulkstat(
xfs_mount_t *mp,
unsigned int cmd,
struct compat_xfs_fsop_bulkreq __user *p32)
@@ -223,8 +223,8 @@ xfs_compat_ioc_bulkstat(
* to userpace memory via bulkreq.ubuffer. Normally the compat
* functions and structure size are the correct ones to use ...
*/
- inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
- bulkstat_one_fmt_pf bs_one_func = xfs_bulkstat_one_fmt_compat;
+ inumbers_fmt_pf inumbers_func = xfs_fsinumbers_fmt_compat;
+ bulkstat_one_fmt_pf bs_one_func = xfs_fsbulkstat_one_fmt_compat;
#ifdef CONFIG_X86_X32
if (in_x32_syscall()) {
@@ -236,8 +236,8 @@ xfs_compat_ioc_bulkstat(
* the data written out in compat layout will not match what
* x32 userspace expects.
*/
- inumbers_func = xfs_inumbers_fmt;
- bs_one_func = xfs_bulkstat_one_fmt;
+ inumbers_func = xfs_fsinumbers_fmt;
+ bs_one_func = xfs_fsbulkstat_one_fmt;
}
#endif
@@ -665,7 +665,7 @@ xfs_file_compat_ioctl(
case XFS_IOC_FSBULKSTAT_32:
case XFS_IOC_FSBULKSTAT_SINGLE_32:
case XFS_IOC_FSINUMBERS_32:
- return xfs_compat_ioc_bulkstat(mp, cmd, arg);
+ return xfs_compat_ioc_fsbulkstat(mp, cmd, arg);
case XFS_IOC_FD_TO_HANDLE_32:
case XFS_IOC_PATH_TO_HANDLE_32:
case XFS_IOC_PATH_TO_FSHANDLE_32: {
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 2/9] xfs: rename bulkstat functions
2019-05-29 22:27 ` [PATCH 2/9] xfs: rename bulkstat functions Darrick J. Wong
@ 2019-06-05 22:29 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:29 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Rename the bulkstat functions to 'fsbulkstat' so that they match the
> ioctl names. We will be introducing a new set of bulkstat/inumbers
> ioctls soon, and it will be important to keep the names straight.
>
Looks OK
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_ioctl.c | 14 +++++++-------
> fs/xfs/xfs_ioctl.h | 5 +++--
> fs/xfs/xfs_ioctl32.c | 18 +++++++++---------
> 3 files changed, 19 insertions(+), 18 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 456a0e132d6d..f02a9bd757ad 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -723,7 +723,7 @@ xfs_ioc_space(
>
> /* Return 0 on success or positive error */
> int
> -xfs_bulkstat_one_fmt(
> +xfs_fsbulkstat_one_fmt(
> struct xfs_ibulk *breq,
> const struct xfs_bstat *bstat)
> {
> @@ -733,7 +733,7 @@ xfs_bulkstat_one_fmt(
> }
>
> int
> -xfs_inumbers_fmt(
> +xfs_fsinumbers_fmt(
> struct xfs_ibulk *breq,
> const struct xfs_inogrp *igrp)
> {
> @@ -743,7 +743,7 @@ xfs_inumbers_fmt(
> }
>
> STATIC int
> -xfs_ioc_bulkstat(
> +xfs_ioc_fsbulkstat(
> xfs_mount_t *mp,
> unsigned int cmd,
> void __user *arg)
> @@ -790,15 +790,15 @@ xfs_ioc_bulkstat(
> */
> if (cmd == XFS_IOC_FSINUMBERS) {
> breq.startino = lastino + 1;
> - error = xfs_inumbers(&breq, xfs_inumbers_fmt);
> + error = xfs_inumbers(&breq, xfs_fsinumbers_fmt);
> lastino = breq.startino - 1;
> } else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE) {
> breq.startino = lastino;
> - error = xfs_bulkstat_one(&breq, xfs_bulkstat_one_fmt);
> + error = xfs_bulkstat_one(&breq, xfs_fsbulkstat_one_fmt);
> lastino = breq.startino;
> } else { /* XFS_IOC_FSBULKSTAT */
> breq.startino = lastino + 1;
> - error = xfs_bulkstat(&breq, xfs_bulkstat_one_fmt);
> + error = xfs_bulkstat(&breq, xfs_fsbulkstat_one_fmt);
> lastino = breq.startino - 1;
> }
>
> @@ -1978,7 +1978,7 @@ xfs_file_ioctl(
> case XFS_IOC_FSBULKSTAT_SINGLE:
> case XFS_IOC_FSBULKSTAT:
> case XFS_IOC_FSINUMBERS:
> - return xfs_ioc_bulkstat(mp, cmd, arg);
> + return xfs_ioc_fsbulkstat(mp, cmd, arg);
>
> case XFS_IOC_FSGEOMETRY_V1:
> return xfs_ioc_fsgeometry(mp, arg, 3);
> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> index fb303eaa8863..cb34bc821201 100644
> --- a/fs/xfs/xfs_ioctl.h
> +++ b/fs/xfs/xfs_ioctl.h
> @@ -81,7 +81,8 @@ struct xfs_ibulk;
> struct xfs_bstat;
> struct xfs_inogrp;
>
> -int xfs_bulkstat_one_fmt(struct xfs_ibulk *breq, const struct xfs_bstat *bstat);
> -int xfs_inumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
> +int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
> + const struct xfs_bstat *bstat);
> +int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
>
> #endif
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 8dcb7046ed15..af753f2708e8 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -84,7 +84,7 @@ xfs_compat_growfs_rt_copyin(
> }
>
> STATIC int
> -xfs_inumbers_fmt_compat(
> +xfs_fsinumbers_fmt_compat(
> struct xfs_ibulk *breq,
> const struct xfs_inogrp *igrp)
> {
> @@ -99,7 +99,7 @@ xfs_inumbers_fmt_compat(
> }
>
> #else
> -#define xfs_inumbers_fmt_compat xfs_inumbers_fmt
> +#define xfs_fsinumbers_fmt_compat xfs_fsinumbers_fmt
> #endif /* BROKEN_X86_ALIGNMENT */
>
> STATIC int
> @@ -169,7 +169,7 @@ xfs_bstime_store_compat(
>
> /* Return 0 on success or positive error (to xfs_bulkstat()) */
> STATIC int
> -xfs_bulkstat_one_fmt_compat(
> +xfs_fsbulkstat_one_fmt_compat(
> struct xfs_ibulk *breq,
> const struct xfs_bstat *buffer)
> {
> @@ -204,7 +204,7 @@ xfs_bulkstat_one_fmt_compat(
>
> /* copied from xfs_ioctl.c */
> STATIC int
> -xfs_compat_ioc_bulkstat(
> +xfs_compat_ioc_fsbulkstat(
> xfs_mount_t *mp,
> unsigned int cmd,
> struct compat_xfs_fsop_bulkreq __user *p32)
> @@ -223,8 +223,8 @@ xfs_compat_ioc_bulkstat(
> * to userpace memory via bulkreq.ubuffer. Normally the compat
> * functions and structure size are the correct ones to use ...
> */
> - inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
> - bulkstat_one_fmt_pf bs_one_func = xfs_bulkstat_one_fmt_compat;
> + inumbers_fmt_pf inumbers_func = xfs_fsinumbers_fmt_compat;
> + bulkstat_one_fmt_pf bs_one_func = xfs_fsbulkstat_one_fmt_compat;
>
> #ifdef CONFIG_X86_X32
> if (in_x32_syscall()) {
> @@ -236,8 +236,8 @@ xfs_compat_ioc_bulkstat(
> * the data written out in compat layout will not match what
> * x32 userspace expects.
> */
> - inumbers_func = xfs_inumbers_fmt;
> - bs_one_func = xfs_bulkstat_one_fmt;
> + inumbers_func = xfs_fsinumbers_fmt;
> + bs_one_func = xfs_fsbulkstat_one_fmt;
> }
> #endif
>
> @@ -665,7 +665,7 @@ xfs_file_compat_ioctl(
> case XFS_IOC_FSBULKSTAT_32:
> case XFS_IOC_FSBULKSTAT_SINGLE_32:
> case XFS_IOC_FSINUMBERS_32:
> - return xfs_compat_ioc_bulkstat(mp, cmd, arg);
> + return xfs_compat_ioc_fsbulkstat(mp, cmd, arg);
> case XFS_IOC_FD_TO_HANDLE_32:
> case XFS_IOC_PATH_TO_HANDLE_32:
> case XFS_IOC_PATH_TO_FSHANDLE_32: {
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/9] xfs: introduce new v5 bulkstat structure
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
2019-05-29 22:27 ` [PATCH 1/9] xfs: remove various bulk request typedef usage Darrick J. Wong
2019-05-29 22:27 ` [PATCH 2/9] xfs: rename bulkstat functions Darrick J. Wong
@ 2019-05-29 22:27 ` Darrick J. Wong
2019-06-05 22:29 ` Allison Collins
2019-05-29 22:27 ` [PATCH 4/9] xfs: introduce v5 inode group structure Darrick J. Wong
` (5 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:27 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Introduce a new version of the in-core bulkstat structure that supports
our new v5 format features. This structure also fills the gaps in the
previous structure. We leave wiring up the ioctls for the next patch.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 48 +++++++++++++++++++++++++++++++-
fs/xfs/libxfs/xfs_health.h | 2 +
fs/xfs/xfs_health.c | 2 +
fs/xfs/xfs_ioctl.c | 9 ++++--
fs/xfs/xfs_ioctl.h | 2 +
fs/xfs/xfs_ioctl32.c | 10 +++++--
fs/xfs/xfs_itable.c | 67 +++++++++++++++++++++++++++++++++-----------
fs/xfs/xfs_itable.h | 4 ++-
fs/xfs/xfs_ondisk.h | 2 +
9 files changed, 119 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index ef0dce229fa4..132e364eb141 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -358,6 +358,52 @@ struct xfs_bstat {
__u16 bs_aextents; /* attribute number of extents */
};
+/* New bulkstat structure that reports v5 features and fixes padding issues */
+struct xfs_bulkstat {
+ uint64_t bs_ino; /* inode number */
+ uint64_t bs_size; /* file size */
+
+ uint64_t bs_blocks; /* number of blocks */
+ uint64_t bs_xflags; /* extended flags */
+
+ uint64_t bs_atime; /* access time, seconds */
+ uint64_t bs_mtime; /* modify time, seconds */
+
+ uint64_t bs_ctime; /* inode change time, seconds */
+ uint64_t bs_btime; /* creation time, seconds */
+
+ uint32_t bs_gen; /* generation count */
+ uint32_t bs_uid; /* user id */
+ uint32_t bs_gid; /* group id */
+ uint32_t bs_projectid; /* project id */
+
+ uint32_t bs_atime_nsec; /* access time, nanoseconds */
+ uint32_t bs_mtime_nsec; /* modify time, nanoseconds */
+ uint32_t bs_ctime_nsec; /* inode change time, nanoseconds */
+ uint32_t bs_btime_nsec; /* creation time, nanoseconds */
+
+ uint32_t bs_blksize; /* block size */
+ uint32_t bs_rdev; /* device value */
+ uint32_t bs_cowextsize_blks; /* cow extent size hint, blocks */
+ uint32_t bs_extsize_blks; /* extent size hint, blocks */
+
+ uint32_t bs_nlink; /* number of links */
+ uint32_t bs_extents; /* number of extents */
+ uint32_t bs_aextents; /* attribute number of extents */
+ uint16_t bs_version; /* structure version */
+ uint16_t bs_forkoff; /* inode fork offset in bytes */
+
+ uint16_t bs_sick; /* sick inode metadata */
+ uint16_t bs_checked; /* checked inode metadata */
+ uint16_t bs_mode; /* type and mode */
+ uint16_t bs_pad2; /* zeroed */
+
+ uint64_t bs_pad[7]; /* zeroed */
+};
+
+#define XFS_BULKSTAT_VERSION_V1 (1)
+#define XFS_BULKSTAT_VERSION_V5 (5)
+
/* bs_sick flags */
#define XFS_BS_SICK_INODE (1 << 0) /* inode core */
#define XFS_BS_SICK_BMBTD (1 << 1) /* data fork */
@@ -374,7 +420,7 @@ struct xfs_bstat {
* to retain compatibility with "old" filesystems).
*/
static inline uint32_t
-bstat_get_projid(struct xfs_bstat *bs)
+bstat_get_projid(const struct xfs_bstat *bs)
{
return (uint32_t)bs->bs_projid_hi << 16 | bs->bs_projid_lo;
}
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 49ddfeac19f2..272005ac8c88 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -185,6 +185,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
-void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
+void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bulkstat *bs);
#endif /* __XFS_HEALTH_H__ */
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 4c4929f9e7bf..e8ba6034b0db 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -373,7 +373,7 @@ static const struct ioctl_sick_map ino_map[] = {
void
xfs_bulkstat_health(
struct xfs_inode *ip,
- struct xfs_bstat *bs)
+ struct xfs_bulkstat *bs)
{
const struct ioctl_sick_map *m;
unsigned int sick;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index f02a9bd757ad..0f70005cbe61 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -724,10 +724,13 @@ xfs_ioc_space(
/* Return 0 on success or positive error */
int
xfs_fsbulkstat_one_fmt(
- struct xfs_ibulk *breq,
- const struct xfs_bstat *bstat)
+ struct xfs_ibulk *breq,
+ const struct xfs_bulkstat *bstat)
{
- if (copy_to_user(breq->ubuffer, bstat, sizeof(*bstat)))
+ struct xfs_bstat bs1;
+
+ xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
+ if (copy_to_user(breq->ubuffer, &bs1, sizeof(bs1)))
return -EFAULT;
return xfs_ibulk_advance(breq, sizeof(struct xfs_bstat));
}
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index cb34bc821201..514d3028a134 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -82,7 +82,7 @@ struct xfs_bstat;
struct xfs_inogrp;
int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
- const struct xfs_bstat *bstat);
+ const struct xfs_bulkstat *bstat);
int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
#endif
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index af753f2708e8..9806d27892df 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -170,10 +170,14 @@ xfs_bstime_store_compat(
/* Return 0 on success or positive error (to xfs_bulkstat()) */
STATIC int
xfs_fsbulkstat_one_fmt_compat(
- struct xfs_ibulk *breq,
- const struct xfs_bstat *buffer)
+ struct xfs_ibulk *breq,
+ const struct xfs_bulkstat *bstat)
{
- struct compat_xfs_bstat __user *p32 = breq->ubuffer;
+ struct compat_xfs_bstat __user *p32 = breq->ubuffer;
+ struct xfs_bstat bs1;
+ struct xfs_bstat *buffer = &bs1;
+
+ xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
if (put_user(buffer->bs_ino, &p32->bs_ino) ||
put_user(buffer->bs_mode, &p32->bs_mode) ||
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index bade54d6ac64..764b7f98fd5b 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -25,7 +25,7 @@
* Bulk Stat
* =========
*
- * Use the inode walking functions to fill out struct xfs_bstat for every
+ * Use the inode walking functions to fill out struct xfs_bulkstat for every
* allocated inode, then pass the stat information to some externally provided
* iteration function.
*/
@@ -62,7 +62,7 @@ xfs_bulkstat_one_int(
struct xfs_icdinode *dic; /* dinode core info pointer */
struct xfs_inode *ip; /* incore inode pointer */
struct inode *inode;
- struct xfs_bstat *buf; /* return buffer */
+ struct xfs_bulkstat *buf; /* return buffer */
int error = 0; /* error value */
if (xfs_internal_inum(mp, ino)) {
@@ -91,37 +91,35 @@ xfs_bulkstat_one_int(
/* xfs_iget returns the following without needing
* further change.
*/
- buf->bs_projid_lo = dic->di_projid_lo;
- buf->bs_projid_hi = dic->di_projid_hi;
+ buf->bs_projectid = xfs_get_projid(ip);
buf->bs_ino = ino;
buf->bs_uid = dic->di_uid;
buf->bs_gid = dic->di_gid;
buf->bs_size = dic->di_size;
buf->bs_nlink = inode->i_nlink;
- buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
- buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
- buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
- buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
- buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
- buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
+ buf->bs_atime = inode->i_atime.tv_sec;
+ buf->bs_atime_nsec = inode->i_atime.tv_nsec;
+ buf->bs_mtime = inode->i_mtime.tv_sec;
+ buf->bs_mtime_nsec = inode->i_mtime.tv_nsec;
+ buf->bs_ctime = inode->i_ctime.tv_sec;
+ buf->bs_ctime_nsec = inode->i_ctime.tv_nsec;
+ buf->bs_btime = dic->di_crtime.t_sec;
+ buf->bs_btime_nsec = dic->di_crtime.t_nsec;
buf->bs_gen = inode->i_generation;
buf->bs_mode = inode->i_mode;
buf->bs_xflags = xfs_ip2xflags(ip);
- buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
+ buf->bs_extsize_blks = dic->di_extsize;
buf->bs_extents = dic->di_nextents;
- memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
xfs_bulkstat_health(ip, buf);
- buf->bs_dmevmask = dic->di_dmevmask;
- buf->bs_dmstate = dic->di_dmstate;
buf->bs_aextents = dic->di_anextents;
buf->bs_forkoff = XFS_IFORK_BOFF(ip);
+ buf->bs_version = XFS_BULKSTAT_VERSION_V5;
if (dic->di_version == 3) {
if (dic->di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
- buf->bs_cowextsize = dic->di_cowextsize <<
- mp->m_sb.sb_blocklog;
+ buf->bs_cowextsize_blks = dic->di_cowextsize;
}
switch (dic->di_format) {
@@ -259,6 +257,43 @@ xfs_bulkstat(
return error;
}
+/* Convert bulkstat (v5) to bstat (v1). */
+void
+xfs_bulkstat_to_bstat(
+ struct xfs_mount *mp,
+ struct xfs_bstat *bs1,
+ const struct xfs_bulkstat *bstat)
+{
+ bs1->bs_ino = bstat->bs_ino;
+ bs1->bs_mode = bstat->bs_mode;
+ bs1->bs_nlink = bstat->bs_nlink;
+ bs1->bs_uid = bstat->bs_uid;
+ bs1->bs_gid = bstat->bs_gid;
+ bs1->bs_rdev = bstat->bs_rdev;
+ bs1->bs_blksize = bstat->bs_blksize;
+ bs1->bs_size = bstat->bs_size;
+ bs1->bs_atime.tv_sec = bstat->bs_atime;
+ bs1->bs_mtime.tv_sec = bstat->bs_mtime;
+ bs1->bs_ctime.tv_sec = bstat->bs_ctime;
+ bs1->bs_atime.tv_nsec = bstat->bs_atime_nsec;
+ bs1->bs_mtime.tv_nsec = bstat->bs_mtime_nsec;
+ bs1->bs_ctime.tv_nsec = bstat->bs_ctime_nsec;
+ bs1->bs_blocks = bstat->bs_blocks;
+ bs1->bs_xflags = bstat->bs_xflags;
+ bs1->bs_extsize = bstat->bs_extsize_blks << mp->m_sb.sb_blocklog;
+ bs1->bs_extents = bstat->bs_extents;
+ bs1->bs_gen = bstat->bs_gen;
+ bs1->bs_projid_lo = bstat->bs_projectid & 0xFFFF;
+ bs1->bs_forkoff = bstat->bs_forkoff;
+ bs1->bs_projid_hi = bstat->bs_projectid >> 16;
+ bs1->bs_sick = bstat->bs_sick;
+ bs1->bs_checked = bstat->bs_checked;
+ bs1->bs_cowextsize = bstat->bs_cowextsize_blks << mp->m_sb.sb_blocklog;
+ bs1->bs_dmevmask = 0;
+ bs1->bs_dmstate = 0;
+ bs1->bs_aextents = bstat->bs_aextents;
+}
+
struct xfs_inumbers_chunk {
inumbers_fmt_pf formatter;
struct xfs_ibulk *breq;
diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
index b4c89454e27a..806069c9838c 100644
--- a/fs/xfs/xfs_itable.h
+++ b/fs/xfs/xfs_itable.h
@@ -41,10 +41,12 @@ xfs_ibulk_advance(
*/
typedef int (*bulkstat_one_fmt_pf)(struct xfs_ibulk *breq,
- const struct xfs_bstat *bstat);
+ const struct xfs_bulkstat *bstat);
int xfs_bulkstat_one(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
int xfs_bulkstat(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
+void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
+ const struct xfs_bulkstat *bstat);
typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
const struct xfs_inogrp *igrp);
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index c8ba98fae30a..0b4cdda68524 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -146,6 +146,8 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0);
XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0);
XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
+
+ XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
}
#endif /* __XFS_ONDISK_H */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 3/9] xfs: introduce new v5 bulkstat structure
2019-05-29 22:27 ` [PATCH 3/9] xfs: introduce new v5 bulkstat structure Darrick J. Wong
@ 2019-06-05 22:29 ` Allison Collins
2019-06-06 20:17 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:29 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Introduce a new version of the in-core bulkstat structure that supports
> our new v5 format features. This structure also fills the gaps in the
> previous structure. We leave wiring up the ioctls for the next patch.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 48 +++++++++++++++++++++++++++++++-
> fs/xfs/libxfs/xfs_health.h | 2 +
> fs/xfs/xfs_health.c | 2 +
> fs/xfs/xfs_ioctl.c | 9 ++++--
> fs/xfs/xfs_ioctl.h | 2 +
> fs/xfs/xfs_ioctl32.c | 10 +++++--
> fs/xfs/xfs_itable.c | 67 +++++++++++++++++++++++++++++++++-----------
> fs/xfs/xfs_itable.h | 4 ++-
> fs/xfs/xfs_ondisk.h | 2 +
> 9 files changed, 119 insertions(+), 27 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index ef0dce229fa4..132e364eb141 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -358,6 +358,52 @@ struct xfs_bstat {
> __u16 bs_aextents; /* attribute number of extents */
> };
>
> +/* New bulkstat structure that reports v5 features and fixes padding issues */
> +struct xfs_bulkstat {
> + uint64_t bs_ino; /* inode number */
> + uint64_t bs_size; /* file size */
> +
> + uint64_t bs_blocks; /* number of blocks */
> + uint64_t bs_xflags; /* extended flags */
> +
> + uint64_t bs_atime; /* access time, seconds */
> + uint64_t bs_mtime; /* modify time, seconds */
> +
> + uint64_t bs_ctime; /* inode change time, seconds */
> + uint64_t bs_btime; /* creation time, seconds */
> +
> + uint32_t bs_gen; /* generation count */
> + uint32_t bs_uid; /* user id */
> + uint32_t bs_gid; /* group id */
> + uint32_t bs_projectid; /* project id */
> +
> + uint32_t bs_atime_nsec; /* access time, nanoseconds */
> + uint32_t bs_mtime_nsec; /* modify time, nanoseconds */
> + uint32_t bs_ctime_nsec; /* inode change time, nanoseconds */
> + uint32_t bs_btime_nsec; /* creation time, nanoseconds */
> +
> + uint32_t bs_blksize; /* block size */
> + uint32_t bs_rdev; /* device value */
> + uint32_t bs_cowextsize_blks; /* cow extent size hint, blocks */
> + uint32_t bs_extsize_blks; /* extent size hint, blocks */
> +
> + uint32_t bs_nlink; /* number of links */
> + uint32_t bs_extents; /* number of extents */
> + uint32_t bs_aextents; /* attribute number of extents */
> + uint16_t bs_version; /* structure version */
> + uint16_t bs_forkoff; /* inode fork offset in bytes */
> +
> + uint16_t bs_sick; /* sick inode metadata */
> + uint16_t bs_checked; /* checked inode metadata */
> + uint16_t bs_mode; /* type and mode */
> + uint16_t bs_pad2; /* zeroed */
> +
> + uint64_t bs_pad[7]; /* zeroed */
> +};
> +
> +#define XFS_BULKSTAT_VERSION_V1 (1)
> +#define XFS_BULKSTAT_VERSION_V5 (5)
Just a nit. I notice we have a V1 and V5, but I dont see the V1 being
used here or else where in the set. Was that intentional? Rest looks
ok though
Allison
> +
> /* bs_sick flags */
> #define XFS_BS_SICK_INODE (1 << 0) /* inode core */
> #define XFS_BS_SICK_BMBTD (1 << 1) /* data fork */
> @@ -374,7 +420,7 @@ struct xfs_bstat {
> * to retain compatibility with "old" filesystems).
> */
> static inline uint32_t
> -bstat_get_projid(struct xfs_bstat *bs)
> +bstat_get_projid(const struct xfs_bstat *bs)
> {
> return (uint32_t)bs->bs_projid_hi << 16 | bs->bs_projid_lo;
> }
> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> index 49ddfeac19f2..272005ac8c88 100644
> --- a/fs/xfs/libxfs/xfs_health.h
> +++ b/fs/xfs/libxfs/xfs_health.h
> @@ -185,6 +185,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
>
> void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
> void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
> -void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
> +void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bulkstat *bs);
>
> #endif /* __XFS_HEALTH_H__ */
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index 4c4929f9e7bf..e8ba6034b0db 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -373,7 +373,7 @@ static const struct ioctl_sick_map ino_map[] = {
> void
> xfs_bulkstat_health(
> struct xfs_inode *ip,
> - struct xfs_bstat *bs)
> + struct xfs_bulkstat *bs)
> {
> const struct ioctl_sick_map *m;
> unsigned int sick;
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index f02a9bd757ad..0f70005cbe61 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -724,10 +724,13 @@ xfs_ioc_space(
> /* Return 0 on success or positive error */
> int
> xfs_fsbulkstat_one_fmt(
> - struct xfs_ibulk *breq,
> - const struct xfs_bstat *bstat)
> + struct xfs_ibulk *breq,
> + const struct xfs_bulkstat *bstat)
> {
> - if (copy_to_user(breq->ubuffer, bstat, sizeof(*bstat)))
> + struct xfs_bstat bs1;
> +
> + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
> + if (copy_to_user(breq->ubuffer, &bs1, sizeof(bs1)))
> return -EFAULT;
> return xfs_ibulk_advance(breq, sizeof(struct xfs_bstat));
> }
> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> index cb34bc821201..514d3028a134 100644
> --- a/fs/xfs/xfs_ioctl.h
> +++ b/fs/xfs/xfs_ioctl.h
> @@ -82,7 +82,7 @@ struct xfs_bstat;
> struct xfs_inogrp;
>
> int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
> - const struct xfs_bstat *bstat);
> + const struct xfs_bulkstat *bstat);
> int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
>
> #endif
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index af753f2708e8..9806d27892df 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -170,10 +170,14 @@ xfs_bstime_store_compat(
> /* Return 0 on success or positive error (to xfs_bulkstat()) */
> STATIC int
> xfs_fsbulkstat_one_fmt_compat(
> - struct xfs_ibulk *breq,
> - const struct xfs_bstat *buffer)
> + struct xfs_ibulk *breq,
> + const struct xfs_bulkstat *bstat)
> {
> - struct compat_xfs_bstat __user *p32 = breq->ubuffer;
> + struct compat_xfs_bstat __user *p32 = breq->ubuffer;
> + struct xfs_bstat bs1;
> + struct xfs_bstat *buffer = &bs1;
> +
> + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
>
> if (put_user(buffer->bs_ino, &p32->bs_ino) ||
> put_user(buffer->bs_mode, &p32->bs_mode) ||
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index bade54d6ac64..764b7f98fd5b 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -25,7 +25,7 @@
> * Bulk Stat
> * =========
> *
> - * Use the inode walking functions to fill out struct xfs_bstat for every
> + * Use the inode walking functions to fill out struct xfs_bulkstat for every
> * allocated inode, then pass the stat information to some externally provided
> * iteration function.
> */
> @@ -62,7 +62,7 @@ xfs_bulkstat_one_int(
> struct xfs_icdinode *dic; /* dinode core info pointer */
> struct xfs_inode *ip; /* incore inode pointer */
> struct inode *inode;
> - struct xfs_bstat *buf; /* return buffer */
> + struct xfs_bulkstat *buf; /* return buffer */
> int error = 0; /* error value */
>
> if (xfs_internal_inum(mp, ino)) {
> @@ -91,37 +91,35 @@ xfs_bulkstat_one_int(
> /* xfs_iget returns the following without needing
> * further change.
> */
> - buf->bs_projid_lo = dic->di_projid_lo;
> - buf->bs_projid_hi = dic->di_projid_hi;
> + buf->bs_projectid = xfs_get_projid(ip);
> buf->bs_ino = ino;
> buf->bs_uid = dic->di_uid;
> buf->bs_gid = dic->di_gid;
> buf->bs_size = dic->di_size;
>
> buf->bs_nlink = inode->i_nlink;
> - buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
> - buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
> - buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
> - buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
> - buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
> - buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
> + buf->bs_atime = inode->i_atime.tv_sec;
> + buf->bs_atime_nsec = inode->i_atime.tv_nsec;
> + buf->bs_mtime = inode->i_mtime.tv_sec;
> + buf->bs_mtime_nsec = inode->i_mtime.tv_nsec;
> + buf->bs_ctime = inode->i_ctime.tv_sec;
> + buf->bs_ctime_nsec = inode->i_ctime.tv_nsec;
> + buf->bs_btime = dic->di_crtime.t_sec;
> + buf->bs_btime_nsec = dic->di_crtime.t_nsec;
> buf->bs_gen = inode->i_generation;
> buf->bs_mode = inode->i_mode;
>
> buf->bs_xflags = xfs_ip2xflags(ip);
> - buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
> + buf->bs_extsize_blks = dic->di_extsize;
> buf->bs_extents = dic->di_nextents;
> - memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
> xfs_bulkstat_health(ip, buf);
> - buf->bs_dmevmask = dic->di_dmevmask;
> - buf->bs_dmstate = dic->di_dmstate;
> buf->bs_aextents = dic->di_anextents;
> buf->bs_forkoff = XFS_IFORK_BOFF(ip);
> + buf->bs_version = XFS_BULKSTAT_VERSION_V5;
>
> if (dic->di_version == 3) {
> if (dic->di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
> - buf->bs_cowextsize = dic->di_cowextsize <<
> - mp->m_sb.sb_blocklog;
> + buf->bs_cowextsize_blks = dic->di_cowextsize;
> }
>
> switch (dic->di_format) {
> @@ -259,6 +257,43 @@ xfs_bulkstat(
> return error;
> }
>
> +/* Convert bulkstat (v5) to bstat (v1). */
> +void
> +xfs_bulkstat_to_bstat(
> + struct xfs_mount *mp,
> + struct xfs_bstat *bs1,
> + const struct xfs_bulkstat *bstat)
> +{
> + bs1->bs_ino = bstat->bs_ino;
> + bs1->bs_mode = bstat->bs_mode;
> + bs1->bs_nlink = bstat->bs_nlink;
> + bs1->bs_uid = bstat->bs_uid;
> + bs1->bs_gid = bstat->bs_gid;
> + bs1->bs_rdev = bstat->bs_rdev;
> + bs1->bs_blksize = bstat->bs_blksize;
> + bs1->bs_size = bstat->bs_size;
> + bs1->bs_atime.tv_sec = bstat->bs_atime;
> + bs1->bs_mtime.tv_sec = bstat->bs_mtime;
> + bs1->bs_ctime.tv_sec = bstat->bs_ctime;
> + bs1->bs_atime.tv_nsec = bstat->bs_atime_nsec;
> + bs1->bs_mtime.tv_nsec = bstat->bs_mtime_nsec;
> + bs1->bs_ctime.tv_nsec = bstat->bs_ctime_nsec;
> + bs1->bs_blocks = bstat->bs_blocks;
> + bs1->bs_xflags = bstat->bs_xflags;
> + bs1->bs_extsize = bstat->bs_extsize_blks << mp->m_sb.sb_blocklog;
> + bs1->bs_extents = bstat->bs_extents;
> + bs1->bs_gen = bstat->bs_gen;
> + bs1->bs_projid_lo = bstat->bs_projectid & 0xFFFF;
> + bs1->bs_forkoff = bstat->bs_forkoff;
> + bs1->bs_projid_hi = bstat->bs_projectid >> 16;
> + bs1->bs_sick = bstat->bs_sick;
> + bs1->bs_checked = bstat->bs_checked;
> + bs1->bs_cowextsize = bstat->bs_cowextsize_blks << mp->m_sb.sb_blocklog;
> + bs1->bs_dmevmask = 0;
> + bs1->bs_dmstate = 0;
> + bs1->bs_aextents = bstat->bs_aextents;
> +}
> +
> struct xfs_inumbers_chunk {
> inumbers_fmt_pf formatter;
> struct xfs_ibulk *breq;
> diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> index b4c89454e27a..806069c9838c 100644
> --- a/fs/xfs/xfs_itable.h
> +++ b/fs/xfs/xfs_itable.h
> @@ -41,10 +41,12 @@ xfs_ibulk_advance(
> */
>
> typedef int (*bulkstat_one_fmt_pf)(struct xfs_ibulk *breq,
> - const struct xfs_bstat *bstat);
> + const struct xfs_bulkstat *bstat);
>
> int xfs_bulkstat_one(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
> int xfs_bulkstat(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
> +void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
> + const struct xfs_bulkstat *bstat);
>
> typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
> const struct xfs_inogrp *igrp);
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index c8ba98fae30a..0b4cdda68524 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -146,6 +146,8 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0);
> XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0);
> XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
> +
> + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> }
>
> #endif /* __XFS_ONDISK_H */
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 3/9] xfs: introduce new v5 bulkstat structure
2019-06-05 22:29 ` Allison Collins
@ 2019-06-06 20:17 ` Darrick J. Wong
2019-06-06 22:41 ` Allison Collins
0 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-06-06 20:17 UTC (permalink / raw)
To: Allison Collins; +Cc: linux-xfs
On Wed, Jun 05, 2019 at 03:29:58PM -0700, Allison Collins wrote:
>
>
> On 5/29/19 3:27 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Introduce a new version of the in-core bulkstat structure that supports
> > our new v5 format features. This structure also fills the gaps in the
> > previous structure. We leave wiring up the ioctls for the next patch.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_fs.h | 48 +++++++++++++++++++++++++++++++-
> > fs/xfs/libxfs/xfs_health.h | 2 +
> > fs/xfs/xfs_health.c | 2 +
> > fs/xfs/xfs_ioctl.c | 9 ++++--
> > fs/xfs/xfs_ioctl.h | 2 +
> > fs/xfs/xfs_ioctl32.c | 10 +++++--
> > fs/xfs/xfs_itable.c | 67 +++++++++++++++++++++++++++++++++-----------
> > fs/xfs/xfs_itable.h | 4 ++-
> > fs/xfs/xfs_ondisk.h | 2 +
> > 9 files changed, 119 insertions(+), 27 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index ef0dce229fa4..132e364eb141 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -358,6 +358,52 @@ struct xfs_bstat {
> > __u16 bs_aextents; /* attribute number of extents */
> > };
> > +/* New bulkstat structure that reports v5 features and fixes padding issues */
> > +struct xfs_bulkstat {
> > + uint64_t bs_ino; /* inode number */
> > + uint64_t bs_size; /* file size */
> > +
> > + uint64_t bs_blocks; /* number of blocks */
> > + uint64_t bs_xflags; /* extended flags */
> > +
> > + uint64_t bs_atime; /* access time, seconds */
> > + uint64_t bs_mtime; /* modify time, seconds */
> > +
> > + uint64_t bs_ctime; /* inode change time, seconds */
> > + uint64_t bs_btime; /* creation time, seconds */
> > +
> > + uint32_t bs_gen; /* generation count */
> > + uint32_t bs_uid; /* user id */
> > + uint32_t bs_gid; /* group id */
> > + uint32_t bs_projectid; /* project id */
> > +
> > + uint32_t bs_atime_nsec; /* access time, nanoseconds */
> > + uint32_t bs_mtime_nsec; /* modify time, nanoseconds */
> > + uint32_t bs_ctime_nsec; /* inode change time, nanoseconds */
> > + uint32_t bs_btime_nsec; /* creation time, nanoseconds */
> > +
> > + uint32_t bs_blksize; /* block size */
> > + uint32_t bs_rdev; /* device value */
> > + uint32_t bs_cowextsize_blks; /* cow extent size hint, blocks */
> > + uint32_t bs_extsize_blks; /* extent size hint, blocks */
> > +
> > + uint32_t bs_nlink; /* number of links */
> > + uint32_t bs_extents; /* number of extents */
> > + uint32_t bs_aextents; /* attribute number of extents */
> > + uint16_t bs_version; /* structure version */
> > + uint16_t bs_forkoff; /* inode fork offset in bytes */
> > +
> > + uint16_t bs_sick; /* sick inode metadata */
> > + uint16_t bs_checked; /* checked inode metadata */
> > + uint16_t bs_mode; /* type and mode */
> > + uint16_t bs_pad2; /* zeroed */
> > +
> > + uint64_t bs_pad[7]; /* zeroed */
> > +};
> > +
> > +#define XFS_BULKSTAT_VERSION_V1 (1)
> > +#define XFS_BULKSTAT_VERSION_V5 (5)
>
> Just a nit. I notice we have a V1 and V5, but I dont see the V1 being used
> here or else where in the set. Was that intentional? Rest looks ok though
There's no version field in the old struct xfs_bstat, so that's why I'm
introducing a new flag now.
Further down the road, xfsprogs will be converted to use a wrapper
function to fill struct xfs_bulkstat. The wrapper will of course have
to emulate the v5 call using the old ioctl when userspace is (somehow)
newer than the kernel, and at that point the emulation will set
bs_version = V1 to hint to its caller that it's running on an old kernel
and therefore not to expect fields that weren't in the old version.
--D
> Allison
>
> > +
> > /* bs_sick flags */
> > #define XFS_BS_SICK_INODE (1 << 0) /* inode core */
> > #define XFS_BS_SICK_BMBTD (1 << 1) /* data fork */
> > @@ -374,7 +420,7 @@ struct xfs_bstat {
> > * to retain compatibility with "old" filesystems).
> > */
> > static inline uint32_t
> > -bstat_get_projid(struct xfs_bstat *bs)
> > +bstat_get_projid(const struct xfs_bstat *bs)
> > {
> > return (uint32_t)bs->bs_projid_hi << 16 | bs->bs_projid_lo;
> > }
> > diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> > index 49ddfeac19f2..272005ac8c88 100644
> > --- a/fs/xfs/libxfs/xfs_health.h
> > +++ b/fs/xfs/libxfs/xfs_health.h
> > @@ -185,6 +185,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
> > void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
> > void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
> > -void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
> > +void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bulkstat *bs);
> > #endif /* __XFS_HEALTH_H__ */
> > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> > index 4c4929f9e7bf..e8ba6034b0db 100644
> > --- a/fs/xfs/xfs_health.c
> > +++ b/fs/xfs/xfs_health.c
> > @@ -373,7 +373,7 @@ static const struct ioctl_sick_map ino_map[] = {
> > void
> > xfs_bulkstat_health(
> > struct xfs_inode *ip,
> > - struct xfs_bstat *bs)
> > + struct xfs_bulkstat *bs)
> > {
> > const struct ioctl_sick_map *m;
> > unsigned int sick;
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index f02a9bd757ad..0f70005cbe61 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -724,10 +724,13 @@ xfs_ioc_space(
> > /* Return 0 on success or positive error */
> > int
> > xfs_fsbulkstat_one_fmt(
> > - struct xfs_ibulk *breq,
> > - const struct xfs_bstat *bstat)
> > + struct xfs_ibulk *breq,
> > + const struct xfs_bulkstat *bstat)
> > {
> > - if (copy_to_user(breq->ubuffer, bstat, sizeof(*bstat)))
> > + struct xfs_bstat bs1;
> > +
> > + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
> > + if (copy_to_user(breq->ubuffer, &bs1, sizeof(bs1)))
> > return -EFAULT;
> > return xfs_ibulk_advance(breq, sizeof(struct xfs_bstat));
> > }
> > diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> > index cb34bc821201..514d3028a134 100644
> > --- a/fs/xfs/xfs_ioctl.h
> > +++ b/fs/xfs/xfs_ioctl.h
> > @@ -82,7 +82,7 @@ struct xfs_bstat;
> > struct xfs_inogrp;
> > int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
> > - const struct xfs_bstat *bstat);
> > + const struct xfs_bulkstat *bstat);
> > int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
> > #endif
> > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > index af753f2708e8..9806d27892df 100644
> > --- a/fs/xfs/xfs_ioctl32.c
> > +++ b/fs/xfs/xfs_ioctl32.c
> > @@ -170,10 +170,14 @@ xfs_bstime_store_compat(
> > /* Return 0 on success or positive error (to xfs_bulkstat()) */
> > STATIC int
> > xfs_fsbulkstat_one_fmt_compat(
> > - struct xfs_ibulk *breq,
> > - const struct xfs_bstat *buffer)
> > + struct xfs_ibulk *breq,
> > + const struct xfs_bulkstat *bstat)
> > {
> > - struct compat_xfs_bstat __user *p32 = breq->ubuffer;
> > + struct compat_xfs_bstat __user *p32 = breq->ubuffer;
> > + struct xfs_bstat bs1;
> > + struct xfs_bstat *buffer = &bs1;
> > +
> > + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
> > if (put_user(buffer->bs_ino, &p32->bs_ino) ||
> > put_user(buffer->bs_mode, &p32->bs_mode) ||
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index bade54d6ac64..764b7f98fd5b 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -25,7 +25,7 @@
> > * Bulk Stat
> > * =========
> > *
> > - * Use the inode walking functions to fill out struct xfs_bstat for every
> > + * Use the inode walking functions to fill out struct xfs_bulkstat for every
> > * allocated inode, then pass the stat information to some externally provided
> > * iteration function.
> > */
> > @@ -62,7 +62,7 @@ xfs_bulkstat_one_int(
> > struct xfs_icdinode *dic; /* dinode core info pointer */
> > struct xfs_inode *ip; /* incore inode pointer */
> > struct inode *inode;
> > - struct xfs_bstat *buf; /* return buffer */
> > + struct xfs_bulkstat *buf; /* return buffer */
> > int error = 0; /* error value */
> > if (xfs_internal_inum(mp, ino)) {
> > @@ -91,37 +91,35 @@ xfs_bulkstat_one_int(
> > /* xfs_iget returns the following without needing
> > * further change.
> > */
> > - buf->bs_projid_lo = dic->di_projid_lo;
> > - buf->bs_projid_hi = dic->di_projid_hi;
> > + buf->bs_projectid = xfs_get_projid(ip);
> > buf->bs_ino = ino;
> > buf->bs_uid = dic->di_uid;
> > buf->bs_gid = dic->di_gid;
> > buf->bs_size = dic->di_size;
> > buf->bs_nlink = inode->i_nlink;
> > - buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
> > - buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
> > - buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
> > - buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
> > - buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
> > - buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
> > + buf->bs_atime = inode->i_atime.tv_sec;
> > + buf->bs_atime_nsec = inode->i_atime.tv_nsec;
> > + buf->bs_mtime = inode->i_mtime.tv_sec;
> > + buf->bs_mtime_nsec = inode->i_mtime.tv_nsec;
> > + buf->bs_ctime = inode->i_ctime.tv_sec;
> > + buf->bs_ctime_nsec = inode->i_ctime.tv_nsec;
> > + buf->bs_btime = dic->di_crtime.t_sec;
> > + buf->bs_btime_nsec = dic->di_crtime.t_nsec;
> > buf->bs_gen = inode->i_generation;
> > buf->bs_mode = inode->i_mode;
> > buf->bs_xflags = xfs_ip2xflags(ip);
> > - buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
> > + buf->bs_extsize_blks = dic->di_extsize;
> > buf->bs_extents = dic->di_nextents;
> > - memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
> > xfs_bulkstat_health(ip, buf);
> > - buf->bs_dmevmask = dic->di_dmevmask;
> > - buf->bs_dmstate = dic->di_dmstate;
> > buf->bs_aextents = dic->di_anextents;
> > buf->bs_forkoff = XFS_IFORK_BOFF(ip);
> > + buf->bs_version = XFS_BULKSTAT_VERSION_V5;
> > if (dic->di_version == 3) {
> > if (dic->di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
> > - buf->bs_cowextsize = dic->di_cowextsize <<
> > - mp->m_sb.sb_blocklog;
> > + buf->bs_cowextsize_blks = dic->di_cowextsize;
> > }
> > switch (dic->di_format) {
> > @@ -259,6 +257,43 @@ xfs_bulkstat(
> > return error;
> > }
> > +/* Convert bulkstat (v5) to bstat (v1). */
> > +void
> > +xfs_bulkstat_to_bstat(
> > + struct xfs_mount *mp,
> > + struct xfs_bstat *bs1,
> > + const struct xfs_bulkstat *bstat)
> > +{
> > + bs1->bs_ino = bstat->bs_ino;
> > + bs1->bs_mode = bstat->bs_mode;
> > + bs1->bs_nlink = bstat->bs_nlink;
> > + bs1->bs_uid = bstat->bs_uid;
> > + bs1->bs_gid = bstat->bs_gid;
> > + bs1->bs_rdev = bstat->bs_rdev;
> > + bs1->bs_blksize = bstat->bs_blksize;
> > + bs1->bs_size = bstat->bs_size;
> > + bs1->bs_atime.tv_sec = bstat->bs_atime;
> > + bs1->bs_mtime.tv_sec = bstat->bs_mtime;
> > + bs1->bs_ctime.tv_sec = bstat->bs_ctime;
> > + bs1->bs_atime.tv_nsec = bstat->bs_atime_nsec;
> > + bs1->bs_mtime.tv_nsec = bstat->bs_mtime_nsec;
> > + bs1->bs_ctime.tv_nsec = bstat->bs_ctime_nsec;
> > + bs1->bs_blocks = bstat->bs_blocks;
> > + bs1->bs_xflags = bstat->bs_xflags;
> > + bs1->bs_extsize = bstat->bs_extsize_blks << mp->m_sb.sb_blocklog;
> > + bs1->bs_extents = bstat->bs_extents;
> > + bs1->bs_gen = bstat->bs_gen;
> > + bs1->bs_projid_lo = bstat->bs_projectid & 0xFFFF;
> > + bs1->bs_forkoff = bstat->bs_forkoff;
> > + bs1->bs_projid_hi = bstat->bs_projectid >> 16;
> > + bs1->bs_sick = bstat->bs_sick;
> > + bs1->bs_checked = bstat->bs_checked;
> > + bs1->bs_cowextsize = bstat->bs_cowextsize_blks << mp->m_sb.sb_blocklog;
> > + bs1->bs_dmevmask = 0;
> > + bs1->bs_dmstate = 0;
> > + bs1->bs_aextents = bstat->bs_aextents;
> > +}
> > +
> > struct xfs_inumbers_chunk {
> > inumbers_fmt_pf formatter;
> > struct xfs_ibulk *breq;
> > diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> > index b4c89454e27a..806069c9838c 100644
> > --- a/fs/xfs/xfs_itable.h
> > +++ b/fs/xfs/xfs_itable.h
> > @@ -41,10 +41,12 @@ xfs_ibulk_advance(
> > */
> > typedef int (*bulkstat_one_fmt_pf)(struct xfs_ibulk *breq,
> > - const struct xfs_bstat *bstat);
> > + const struct xfs_bulkstat *bstat);
> > int xfs_bulkstat_one(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
> > int xfs_bulkstat(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
> > +void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
> > + const struct xfs_bulkstat *bstat);
> > typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
> > const struct xfs_inogrp *igrp);
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index c8ba98fae30a..0b4cdda68524 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -146,6 +146,8 @@ xfs_check_ondisk_structs(void)
> > XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0);
> > XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0);
> > XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
> > +
> > + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> > }
> > #endif /* __XFS_ONDISK_H */
> >
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 3/9] xfs: introduce new v5 bulkstat structure
2019-06-06 20:17 ` Darrick J. Wong
@ 2019-06-06 22:41 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-06 22:41 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 6/6/19 1:17 PM, Darrick J. Wong wrote:
> On Wed, Jun 05, 2019 at 03:29:58PM -0700, Allison Collins wrote:
>>
>>
>> On 5/29/19 3:27 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Introduce a new version of the in-core bulkstat structure that supports
>>> our new v5 format features. This structure also fills the gaps in the
>>> previous structure. We leave wiring up the ioctls for the next patch.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> ---
>>> fs/xfs/libxfs/xfs_fs.h | 48 +++++++++++++++++++++++++++++++-
>>> fs/xfs/libxfs/xfs_health.h | 2 +
>>> fs/xfs/xfs_health.c | 2 +
>>> fs/xfs/xfs_ioctl.c | 9 ++++--
>>> fs/xfs/xfs_ioctl.h | 2 +
>>> fs/xfs/xfs_ioctl32.c | 10 +++++--
>>> fs/xfs/xfs_itable.c | 67 +++++++++++++++++++++++++++++++++-----------
>>> fs/xfs/xfs_itable.h | 4 ++-
>>> fs/xfs/xfs_ondisk.h | 2 +
>>> 9 files changed, 119 insertions(+), 27 deletions(-)
>>>
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
>>> index ef0dce229fa4..132e364eb141 100644
>>> --- a/fs/xfs/libxfs/xfs_fs.h
>>> +++ b/fs/xfs/libxfs/xfs_fs.h
>>> @@ -358,6 +358,52 @@ struct xfs_bstat {
>>> __u16 bs_aextents; /* attribute number of extents */
>>> };
>>> +/* New bulkstat structure that reports v5 features and fixes padding issues */
>>> +struct xfs_bulkstat {
>>> + uint64_t bs_ino; /* inode number */
>>> + uint64_t bs_size; /* file size */
>>> +
>>> + uint64_t bs_blocks; /* number of blocks */
>>> + uint64_t bs_xflags; /* extended flags */
>>> +
>>> + uint64_t bs_atime; /* access time, seconds */
>>> + uint64_t bs_mtime; /* modify time, seconds */
>>> +
>>> + uint64_t bs_ctime; /* inode change time, seconds */
>>> + uint64_t bs_btime; /* creation time, seconds */
>>> +
>>> + uint32_t bs_gen; /* generation count */
>>> + uint32_t bs_uid; /* user id */
>>> + uint32_t bs_gid; /* group id */
>>> + uint32_t bs_projectid; /* project id */
>>> +
>>> + uint32_t bs_atime_nsec; /* access time, nanoseconds */
>>> + uint32_t bs_mtime_nsec; /* modify time, nanoseconds */
>>> + uint32_t bs_ctime_nsec; /* inode change time, nanoseconds */
>>> + uint32_t bs_btime_nsec; /* creation time, nanoseconds */
>>> +
>>> + uint32_t bs_blksize; /* block size */
>>> + uint32_t bs_rdev; /* device value */
>>> + uint32_t bs_cowextsize_blks; /* cow extent size hint, blocks */
>>> + uint32_t bs_extsize_blks; /* extent size hint, blocks */
>>> +
>>> + uint32_t bs_nlink; /* number of links */
>>> + uint32_t bs_extents; /* number of extents */
>>> + uint32_t bs_aextents; /* attribute number of extents */
>>> + uint16_t bs_version; /* structure version */
>>> + uint16_t bs_forkoff; /* inode fork offset in bytes */
>>> +
>>> + uint16_t bs_sick; /* sick inode metadata */
>>> + uint16_t bs_checked; /* checked inode metadata */
>>> + uint16_t bs_mode; /* type and mode */
>>> + uint16_t bs_pad2; /* zeroed */
>>> +
>>> + uint64_t bs_pad[7]; /* zeroed */
>>> +};
>>> +
>>> +#define XFS_BULKSTAT_VERSION_V1 (1)
>>> +#define XFS_BULKSTAT_VERSION_V5 (5)
>>
>> Just a nit. I notice we have a V1 and V5, but I dont see the V1 being used
>> here or else where in the set. Was that intentional? Rest looks ok though
>
> There's no version field in the old struct xfs_bstat, so that's why I'm
> introducing a new flag now.
>
> Further down the road, xfsprogs will be converted to use a wrapper
> function to fill struct xfs_bulkstat. The wrapper will of course have
> to emulate the v5 call using the old ioctl when userspace is (somehow)
> newer than the kernel, and at that point the emulation will set
> bs_version = V1 to hint to its caller that it's running on an old kernel
> and therefore not to expect fields that weren't in the old version.
>
> --D
I see, that makes sense then. You can add my review for this patch and
patch 4. Thx!
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
>
>> Allison
>>
>>> +
>>> /* bs_sick flags */
>>> #define XFS_BS_SICK_INODE (1 << 0) /* inode core */
>>> #define XFS_BS_SICK_BMBTD (1 << 1) /* data fork */
>>> @@ -374,7 +420,7 @@ struct xfs_bstat {
>>> * to retain compatibility with "old" filesystems).
>>> */
>>> static inline uint32_t
>>> -bstat_get_projid(struct xfs_bstat *bs)
>>> +bstat_get_projid(const struct xfs_bstat *bs)
>>> {
>>> return (uint32_t)bs->bs_projid_hi << 16 | bs->bs_projid_lo;
>>> }
>>> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
>>> index 49ddfeac19f2..272005ac8c88 100644
>>> --- a/fs/xfs/libxfs/xfs_health.h
>>> +++ b/fs/xfs/libxfs/xfs_health.h
>>> @@ -185,6 +185,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
>>> void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
>>> void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
>>> -void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
>>> +void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bulkstat *bs);
>>> #endif /* __XFS_HEALTH_H__ */
>>> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
>>> index 4c4929f9e7bf..e8ba6034b0db 100644
>>> --- a/fs/xfs/xfs_health.c
>>> +++ b/fs/xfs/xfs_health.c
>>> @@ -373,7 +373,7 @@ static const struct ioctl_sick_map ino_map[] = {
>>> void
>>> xfs_bulkstat_health(
>>> struct xfs_inode *ip,
>>> - struct xfs_bstat *bs)
>>> + struct xfs_bulkstat *bs)
>>> {
>>> const struct ioctl_sick_map *m;
>>> unsigned int sick;
>>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>>> index f02a9bd757ad..0f70005cbe61 100644
>>> --- a/fs/xfs/xfs_ioctl.c
>>> +++ b/fs/xfs/xfs_ioctl.c
>>> @@ -724,10 +724,13 @@ xfs_ioc_space(
>>> /* Return 0 on success or positive error */
>>> int
>>> xfs_fsbulkstat_one_fmt(
>>> - struct xfs_ibulk *breq,
>>> - const struct xfs_bstat *bstat)
>>> + struct xfs_ibulk *breq,
>>> + const struct xfs_bulkstat *bstat)
>>> {
>>> - if (copy_to_user(breq->ubuffer, bstat, sizeof(*bstat)))
>>> + struct xfs_bstat bs1;
>>> +
>>> + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
>>> + if (copy_to_user(breq->ubuffer, &bs1, sizeof(bs1)))
>>> return -EFAULT;
>>> return xfs_ibulk_advance(breq, sizeof(struct xfs_bstat));
>>> }
>>> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
>>> index cb34bc821201..514d3028a134 100644
>>> --- a/fs/xfs/xfs_ioctl.h
>>> +++ b/fs/xfs/xfs_ioctl.h
>>> @@ -82,7 +82,7 @@ struct xfs_bstat;
>>> struct xfs_inogrp;
>>> int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
>>> - const struct xfs_bstat *bstat);
>>> + const struct xfs_bulkstat *bstat);
>>> int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
>>> #endif
>>> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
>>> index af753f2708e8..9806d27892df 100644
>>> --- a/fs/xfs/xfs_ioctl32.c
>>> +++ b/fs/xfs/xfs_ioctl32.c
>>> @@ -170,10 +170,14 @@ xfs_bstime_store_compat(
>>> /* Return 0 on success or positive error (to xfs_bulkstat()) */
>>> STATIC int
>>> xfs_fsbulkstat_one_fmt_compat(
>>> - struct xfs_ibulk *breq,
>>> - const struct xfs_bstat *buffer)
>>> + struct xfs_ibulk *breq,
>>> + const struct xfs_bulkstat *bstat)
>>> {
>>> - struct compat_xfs_bstat __user *p32 = breq->ubuffer;
>>> + struct compat_xfs_bstat __user *p32 = breq->ubuffer;
>>> + struct xfs_bstat bs1;
>>> + struct xfs_bstat *buffer = &bs1;
>>> +
>>> + xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
>>> if (put_user(buffer->bs_ino, &p32->bs_ino) ||
>>> put_user(buffer->bs_mode, &p32->bs_mode) ||
>>> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
>>> index bade54d6ac64..764b7f98fd5b 100644
>>> --- a/fs/xfs/xfs_itable.c
>>> +++ b/fs/xfs/xfs_itable.c
>>> @@ -25,7 +25,7 @@
>>> * Bulk Stat
>>> * =========
>>> *
>>> - * Use the inode walking functions to fill out struct xfs_bstat for every
>>> + * Use the inode walking functions to fill out struct xfs_bulkstat for every
>>> * allocated inode, then pass the stat information to some externally provided
>>> * iteration function.
>>> */
>>> @@ -62,7 +62,7 @@ xfs_bulkstat_one_int(
>>> struct xfs_icdinode *dic; /* dinode core info pointer */
>>> struct xfs_inode *ip; /* incore inode pointer */
>>> struct inode *inode;
>>> - struct xfs_bstat *buf; /* return buffer */
>>> + struct xfs_bulkstat *buf; /* return buffer */
>>> int error = 0; /* error value */
>>> if (xfs_internal_inum(mp, ino)) {
>>> @@ -91,37 +91,35 @@ xfs_bulkstat_one_int(
>>> /* xfs_iget returns the following without needing
>>> * further change.
>>> */
>>> - buf->bs_projid_lo = dic->di_projid_lo;
>>> - buf->bs_projid_hi = dic->di_projid_hi;
>>> + buf->bs_projectid = xfs_get_projid(ip);
>>> buf->bs_ino = ino;
>>> buf->bs_uid = dic->di_uid;
>>> buf->bs_gid = dic->di_gid;
>>> buf->bs_size = dic->di_size;
>>> buf->bs_nlink = inode->i_nlink;
>>> - buf->bs_atime.tv_sec = inode->i_atime.tv_sec;
>>> - buf->bs_atime.tv_nsec = inode->i_atime.tv_nsec;
>>> - buf->bs_mtime.tv_sec = inode->i_mtime.tv_sec;
>>> - buf->bs_mtime.tv_nsec = inode->i_mtime.tv_nsec;
>>> - buf->bs_ctime.tv_sec = inode->i_ctime.tv_sec;
>>> - buf->bs_ctime.tv_nsec = inode->i_ctime.tv_nsec;
>>> + buf->bs_atime = inode->i_atime.tv_sec;
>>> + buf->bs_atime_nsec = inode->i_atime.tv_nsec;
>>> + buf->bs_mtime = inode->i_mtime.tv_sec;
>>> + buf->bs_mtime_nsec = inode->i_mtime.tv_nsec;
>>> + buf->bs_ctime = inode->i_ctime.tv_sec;
>>> + buf->bs_ctime_nsec = inode->i_ctime.tv_nsec;
>>> + buf->bs_btime = dic->di_crtime.t_sec;
>>> + buf->bs_btime_nsec = dic->di_crtime.t_nsec;
>>> buf->bs_gen = inode->i_generation;
>>> buf->bs_mode = inode->i_mode;
>>> buf->bs_xflags = xfs_ip2xflags(ip);
>>> - buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
>>> + buf->bs_extsize_blks = dic->di_extsize;
>>> buf->bs_extents = dic->di_nextents;
>>> - memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
>>> xfs_bulkstat_health(ip, buf);
>>> - buf->bs_dmevmask = dic->di_dmevmask;
>>> - buf->bs_dmstate = dic->di_dmstate;
>>> buf->bs_aextents = dic->di_anextents;
>>> buf->bs_forkoff = XFS_IFORK_BOFF(ip);
>>> + buf->bs_version = XFS_BULKSTAT_VERSION_V5;
>>> if (dic->di_version == 3) {
>>> if (dic->di_flags2 & XFS_DIFLAG2_COWEXTSIZE)
>>> - buf->bs_cowextsize = dic->di_cowextsize <<
>>> - mp->m_sb.sb_blocklog;
>>> + buf->bs_cowextsize_blks = dic->di_cowextsize;
>>> }
>>> switch (dic->di_format) {
>>> @@ -259,6 +257,43 @@ xfs_bulkstat(
>>> return error;
>>> }
>>> +/* Convert bulkstat (v5) to bstat (v1). */
>>> +void
>>> +xfs_bulkstat_to_bstat(
>>> + struct xfs_mount *mp,
>>> + struct xfs_bstat *bs1,
>>> + const struct xfs_bulkstat *bstat)
>>> +{
>>> + bs1->bs_ino = bstat->bs_ino;
>>> + bs1->bs_mode = bstat->bs_mode;
>>> + bs1->bs_nlink = bstat->bs_nlink;
>>> + bs1->bs_uid = bstat->bs_uid;
>>> + bs1->bs_gid = bstat->bs_gid;
>>> + bs1->bs_rdev = bstat->bs_rdev;
>>> + bs1->bs_blksize = bstat->bs_blksize;
>>> + bs1->bs_size = bstat->bs_size;
>>> + bs1->bs_atime.tv_sec = bstat->bs_atime;
>>> + bs1->bs_mtime.tv_sec = bstat->bs_mtime;
>>> + bs1->bs_ctime.tv_sec = bstat->bs_ctime;
>>> + bs1->bs_atime.tv_nsec = bstat->bs_atime_nsec;
>>> + bs1->bs_mtime.tv_nsec = bstat->bs_mtime_nsec;
>>> + bs1->bs_ctime.tv_nsec = bstat->bs_ctime_nsec;
>>> + bs1->bs_blocks = bstat->bs_blocks;
>>> + bs1->bs_xflags = bstat->bs_xflags;
>>> + bs1->bs_extsize = bstat->bs_extsize_blks << mp->m_sb.sb_blocklog;
>>> + bs1->bs_extents = bstat->bs_extents;
>>> + bs1->bs_gen = bstat->bs_gen;
>>> + bs1->bs_projid_lo = bstat->bs_projectid & 0xFFFF;
>>> + bs1->bs_forkoff = bstat->bs_forkoff;
>>> + bs1->bs_projid_hi = bstat->bs_projectid >> 16;
>>> + bs1->bs_sick = bstat->bs_sick;
>>> + bs1->bs_checked = bstat->bs_checked;
>>> + bs1->bs_cowextsize = bstat->bs_cowextsize_blks << mp->m_sb.sb_blocklog;
>>> + bs1->bs_dmevmask = 0;
>>> + bs1->bs_dmstate = 0;
>>> + bs1->bs_aextents = bstat->bs_aextents;
>>> +}
>>> +
>>> struct xfs_inumbers_chunk {
>>> inumbers_fmt_pf formatter;
>>> struct xfs_ibulk *breq;
>>> diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
>>> index b4c89454e27a..806069c9838c 100644
>>> --- a/fs/xfs/xfs_itable.h
>>> +++ b/fs/xfs/xfs_itable.h
>>> @@ -41,10 +41,12 @@ xfs_ibulk_advance(
>>> */
>>> typedef int (*bulkstat_one_fmt_pf)(struct xfs_ibulk *breq,
>>> - const struct xfs_bstat *bstat);
>>> + const struct xfs_bulkstat *bstat);
>>> int xfs_bulkstat_one(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
>>> int xfs_bulkstat(struct xfs_ibulk *breq, bulkstat_one_fmt_pf formatter);
>>> +void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
>>> + const struct xfs_bulkstat *bstat);
>>> typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
>>> const struct xfs_inogrp *igrp);
>>> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
>>> index c8ba98fae30a..0b4cdda68524 100644
>>> --- a/fs/xfs/xfs_ondisk.h
>>> +++ b/fs/xfs/xfs_ondisk.h
>>> @@ -146,6 +146,8 @@ xfs_check_ondisk_structs(void)
>>> XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0);
>>> XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0);
>>> XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
>>> +
>>> + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
>>> }
>>> #endif /* __XFS_ONDISK_H */
>>>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 4/9] xfs: introduce v5 inode group structure
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (2 preceding siblings ...)
2019-05-29 22:27 ` [PATCH 3/9] xfs: introduce new v5 bulkstat structure Darrick J. Wong
@ 2019-05-29 22:27 ` Darrick J. Wong
2019-06-05 22:30 ` Allison Collins
2019-05-29 22:28 ` [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls Darrick J. Wong
` (4 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:27 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Introduce a new "v5" inode group structure that fixes the alignment
and padding problems of the existing structure.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 11 +++++++++++
fs/xfs/xfs_ioctl.c | 9 ++++++---
fs/xfs/xfs_ioctl.h | 2 +-
fs/xfs/xfs_ioctl32.c | 10 +++++++---
fs/xfs/xfs_itable.c | 14 +++++++++++++-
fs/xfs/xfs_itable.h | 3 ++-
fs/xfs/xfs_ondisk.h | 1 +
7 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 132e364eb141..8b8fe78511fb 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -445,6 +445,17 @@ struct xfs_inogrp {
__u64 xi_allocmask; /* mask of allocated inodes */
};
+/* New inumbers structure that reports v5 features and fixes padding issues */
+struct xfs_inumbers {
+ uint64_t xi_startino; /* starting inode number */
+ uint64_t xi_allocmask; /* mask of allocated inodes */
+ uint8_t xi_alloccount; /* # bits set in allocmask */
+ uint8_t xi_version; /* version */
+ uint8_t xi_padding[6]; /* zero */
+};
+
+#define XFS_INUMBERS_VERSION_V1 (1)
+#define XFS_INUMBERS_VERSION_V5 (5)
/*
* Error injection.
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 0f70005cbe61..e43ad688e683 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -737,10 +737,13 @@ xfs_fsbulkstat_one_fmt(
int
xfs_fsinumbers_fmt(
- struct xfs_ibulk *breq,
- const struct xfs_inogrp *igrp)
+ struct xfs_ibulk *breq,
+ const struct xfs_inumbers *igrp)
{
- if (copy_to_user(breq->ubuffer, igrp, sizeof(*igrp)))
+ struct xfs_inogrp ig1;
+
+ xfs_inumbers_to_inogrp(&ig1, igrp);
+ if (copy_to_user(breq->ubuffer, &ig1, sizeof(struct xfs_inogrp)))
return -EFAULT;
return xfs_ibulk_advance(breq, sizeof(struct xfs_inogrp));
}
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index 514d3028a134..654c0bb1bcf8 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -83,6 +83,6 @@ struct xfs_inogrp;
int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
const struct xfs_bulkstat *bstat);
-int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
+int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inumbers *igrp);
#endif
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 9806d27892df..bfe71747776b 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -85,10 +85,14 @@ xfs_compat_growfs_rt_copyin(
STATIC int
xfs_fsinumbers_fmt_compat(
- struct xfs_ibulk *breq,
- const struct xfs_inogrp *igrp)
+ struct xfs_ibulk *breq,
+ const struct xfs_inumbers *ig)
{
- struct compat_xfs_inogrp __user *p32 = breq->ubuffer;
+ struct compat_xfs_inogrp __user *p32 = breq->ubuffer;
+ struct xfs_inogrp ig1;
+ struct xfs_inogrp *igrp = &ig1;
+
+ xfs_inumbers_to_inogrp(&ig1, ig);
if (put_user(igrp->xi_startino, &p32->xi_startino) ||
put_user(igrp->xi_alloccount, &p32->xi_alloccount) ||
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 764b7f98fd5b..8701596976bb 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -322,10 +322,11 @@ xfs_inumbers_walk(
const struct xfs_inobt_rec_incore *irec,
void *data)
{
- struct xfs_inogrp inogrp = {
+ struct xfs_inumbers inogrp = {
.xi_startino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino),
.xi_alloccount = irec->ir_count - irec->ir_freecount,
.xi_allocmask = ~irec->ir_free,
+ .xi_version = XFS_INUMBERS_VERSION_V5,
};
struct xfs_inumbers_chunk *ic = data;
xfs_agino_t agino;
@@ -376,3 +377,14 @@ xfs_inumbers(
return error;
}
+
+/* Convert an inumbers (v5) struct to a inogrp (v1) struct. */
+void
+xfs_inumbers_to_inogrp(
+ struct xfs_inogrp *ig1,
+ const struct xfs_inumbers *ig)
+{
+ ig1->xi_startino = ig->xi_startino;
+ ig1->xi_alloccount = ig->xi_alloccount;
+ ig1->xi_allocmask = ig->xi_allocmask;
+}
diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
index 806069c9838c..2987f3eb335f 100644
--- a/fs/xfs/xfs_itable.h
+++ b/fs/xfs/xfs_itable.h
@@ -49,8 +49,9 @@ void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
const struct xfs_bulkstat *bstat);
typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
- const struct xfs_inogrp *igrp);
+ const struct xfs_inumbers *igrp);
int xfs_inumbers(struct xfs_ibulk *breq, inumbers_fmt_pf formatter);
+void xfs_inumbers_to_inogrp(struct xfs_inogrp *ig1, const struct xfs_inumbers *ig);
#endif /* __XFS_ITABLE_H__ */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 0b4cdda68524..d8f941b4d51c 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -148,6 +148,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
+ XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
}
#endif /* __XFS_ONDISK_H */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 4/9] xfs: introduce v5 inode group structure
2019-05-29 22:27 ` [PATCH 4/9] xfs: introduce v5 inode group structure Darrick J. Wong
@ 2019-06-05 22:30 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Introduce a new "v5" inode group structure that fixes the alignment
> and padding problems of the existing structure.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 11 +++++++++++
> fs/xfs/xfs_ioctl.c | 9 ++++++---
> fs/xfs/xfs_ioctl.h | 2 +-
> fs/xfs/xfs_ioctl32.c | 10 +++++++---
> fs/xfs/xfs_itable.c | 14 +++++++++++++-
> fs/xfs/xfs_itable.h | 3 ++-
> fs/xfs/xfs_ondisk.h | 1 +
> 7 files changed, 41 insertions(+), 9 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 132e364eb141..8b8fe78511fb 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -445,6 +445,17 @@ struct xfs_inogrp {
> __u64 xi_allocmask; /* mask of allocated inodes */
> };
>
> +/* New inumbers structure that reports v5 features and fixes padding issues */
> +struct xfs_inumbers {
> + uint64_t xi_startino; /* starting inode number */
> + uint64_t xi_allocmask; /* mask of allocated inodes */
> + uint8_t xi_alloccount; /* # bits set in allocmask */
> + uint8_t xi_version; /* version */
> + uint8_t xi_padding[6]; /* zero */
> +};
> +
> +#define XFS_INUMBERS_VERSION_V1 (1)
> +#define XFS_INUMBERS_VERSION_V5 (5)
Ditto on the V1 not being used here too. Other than that it, this one
looks ok too
Allison
>
> /*
> * Error injection.
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 0f70005cbe61..e43ad688e683 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -737,10 +737,13 @@ xfs_fsbulkstat_one_fmt(
>
> int
> xfs_fsinumbers_fmt(
> - struct xfs_ibulk *breq,
> - const struct xfs_inogrp *igrp)
> + struct xfs_ibulk *breq,
> + const struct xfs_inumbers *igrp)
> {
> - if (copy_to_user(breq->ubuffer, igrp, sizeof(*igrp)))
> + struct xfs_inogrp ig1;
> +
> + xfs_inumbers_to_inogrp(&ig1, igrp);
> + if (copy_to_user(breq->ubuffer, &ig1, sizeof(struct xfs_inogrp)))
> return -EFAULT;
> return xfs_ibulk_advance(breq, sizeof(struct xfs_inogrp));
> }
> diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
> index 514d3028a134..654c0bb1bcf8 100644
> --- a/fs/xfs/xfs_ioctl.h
> +++ b/fs/xfs/xfs_ioctl.h
> @@ -83,6 +83,6 @@ struct xfs_inogrp;
>
> int xfs_fsbulkstat_one_fmt(struct xfs_ibulk *breq,
> const struct xfs_bulkstat *bstat);
> -int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inogrp *igrp);
> +int xfs_fsinumbers_fmt(struct xfs_ibulk *breq, const struct xfs_inumbers *igrp);
>
> #endif
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 9806d27892df..bfe71747776b 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -85,10 +85,14 @@ xfs_compat_growfs_rt_copyin(
>
> STATIC int
> xfs_fsinumbers_fmt_compat(
> - struct xfs_ibulk *breq,
> - const struct xfs_inogrp *igrp)
> + struct xfs_ibulk *breq,
> + const struct xfs_inumbers *ig)
> {
> - struct compat_xfs_inogrp __user *p32 = breq->ubuffer;
> + struct compat_xfs_inogrp __user *p32 = breq->ubuffer;
> + struct xfs_inogrp ig1;
> + struct xfs_inogrp *igrp = &ig1;
> +
> + xfs_inumbers_to_inogrp(&ig1, ig);
>
> if (put_user(igrp->xi_startino, &p32->xi_startino) ||
> put_user(igrp->xi_alloccount, &p32->xi_alloccount) ||
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 764b7f98fd5b..8701596976bb 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -322,10 +322,11 @@ xfs_inumbers_walk(
> const struct xfs_inobt_rec_incore *irec,
> void *data)
> {
> - struct xfs_inogrp inogrp = {
> + struct xfs_inumbers inogrp = {
> .xi_startino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino),
> .xi_alloccount = irec->ir_count - irec->ir_freecount,
> .xi_allocmask = ~irec->ir_free,
> + .xi_version = XFS_INUMBERS_VERSION_V5,
> };
> struct xfs_inumbers_chunk *ic = data;
> xfs_agino_t agino;
> @@ -376,3 +377,14 @@ xfs_inumbers(
>
> return error;
> }
> +
> +/* Convert an inumbers (v5) struct to a inogrp (v1) struct. */
> +void
> +xfs_inumbers_to_inogrp(
> + struct xfs_inogrp *ig1,
> + const struct xfs_inumbers *ig)
> +{
> + ig1->xi_startino = ig->xi_startino;
> + ig1->xi_alloccount = ig->xi_alloccount;
> + ig1->xi_allocmask = ig->xi_allocmask;
> +}
> diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> index 806069c9838c..2987f3eb335f 100644
> --- a/fs/xfs/xfs_itable.h
> +++ b/fs/xfs/xfs_itable.h
> @@ -49,8 +49,9 @@ void xfs_bulkstat_to_bstat(struct xfs_mount *mp, struct xfs_bstat *bs1,
> const struct xfs_bulkstat *bstat);
>
> typedef int (*inumbers_fmt_pf)(struct xfs_ibulk *breq,
> - const struct xfs_inogrp *igrp);
> + const struct xfs_inumbers *igrp);
>
> int xfs_inumbers(struct xfs_ibulk *breq, inumbers_fmt_pf formatter);
> +void xfs_inumbers_to_inogrp(struct xfs_inogrp *ig1, const struct xfs_inumbers *ig);
>
> #endif /* __XFS_ITABLE_H__ */
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 0b4cdda68524..d8f941b4d51c 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -148,6 +148,7 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0);
>
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> }
>
> #endif /* __XFS_ONDISK_H */
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (3 preceding siblings ...)
2019-05-29 22:27 ` [PATCH 4/9] xfs: introduce v5 inode group structure Darrick J. Wong
@ 2019-05-29 22:28 ` Darrick J. Wong
2019-06-05 22:30 ` Allison Collins
2019-05-29 22:28 ` [PATCH 6/9] xfs: wire up the new v5 bulkstat_single ioctl Darrick J. Wong
` (3 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:28 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Wire up the new v5 BULKSTAT ioctl and rename the old one to V1.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 24 +++++++++++
fs/xfs/xfs_ioctl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_ioctl32.c | 1
fs/xfs/xfs_ondisk.h | 1
4 files changed, 129 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 8b8fe78511fb..960f3542e207 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -435,7 +435,6 @@ struct xfs_fsop_bulkreq {
__s32 __user *ocount; /* output count pointer */
};
-
/*
* Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
*/
@@ -457,6 +456,28 @@ struct xfs_inumbers {
#define XFS_INUMBERS_VERSION_V1 (1)
#define XFS_INUMBERS_VERSION_V5 (5)
+/* Header for bulk inode requests. */
+struct xfs_bulk_ireq {
+ uint64_t ino; /* I/O: start with this inode */
+ uint32_t flags; /* I/O: operation flags */
+ uint32_t icount; /* I: count of entries in buffer */
+ uint32_t ocount; /* O: count of entries filled out */
+ uint32_t reserved32; /* must be zero */
+ uint64_t reserved[5]; /* must be zero */
+};
+
+#define XFS_BULK_IREQ_FLAGS_ALL (0)
+
+/*
+ * ioctl structures for v5 bulkstat and inumbers requests
+ */
+struct xfs_bulkstat_req {
+ struct xfs_bulk_ireq hdr;
+ struct xfs_bulkstat bulkstat[];
+};
+#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
+ (nr) * sizeof(struct xfs_bulkstat))
+
/*
* Error injection.
*/
@@ -758,6 +779,7 @@ struct xfs_scrub_metadata {
#define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
#define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
#define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
+#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
/* XFS_IOC_GETFSUUID ---------- deprecated 140 */
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index e43ad688e683..f6724c75ba97 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -822,6 +822,107 @@ xfs_ioc_fsbulkstat(
return 0;
}
+/* Return 0 on success or positive error */
+static int
+xfs_bulkstat_fmt(
+ struct xfs_ibulk *breq,
+ const struct xfs_bulkstat *bstat)
+{
+ if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
+ return -EFAULT;
+ return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
+}
+
+/*
+ * Check the incoming bulk request @hdr from userspace and initialize the
+ * internal @breq bulk request appropriately. Returns 0 if the bulk request
+ * should proceed; 1 if there's nothing to do; or the usual negative error
+ * code.
+ */
+static int
+xfs_bulk_ireq_setup(
+ struct xfs_mount *mp,
+ struct xfs_bulk_ireq *hdr,
+ struct xfs_ibulk *breq,
+ void __user *ubuffer)
+{
+ if (hdr->icount == 0 ||
+ (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
+ hdr->reserved32 ||
+ memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+ return -EINVAL;
+
+ if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
+ goto no_results;
+
+ breq->ubuffer = ubuffer;
+ breq->icount = hdr->icount;
+ breq->startino = hdr->ino;
+ return 0;
+no_results:
+ hdr->ocount = 0;
+ return 1;
+}
+
+/*
+ * Update the userspace bulk request @hdr to reflect the end state of the
+ * internal bulk request @breq. If @error is negative then we return just
+ * that; otherwise (@error is 0 or 1) we copy the state so that userspace
+ * can discover what happened.
+ */
+static int
+xfs_bulk_ireq_teardown(
+ struct xfs_bulk_ireq *hdr,
+ struct xfs_ibulk *breq,
+ int error)
+{
+ if (error < 0)
+ return error;
+
+ hdr->ino = breq->startino;
+ hdr->ocount = breq->ocount;
+ return 0;
+}
+
+/* Handle the v5 bulkstat ioctl. */
+STATIC int
+xfs_ioc_bulkstat(
+ struct xfs_mount *mp,
+ unsigned int cmd,
+ struct xfs_bulkstat_req __user *arg)
+{
+ struct xfs_bulk_ireq hdr;
+ struct xfs_ibulk breq = {
+ .mp = mp,
+ };
+ int error;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->bulkstat);
+ if (error < 0)
+ return error;
+
+ if (!error)
+ error = xfs_bulkstat(&breq, xfs_bulkstat_fmt);
+
+ error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
+ if (error)
+ return error;
+
+ if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ return 0;
+}
+
STATIC int
xfs_ioc_fsgeometry(
struct xfs_mount *mp,
@@ -1986,6 +2087,9 @@ xfs_file_ioctl(
case XFS_IOC_FSINUMBERS:
return xfs_ioc_fsbulkstat(mp, cmd, arg);
+ case XFS_IOC_BULKSTAT:
+ return xfs_ioc_bulkstat(mp, cmd, arg);
+
case XFS_IOC_FSGEOMETRY_V1:
return xfs_ioc_fsgeometry(mp, arg, 3);
case XFS_IOC_FSGEOMETRY_V4:
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index bfe71747776b..84c342be4536 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -576,6 +576,7 @@ xfs_file_compat_ioctl(
case XFS_IOC_ERROR_CLEARALL:
case FS_IOC_GETFSMAP:
case XFS_IOC_SCRUB_METADATA:
+ case XFS_IOC_BULKSTAT:
return xfs_file_ioctl(filp, cmd, p);
#if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
/*
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index d8f941b4d51c..954484c6eb96 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -149,6 +149,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
+ XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
}
#endif /* __XFS_ONDISK_H */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls
2019-05-29 22:28 ` [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls Darrick J. Wong
@ 2019-06-05 22:30 ` Allison Collins
2019-06-06 21:10 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Wire up the new v5 BULKSTAT ioctl and rename the old one to V1.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 24 +++++++++++
> fs/xfs/xfs_ioctl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_ioctl32.c | 1
> fs/xfs/xfs_ondisk.h | 1
> 4 files changed, 129 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 8b8fe78511fb..960f3542e207 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -435,7 +435,6 @@ struct xfs_fsop_bulkreq {
> __s32 __user *ocount; /* output count pointer */
> };
>
> -
> /*
> * Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
> */
> @@ -457,6 +456,28 @@ struct xfs_inumbers {
> #define XFS_INUMBERS_VERSION_V1 (1)
> #define XFS_INUMBERS_VERSION_V5 (5)
>
> +/* Header for bulk inode requests. */
> +struct xfs_bulk_ireq {
> + uint64_t ino; /* I/O: start with this inode */
> + uint32_t flags; /* I/O: operation flags */
> + uint32_t icount; /* I: count of entries in buffer */
> + uint32_t ocount; /* O: count of entries filled out */
> + uint32_t reserved32; /* must be zero */
> + uint64_t reserved[5]; /* must be zero */
> +};
> +
> +#define XFS_BULK_IREQ_FLAGS_ALL (0)
> +
> +/*
> + * ioctl structures for v5 bulkstat and inumbers requests
> + */
> +struct xfs_bulkstat_req {
> + struct xfs_bulk_ireq hdr;
> + struct xfs_bulkstat bulkstat[];
> +};
> +#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
> + (nr) * sizeof(struct xfs_bulkstat))
> +
> /*
> * Error injection.
> */
> @@ -758,6 +779,7 @@ struct xfs_scrub_metadata {
> #define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
> #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> +#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
>
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index e43ad688e683..f6724c75ba97 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -822,6 +822,107 @@ xfs_ioc_fsbulkstat(
> return 0;
> }
>
> +/* Return 0 on success or positive error */
> +static int
> +xfs_bulkstat_fmt(
> + struct xfs_ibulk *breq,
> + const struct xfs_bulkstat *bstat)
> +{
> + if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
> + return -EFAULT;
> + return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
> +}
> +
> +/*
> + * Check the incoming bulk request @hdr from userspace and initialize the
> + * internal @breq bulk request appropriately. Returns 0 if the bulk request
> + * should proceed; 1 if there's nothing to do; or the usual negative error
> + * code.
> + */
> +static int
> +xfs_bulk_ireq_setup(
> + struct xfs_mount *mp,
> + struct xfs_bulk_ireq *hdr,
> + struct xfs_ibulk *breq,
> + void __user *ubuffer)
> +{
> + if (hdr->icount == 0 ||
> + (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
> + hdr->reserved32 ||
> + memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> + return -EINVAL;
> +
> + if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> + goto no_results;
> +
> + breq->ubuffer = ubuffer;
> + breq->icount = hdr->icount;
> + breq->startino = hdr->ino;
> + return 0;
> +no_results:
> + hdr->ocount = 0;
> + return 1;
> +}
> +
> +/*
> + * Update the userspace bulk request @hdr to reflect the end state of the
> + * internal bulk request @breq. If @error is negative then we return just
> + * that; otherwise (@error is 0 or 1) we copy the state so that userspace
> + * can discover what happened.
> + */
> +static int
> +xfs_bulk_ireq_teardown(
> + struct xfs_bulk_ireq *hdr,
> + struct xfs_ibulk *breq,
> + int error)
> +{
> + if (error < 0)
> + return error;
Hmm, passing in error just to bail on error seemed a little out of scope
to me. Is there a reason we're doing it here? Instead of after the
preceding call made in the caller? Referenced below.....
> +
> + hdr->ino = breq->startino;
> + hdr->ocount = breq->ocount;
> + return 0;
> +}
> +
> +/* Handle the v5 bulkstat ioctl. */
> +STATIC int
> +xfs_ioc_bulkstat(
> + struct xfs_mount *mp,
> + unsigned int cmd,
> + struct xfs_bulkstat_req __user *arg)
> +{
> + struct xfs_bulk_ireq hdr;
> + struct xfs_ibulk breq = {
> + .mp = mp,
> + };
> + int error;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return -EIO;
> +
> + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->bulkstat);
> + if (error < 0)
> + return error;
> +
> + if (!error)
> + error = xfs_bulkstat(&breq, xfs_bulkstat_fmt);
> +
Right here. How about
if (error < 0)
return error;
It seems functionally equivalent. If error < 0, teardown will
bounce it back anyway and then the error check below will toss it back
up. Is that what you meant?
Allison
> + error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
> + if (error)
> + return error;
> +
> + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> STATIC int
> xfs_ioc_fsgeometry(
> struct xfs_mount *mp,
> @@ -1986,6 +2087,9 @@ xfs_file_ioctl(
> case XFS_IOC_FSINUMBERS:
> return xfs_ioc_fsbulkstat(mp, cmd, arg);
>
> + case XFS_IOC_BULKSTAT:
> + return xfs_ioc_bulkstat(mp, cmd, arg);
> +
> case XFS_IOC_FSGEOMETRY_V1:
> return xfs_ioc_fsgeometry(mp, arg, 3);
> case XFS_IOC_FSGEOMETRY_V4:
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index bfe71747776b..84c342be4536 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -576,6 +576,7 @@ xfs_file_compat_ioctl(
> case XFS_IOC_ERROR_CLEARALL:
> case FS_IOC_GETFSMAP:
> case XFS_IOC_SCRUB_METADATA:
> + case XFS_IOC_BULKSTAT:
> return xfs_file_ioctl(filp, cmd, p);
> #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
> /*
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index d8f941b4d51c..954484c6eb96 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -149,6 +149,7 @@ xfs_check_ondisk_structs(void)
>
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
> }
>
> #endif /* __XFS_ONDISK_H */
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls
2019-06-05 22:30 ` Allison Collins
@ 2019-06-06 21:10 ` Darrick J. Wong
2019-06-06 22:37 ` Allison Collins
0 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-06-06 21:10 UTC (permalink / raw)
To: Allison Collins; +Cc: linux-xfs
On Wed, Jun 05, 2019 at 03:30:21PM -0700, Allison Collins wrote:
> On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Wire up the new v5 BULKSTAT ioctl and rename the old one to V1.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_fs.h | 24 +++++++++++
> > fs/xfs/xfs_ioctl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/xfs_ioctl32.c | 1
> > fs/xfs/xfs_ondisk.h | 1
> > 4 files changed, 129 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 8b8fe78511fb..960f3542e207 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -435,7 +435,6 @@ struct xfs_fsop_bulkreq {
> > __s32 __user *ocount; /* output count pointer */
> > };
> > -
> > /*
> > * Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
> > */
> > @@ -457,6 +456,28 @@ struct xfs_inumbers {
> > #define XFS_INUMBERS_VERSION_V1 (1)
> > #define XFS_INUMBERS_VERSION_V5 (5)
> > +/* Header for bulk inode requests. */
> > +struct xfs_bulk_ireq {
> > + uint64_t ino; /* I/O: start with this inode */
> > + uint32_t flags; /* I/O: operation flags */
> > + uint32_t icount; /* I: count of entries in buffer */
> > + uint32_t ocount; /* O: count of entries filled out */
> > + uint32_t reserved32; /* must be zero */
> > + uint64_t reserved[5]; /* must be zero */
> > +};
> > +
> > +#define XFS_BULK_IREQ_FLAGS_ALL (0)
> > +
> > +/*
> > + * ioctl structures for v5 bulkstat and inumbers requests
> > + */
> > +struct xfs_bulkstat_req {
> > + struct xfs_bulk_ireq hdr;
> > + struct xfs_bulkstat bulkstat[];
> > +};
> > +#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
> > + (nr) * sizeof(struct xfs_bulkstat))
> > +
> > /*
> > * Error injection.
> > */
> > @@ -758,6 +779,7 @@ struct xfs_scrub_metadata {
> > #define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
> > #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> > #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> > +#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> > /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index e43ad688e683..f6724c75ba97 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -822,6 +822,107 @@ xfs_ioc_fsbulkstat(
> > return 0;
> > }
> > +/* Return 0 on success or positive error */
> > +static int
> > +xfs_bulkstat_fmt(
> > + struct xfs_ibulk *breq,
> > + const struct xfs_bulkstat *bstat)
> > +{
> > + if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
> > + return -EFAULT;
> > + return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
> > +}
> > +
> > +/*
> > + * Check the incoming bulk request @hdr from userspace and initialize the
> > + * internal @breq bulk request appropriately. Returns 0 if the bulk request
> > + * should proceed; 1 if there's nothing to do; or the usual negative error
> > + * code.
> > + */
> > +static int
> > +xfs_bulk_ireq_setup(
> > + struct xfs_mount *mp,
> > + struct xfs_bulk_ireq *hdr,
> > + struct xfs_ibulk *breq,
> > + void __user *ubuffer)
> > +{
> > + if (hdr->icount == 0 ||
> > + (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
> > + hdr->reserved32 ||
> > + memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> > + return -EINVAL;
> > +
> > + if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> > + goto no_results;
> > +
> > + breq->ubuffer = ubuffer;
> > + breq->icount = hdr->icount;
> > + breq->startino = hdr->ino;
> > + return 0;
> > +no_results:
> > + hdr->ocount = 0;
> > + return 1;
> > +}
> > +
> > +/*
> > + * Update the userspace bulk request @hdr to reflect the end state of the
> > + * internal bulk request @breq. If @error is negative then we return just
> > + * that; otherwise (@error is 0 or 1) we copy the state so that userspace
> > + * can discover what happened.
> > + */
> > +static int
> > +xfs_bulk_ireq_teardown(
> > + struct xfs_bulk_ireq *hdr,
> > + struct xfs_ibulk *breq,
> > + int error)
> > +{
> > + if (error < 0)
> > + return error;
> Hmm, passing in error just to bail on error seemed a little out of scope to
> me. Is there a reason we're doing it here? Instead of after the preceding
> call made in the caller? Referenced below.....
>
> > +
> > + hdr->ino = breq->startino;
> > + hdr->ocount = breq->ocount;
> > + return 0;
> > +}
> > +
> > +/* Handle the v5 bulkstat ioctl. */
> > +STATIC int
> > +xfs_ioc_bulkstat(
> > + struct xfs_mount *mp,
> > + unsigned int cmd,
> > + struct xfs_bulkstat_req __user *arg)
> > +{
> > + struct xfs_bulk_ireq hdr;
> > + struct xfs_ibulk breq = {
> > + .mp = mp,
> > + };
> > + int error;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + if (XFS_FORCED_SHUTDOWN(mp))
> > + return -EIO;
> > +
> > + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
> > + return -EFAULT;
> > +
> > + error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->bulkstat);
> > + if (error < 0)
> > + return error;
> > +
> > + if (!error)
> > + error = xfs_bulkstat(&breq, xfs_bulkstat_fmt);
> > +
> Right here. How about
>
> if (error < 0)
> return error;
>
> It seems functionally equivalent. If error < 0, teardown will
> bounce it back anyway and then the error check below will toss it back up.
> Is that what you meant?
Yeah, I could do that too. :)
TBH I only threw it in as a helper function because xfs_bulk_ireq_setup
seemed to need a counterpart; it's such a short function that I could
just opencode it... Thoughts?
--D
> Allison
>
> > + error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
> > + if (error)
> > + return error;
> > +
> > + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > STATIC int
> > xfs_ioc_fsgeometry(
> > struct xfs_mount *mp,
> > @@ -1986,6 +2087,9 @@ xfs_file_ioctl(
> > case XFS_IOC_FSINUMBERS:
> > return xfs_ioc_fsbulkstat(mp, cmd, arg);
> > + case XFS_IOC_BULKSTAT:
> > + return xfs_ioc_bulkstat(mp, cmd, arg);
> > +
> > case XFS_IOC_FSGEOMETRY_V1:
> > return xfs_ioc_fsgeometry(mp, arg, 3);
> > case XFS_IOC_FSGEOMETRY_V4:
> > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > index bfe71747776b..84c342be4536 100644
> > --- a/fs/xfs/xfs_ioctl32.c
> > +++ b/fs/xfs/xfs_ioctl32.c
> > @@ -576,6 +576,7 @@ xfs_file_compat_ioctl(
> > case XFS_IOC_ERROR_CLEARALL:
> > case FS_IOC_GETFSMAP:
> > case XFS_IOC_SCRUB_METADATA:
> > + case XFS_IOC_BULKSTAT:
> > return xfs_file_ioctl(filp, cmd, p);
> > #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
> > /*
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index d8f941b4d51c..954484c6eb96 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -149,6 +149,7 @@ xfs_check_ondisk_structs(void)
> > XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> > XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> > + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
> > }
> > #endif /* __XFS_ONDISK_H */
> >
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls
2019-06-06 21:10 ` Darrick J. Wong
@ 2019-06-06 22:37 ` Allison Collins
2019-06-07 16:10 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-06-06 22:37 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 6/6/19 2:10 PM, Darrick J. Wong wrote:
> On Wed, Jun 05, 2019 at 03:30:21PM -0700, Allison Collins wrote:
>> On 5/29/19 3:28 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Wire up the new v5 BULKSTAT ioctl and rename the old one to V1.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> ---
>>> fs/xfs/libxfs/xfs_fs.h | 24 +++++++++++
>>> fs/xfs/xfs_ioctl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> fs/xfs/xfs_ioctl32.c | 1
>>> fs/xfs/xfs_ondisk.h | 1
>>> 4 files changed, 129 insertions(+), 1 deletion(-)
>>>
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
>>> index 8b8fe78511fb..960f3542e207 100644
>>> --- a/fs/xfs/libxfs/xfs_fs.h
>>> +++ b/fs/xfs/libxfs/xfs_fs.h
>>> @@ -435,7 +435,6 @@ struct xfs_fsop_bulkreq {
>>> __s32 __user *ocount; /* output count pointer */
>>> };
>>> -
>>> /*
>>> * Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
>>> */
>>> @@ -457,6 +456,28 @@ struct xfs_inumbers {
>>> #define XFS_INUMBERS_VERSION_V1 (1)
>>> #define XFS_INUMBERS_VERSION_V5 (5)
>>> +/* Header for bulk inode requests. */
>>> +struct xfs_bulk_ireq {
>>> + uint64_t ino; /* I/O: start with this inode */
>>> + uint32_t flags; /* I/O: operation flags */
>>> + uint32_t icount; /* I: count of entries in buffer */
>>> + uint32_t ocount; /* O: count of entries filled out */
>>> + uint32_t reserved32; /* must be zero */
>>> + uint64_t reserved[5]; /* must be zero */
>>> +};
>>> +
>>> +#define XFS_BULK_IREQ_FLAGS_ALL (0)
>>> +
>>> +/*
>>> + * ioctl structures for v5 bulkstat and inumbers requests
>>> + */
>>> +struct xfs_bulkstat_req {
>>> + struct xfs_bulk_ireq hdr;
>>> + struct xfs_bulkstat bulkstat[];
>>> +};
>>> +#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
>>> + (nr) * sizeof(struct xfs_bulkstat))
>>> +
>>> /*
>>> * Error injection.
>>> */
>>> @@ -758,6 +779,7 @@ struct xfs_scrub_metadata {
>>> #define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
>>> #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
>>> #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
>>> +#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
>>> /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
>>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>>> index e43ad688e683..f6724c75ba97 100644
>>> --- a/fs/xfs/xfs_ioctl.c
>>> +++ b/fs/xfs/xfs_ioctl.c
>>> @@ -822,6 +822,107 @@ xfs_ioc_fsbulkstat(
>>> return 0;
>>> }
>>> +/* Return 0 on success or positive error */
>>> +static int
>>> +xfs_bulkstat_fmt(
>>> + struct xfs_ibulk *breq,
>>> + const struct xfs_bulkstat *bstat)
>>> +{
>>> + if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
>>> + return -EFAULT;
>>> + return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
>>> +}
>>> +
>>> +/*
>>> + * Check the incoming bulk request @hdr from userspace and initialize the
>>> + * internal @breq bulk request appropriately. Returns 0 if the bulk request
>>> + * should proceed; 1 if there's nothing to do; or the usual negative error
>>> + * code.
>>> + */
>>> +static int
>>> +xfs_bulk_ireq_setup(
>>> + struct xfs_mount *mp,
>>> + struct xfs_bulk_ireq *hdr,
>>> + struct xfs_ibulk *breq,
>>> + void __user *ubuffer)
>>> +{
>>> + if (hdr->icount == 0 ||
>>> + (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
>>> + hdr->reserved32 ||
>>> + memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
>>> + return -EINVAL;
>>> +
>>> + if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
>>> + goto no_results;
>>> +
>>> + breq->ubuffer = ubuffer;
>>> + breq->icount = hdr->icount;
>>> + breq->startino = hdr->ino;
>>> + return 0;
>>> +no_results:
>>> + hdr->ocount = 0;
>>> + return 1;
>>> +}
>>> +
>>> +/*
>>> + * Update the userspace bulk request @hdr to reflect the end state of the
>>> + * internal bulk request @breq. If @error is negative then we return just
>>> + * that; otherwise (@error is 0 or 1) we copy the state so that userspace
>>> + * can discover what happened.
>>> + */
>>> +static int
>>> +xfs_bulk_ireq_teardown(
>>> + struct xfs_bulk_ireq *hdr,
>>> + struct xfs_ibulk *breq,
>>> + int error)
>>> +{
>>> + if (error < 0)
>>> + return error;
>> Hmm, passing in error just to bail on error seemed a little out of scope to
>> me. Is there a reason we're doing it here? Instead of after the preceding
>> call made in the caller? Referenced below.....
>>
>>> +
>>> + hdr->ino = breq->startino;
>>> + hdr->ocount = breq->ocount;
>>> + return 0;
>>> +}
>>> +
>>> +/* Handle the v5 bulkstat ioctl. */
>>> +STATIC int
>>> +xfs_ioc_bulkstat(
>>> + struct xfs_mount *mp,
>>> + unsigned int cmd,
>>> + struct xfs_bulkstat_req __user *arg)
>>> +{
>>> + struct xfs_bulk_ireq hdr;
>>> + struct xfs_ibulk breq = {
>>> + .mp = mp,
>>> + };
>>> + int error;
>>> +
>>> + if (!capable(CAP_SYS_ADMIN))
>>> + return -EPERM;
>>> +
>>> + if (XFS_FORCED_SHUTDOWN(mp))
>>> + return -EIO;
>>> +
>>> + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
>>> + return -EFAULT;
>>> +
>>> + error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->bulkstat);
>>> + if (error < 0)
>>> + return error;
>>> +
>>> + if (!error)
>>> + error = xfs_bulkstat(&breq, xfs_bulkstat_fmt);
>>> +
>> Right here. How about
>>
>> if (error < 0)
>> return error;
>>
>> It seems functionally equivalent. If error < 0, teardown will
>> bounce it back anyway and then the error check below will toss it back up.
>> Is that what you meant?
>
> Yeah, I could do that too. :)
>
> TBH I only threw it in as a helper function because xfs_bulk_ireq_setup
> seemed to need a counterpart; it's such a short function that I could
> just opencode it... Thoughts?
>
> --D
Sure, I'd be fine with open coding it too. I think I made a similar
comment in patch 6 since these helper functions are so small. I think
if you just add a comment or two about updating the header that would be
fine.
Allison
>
>> Allison
>>
>>> + error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
>>> + if (error)
>>> + return error;
>>> +
>>> + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
>>> + return -EFAULT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> STATIC int
>>> xfs_ioc_fsgeometry(
>>> struct xfs_mount *mp,
>>> @@ -1986,6 +2087,9 @@ xfs_file_ioctl(
>>> case XFS_IOC_FSINUMBERS:
>>> return xfs_ioc_fsbulkstat(mp, cmd, arg);
>>> + case XFS_IOC_BULKSTAT:
>>> + return xfs_ioc_bulkstat(mp, cmd, arg);
>>> +
>>> case XFS_IOC_FSGEOMETRY_V1:
>>> return xfs_ioc_fsgeometry(mp, arg, 3);
>>> case XFS_IOC_FSGEOMETRY_V4:
>>> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
>>> index bfe71747776b..84c342be4536 100644
>>> --- a/fs/xfs/xfs_ioctl32.c
>>> +++ b/fs/xfs/xfs_ioctl32.c
>>> @@ -576,6 +576,7 @@ xfs_file_compat_ioctl(
>>> case XFS_IOC_ERROR_CLEARALL:
>>> case FS_IOC_GETFSMAP:
>>> case XFS_IOC_SCRUB_METADATA:
>>> + case XFS_IOC_BULKSTAT:
>>> return xfs_file_ioctl(filp, cmd, p);
>>> #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
>>> /*
>>> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
>>> index d8f941b4d51c..954484c6eb96 100644
>>> --- a/fs/xfs/xfs_ondisk.h
>>> +++ b/fs/xfs/xfs_ondisk.h
>>> @@ -149,6 +149,7 @@ xfs_check_ondisk_structs(void)
>>> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
>>> XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
>>> + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
>>> }
>>> #endif /* __XFS_ONDISK_H */
>>>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls
2019-06-06 22:37 ` Allison Collins
@ 2019-06-07 16:10 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-06-07 16:10 UTC (permalink / raw)
To: Allison Collins; +Cc: linux-xfs
On Thu, Jun 06, 2019 at 03:37:40PM -0700, Allison Collins wrote:
> On 6/6/19 2:10 PM, Darrick J. Wong wrote:
> > On Wed, Jun 05, 2019 at 03:30:21PM -0700, Allison Collins wrote:
> > > On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > Wire up the new v5 BULKSTAT ioctl and rename the old one to V1.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > fs/xfs/libxfs/xfs_fs.h | 24 +++++++++++
> > > > fs/xfs/xfs_ioctl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > fs/xfs/xfs_ioctl32.c | 1
> > > > fs/xfs/xfs_ondisk.h | 1
> > > > 4 files changed, 129 insertions(+), 1 deletion(-)
> > > >
> > > >
> > > > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > > > index 8b8fe78511fb..960f3542e207 100644
> > > > --- a/fs/xfs/libxfs/xfs_fs.h
> > > > +++ b/fs/xfs/libxfs/xfs_fs.h
> > > > @@ -435,7 +435,6 @@ struct xfs_fsop_bulkreq {
> > > > __s32 __user *ocount; /* output count pointer */
> > > > };
> > > > -
> > > > /*
> > > > * Structures returned from xfs_inumbers routine (XFS_IOC_FSINUMBERS).
> > > > */
> > > > @@ -457,6 +456,28 @@ struct xfs_inumbers {
> > > > #define XFS_INUMBERS_VERSION_V1 (1)
> > > > #define XFS_INUMBERS_VERSION_V5 (5)
> > > > +/* Header for bulk inode requests. */
> > > > +struct xfs_bulk_ireq {
> > > > + uint64_t ino; /* I/O: start with this inode */
> > > > + uint32_t flags; /* I/O: operation flags */
> > > > + uint32_t icount; /* I: count of entries in buffer */
> > > > + uint32_t ocount; /* O: count of entries filled out */
> > > > + uint32_t reserved32; /* must be zero */
> > > > + uint64_t reserved[5]; /* must be zero */
> > > > +};
> > > > +
> > > > +#define XFS_BULK_IREQ_FLAGS_ALL (0)
> > > > +
> > > > +/*
> > > > + * ioctl structures for v5 bulkstat and inumbers requests
> > > > + */
> > > > +struct xfs_bulkstat_req {
> > > > + struct xfs_bulk_ireq hdr;
> > > > + struct xfs_bulkstat bulkstat[];
> > > > +};
> > > > +#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
> > > > + (nr) * sizeof(struct xfs_bulkstat))
> > > > +
> > > > /*
> > > > * Error injection.
> > > > */
> > > > @@ -758,6 +779,7 @@ struct xfs_scrub_metadata {
> > > > #define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
> > > > #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> > > > #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> > > > +#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> > > > /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
> > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > > > index e43ad688e683..f6724c75ba97 100644
> > > > --- a/fs/xfs/xfs_ioctl.c
> > > > +++ b/fs/xfs/xfs_ioctl.c
> > > > @@ -822,6 +822,107 @@ xfs_ioc_fsbulkstat(
> > > > return 0;
> > > > }
> > > > +/* Return 0 on success or positive error */
> > > > +static int
> > > > +xfs_bulkstat_fmt(
> > > > + struct xfs_ibulk *breq,
> > > > + const struct xfs_bulkstat *bstat)
> > > > +{
> > > > + if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
> > > > + return -EFAULT;
> > > > + return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
> > > > +}
> > > > +
> > > > +/*
> > > > + * Check the incoming bulk request @hdr from userspace and initialize the
> > > > + * internal @breq bulk request appropriately. Returns 0 if the bulk request
> > > > + * should proceed; 1 if there's nothing to do; or the usual negative error
> > > > + * code.
> > > > + */
> > > > +static int
> > > > +xfs_bulk_ireq_setup(
> > > > + struct xfs_mount *mp,
> > > > + struct xfs_bulk_ireq *hdr,
> > > > + struct xfs_ibulk *breq,
> > > > + void __user *ubuffer)
> > > > +{
> > > > + if (hdr->icount == 0 ||
> > > > + (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
> > > > + hdr->reserved32 ||
> > > > + memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> > > > + return -EINVAL;
> > > > +
> > > > + if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> > > > + goto no_results;
> > > > +
> > > > + breq->ubuffer = ubuffer;
> > > > + breq->icount = hdr->icount;
> > > > + breq->startino = hdr->ino;
> > > > + return 0;
> > > > +no_results:
> > > > + hdr->ocount = 0;
> > > > + return 1;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Update the userspace bulk request @hdr to reflect the end state of the
> > > > + * internal bulk request @breq. If @error is negative then we return just
> > > > + * that; otherwise (@error is 0 or 1) we copy the state so that userspace
> > > > + * can discover what happened.
> > > > + */
> > > > +static int
> > > > +xfs_bulk_ireq_teardown(
> > > > + struct xfs_bulk_ireq *hdr,
> > > > + struct xfs_ibulk *breq,
> > > > + int error)
> > > > +{
> > > > + if (error < 0)
> > > > + return error;
> > > Hmm, passing in error just to bail on error seemed a little out of scope to
> > > me. Is there a reason we're doing it here? Instead of after the preceding
> > > call made in the caller? Referenced below.....
> > >
> > > > +
> > > > + hdr->ino = breq->startino;
> > > > + hdr->ocount = breq->ocount;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/* Handle the v5 bulkstat ioctl. */
> > > > +STATIC int
> > > > +xfs_ioc_bulkstat(
> > > > + struct xfs_mount *mp,
> > > > + unsigned int cmd,
> > > > + struct xfs_bulkstat_req __user *arg)
> > > > +{
> > > > + struct xfs_bulk_ireq hdr;
> > > > + struct xfs_ibulk breq = {
> > > > + .mp = mp,
> > > > + };
> > > > + int error;
> > > > +
> > > > + if (!capable(CAP_SYS_ADMIN))
> > > > + return -EPERM;
> > > > +
> > > > + if (XFS_FORCED_SHUTDOWN(mp))
> > > > + return -EIO;
> > > > +
> > > > + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
> > > > + return -EFAULT;
> > > > +
> > > > + error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->bulkstat);
> > > > + if (error < 0)
> > > > + return error;
> > > > +
> > > > + if (!error)
> > > > + error = xfs_bulkstat(&breq, xfs_bulkstat_fmt);
> > > > +
> > > Right here. How about
> > >
> > > if (error < 0)
> > > return error;
> > >
> > > It seems functionally equivalent. If error < 0, teardown will
> > > bounce it back anyway and then the error check below will toss it back up.
> > > Is that what you meant?
> >
> > Yeah, I could do that too. :)
> >
> > TBH I only threw it in as a helper function because xfs_bulk_ireq_setup
> > seemed to need a counterpart; it's such a short function that I could
> > just opencode it... Thoughts?
> >
> > --D
>
> Sure, I'd be fine with open coding it too. I think I made a similar comment
> in patch 6 since these helper functions are so small. I think if you just
> add a comment or two about updating the header that would be fine.
I could even keep the teardown helper and just adjust the calling style:
error = xfs_bulk_ireq_setup(...);
if (error == 1)
goto teardown;
else if (error < 0)
goto out;
error = xfs_bulkstat(...);
if (error)
goto out;
teardown:
error = 0;
xfs_bulk_ireq_teardown(...);
if (copy_to_user(...)))
return -EFAULT;
out:
return error;
Yeah, maybe I'll do that, it's looks more like our normal practice.
--D
> Allison
>
> >
> > > Allison
> > >
> > > > + error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
> > > > + if (error)
> > > > + return error;
> > > > +
> > > > + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
> > > > + return -EFAULT;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > STATIC int
> > > > xfs_ioc_fsgeometry(
> > > > struct xfs_mount *mp,
> > > > @@ -1986,6 +2087,9 @@ xfs_file_ioctl(
> > > > case XFS_IOC_FSINUMBERS:
> > > > return xfs_ioc_fsbulkstat(mp, cmd, arg);
> > > > + case XFS_IOC_BULKSTAT:
> > > > + return xfs_ioc_bulkstat(mp, cmd, arg);
> > > > +
> > > > case XFS_IOC_FSGEOMETRY_V1:
> > > > return xfs_ioc_fsgeometry(mp, arg, 3);
> > > > case XFS_IOC_FSGEOMETRY_V4:
> > > > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > > > index bfe71747776b..84c342be4536 100644
> > > > --- a/fs/xfs/xfs_ioctl32.c
> > > > +++ b/fs/xfs/xfs_ioctl32.c
> > > > @@ -576,6 +576,7 @@ xfs_file_compat_ioctl(
> > > > case XFS_IOC_ERROR_CLEARALL:
> > > > case FS_IOC_GETFSMAP:
> > > > case XFS_IOC_SCRUB_METADATA:
> > > > + case XFS_IOC_BULKSTAT:
> > > > return xfs_file_ioctl(filp, cmd, p);
> > > > #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
> > > > /*
> > > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > > > index d8f941b4d51c..954484c6eb96 100644
> > > > --- a/fs/xfs/xfs_ondisk.h
> > > > +++ b/fs/xfs/xfs_ondisk.h
> > > > @@ -149,6 +149,7 @@ xfs_check_ondisk_structs(void)
> > > > XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> > > > XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> > > > + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
> > > > }
> > > > #endif /* __XFS_ONDISK_H */
> > > >
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/9] xfs: wire up the new v5 bulkstat_single ioctl
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (4 preceding siblings ...)
2019-05-29 22:28 ` [PATCH 5/9] xfs: wire up new v5 bulkstat ioctls Darrick J. Wong
@ 2019-05-29 22:28 ` Darrick J. Wong
2019-06-05 22:30 ` Allison Collins
2019-05-29 22:28 ` [PATCH 7/9] xfs: wire up the v5 INUMBERS ioctl Darrick J. Wong
` (2 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:28 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Wire up the V5 BULKSTAT_SINGLE ioctl and rename the old one V1.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 16 +++++++++
fs/xfs/xfs_ioctl.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_ioctl32.c | 1 +
fs/xfs/xfs_ondisk.h | 1 +
4 files changed, 103 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 960f3542e207..95d0411dae9b 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -468,6 +468,16 @@ struct xfs_bulk_ireq {
#define XFS_BULK_IREQ_FLAGS_ALL (0)
+/* Header for a single inode request. */
+struct xfs_ireq {
+ uint64_t ino; /* I/O: start with this inode */
+ uint32_t flags; /* I/O: operation flags */
+ uint32_t reserved32; /* must be zero */
+ uint64_t reserved[2]; /* must be zero */
+};
+
+#define XFS_IREQ_FLAGS_ALL (0)
+
/*
* ioctl structures for v5 bulkstat and inumbers requests
*/
@@ -478,6 +488,11 @@ struct xfs_bulkstat_req {
#define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
(nr) * sizeof(struct xfs_bulkstat))
+struct xfs_bulkstat_single_req {
+ struct xfs_ireq hdr;
+ struct xfs_bulkstat bulkstat;
+};
+
/*
* Error injection.
*/
@@ -780,6 +795,7 @@ struct xfs_scrub_metadata {
#define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
#define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
+#define XFS_IOC_BULKSTAT_SINGLE _IOR ('X', 128, struct xfs_bulkstat_single_req)
/* XFS_IOC_GETFSUUID ---------- deprecated 140 */
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index f6724c75ba97..f6971eb9561e 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -923,6 +923,89 @@ xfs_ioc_bulkstat(
return 0;
}
+/*
+ * Check the incoming singleton request @hdr from userspace and initialize the
+ * internal @breq bulk request appropriately. Returns 0 if the bulk request
+ * should proceed; or the usual negative error code.
+ */
+static int
+xfs_ireq_setup(
+ struct xfs_mount *mp,
+ struct xfs_ireq *hdr,
+ struct xfs_ibulk *breq,
+ void __user *ubuffer)
+{
+ if ((hdr->flags & ~XFS_IREQ_FLAGS_ALL) ||
+ hdr->reserved32 ||
+ memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+ return -EINVAL;
+
+ if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
+ return -EINVAL;
+
+ breq->ubuffer = ubuffer;
+ breq->icount = 1;
+ breq->startino = hdr->ino;
+ return 0;
+}
+
+/*
+ * Update the userspace singleton request @hdr to reflect the end state of the
+ * internal bulk request @breq. If @error is negative then we return just
+ * that; otherwise we copy the state so that userspace can discover what
+ * happened.
+ */
+static int
+xfs_ireq_teardown(
+ struct xfs_ireq *hdr,
+ struct xfs_ibulk *breq,
+ int error)
+{
+ if (error < 0)
+ return error;
+
+ hdr->ino = breq->startino;
+ return 0;
+}
+
+/* Handle the v5 bulkstat_single ioctl. */
+STATIC int
+xfs_ioc_bulkstat_single(
+ struct xfs_mount *mp,
+ unsigned int cmd,
+ struct xfs_bulkstat_single_req __user *arg)
+{
+ struct xfs_ireq hdr;
+ struct xfs_ibulk breq = {
+ .mp = mp,
+ };
+ int error;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ error = xfs_ireq_setup(mp, &hdr, &breq, &arg->bulkstat);
+ if (error)
+ return error;
+
+ error = xfs_bulkstat_one(&breq, xfs_bulkstat_fmt);
+
+ error = xfs_ireq_teardown(&hdr, &breq, error);
+ if (error)
+ return error;
+
+ if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ return 0;
+}
+
STATIC int
xfs_ioc_fsgeometry(
struct xfs_mount *mp,
@@ -2089,6 +2172,8 @@ xfs_file_ioctl(
case XFS_IOC_BULKSTAT:
return xfs_ioc_bulkstat(mp, cmd, arg);
+ case XFS_IOC_BULKSTAT_SINGLE:
+ return xfs_ioc_bulkstat_single(mp, cmd, arg);
case XFS_IOC_FSGEOMETRY_V1:
return xfs_ioc_fsgeometry(mp, arg, 3);
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 84c342be4536..08a90e3459d1 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -577,6 +577,7 @@ xfs_file_compat_ioctl(
case FS_IOC_GETFSMAP:
case XFS_IOC_SCRUB_METADATA:
case XFS_IOC_BULKSTAT:
+ case XFS_IOC_BULKSTAT_SINGLE:
return xfs_file_ioctl(filp, cmd, p);
#if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
/*
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 954484c6eb96..fa1252657b08 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -150,6 +150,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
+ XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_single_req, 224);
}
#endif /* __XFS_ONDISK_H */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 6/9] xfs: wire up the new v5 bulkstat_single ioctl
2019-05-29 22:28 ` [PATCH 6/9] xfs: wire up the new v5 bulkstat_single ioctl Darrick J. Wong
@ 2019-06-05 22:30 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Wire up the V5 BULKSTAT_SINGLE ioctl and rename the old one V1.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 16 +++++++++
> fs/xfs/xfs_ioctl.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_ioctl32.c | 1 +
> fs/xfs/xfs_ondisk.h | 1 +
> 4 files changed, 103 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 960f3542e207..95d0411dae9b 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -468,6 +468,16 @@ struct xfs_bulk_ireq {
>
> #define XFS_BULK_IREQ_FLAGS_ALL (0)
>
> +/* Header for a single inode request. */
> +struct xfs_ireq {
> + uint64_t ino; /* I/O: start with this inode */
> + uint32_t flags; /* I/O: operation flags */
> + uint32_t reserved32; /* must be zero */
> + uint64_t reserved[2]; /* must be zero */
> +};
> +
> +#define XFS_IREQ_FLAGS_ALL (0)
> +
> /*
> * ioctl structures for v5 bulkstat and inumbers requests
> */
> @@ -478,6 +488,11 @@ struct xfs_bulkstat_req {
> #define XFS_BULKSTAT_REQ_SIZE(nr) (sizeof(struct xfs_bulkstat_req) + \
> (nr) * sizeof(struct xfs_bulkstat))
>
> +struct xfs_bulkstat_single_req {
> + struct xfs_ireq hdr;
> + struct xfs_bulkstat bulkstat;
> +};
> +
> /*
> * Error injection.
> */
> @@ -780,6 +795,7 @@ struct xfs_scrub_metadata {
> #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> #define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> +#define XFS_IOC_BULKSTAT_SINGLE _IOR ('X', 128, struct xfs_bulkstat_single_req)
> /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
>
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index f6724c75ba97..f6971eb9561e 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -923,6 +923,89 @@ xfs_ioc_bulkstat(
> return 0;
> }
>
> +/*
> + * Check the incoming singleton request @hdr from userspace and initialize the
> + * internal @breq bulk request appropriately. Returns 0 if the bulk request
> + * should proceed; or the usual negative error code.
> + */
> +static int
> +xfs_ireq_setup(
> + struct xfs_mount *mp,
> + struct xfs_ireq *hdr,
> + struct xfs_ibulk *breq,
> + void __user *ubuffer)
> +{
> + if ((hdr->flags & ~XFS_IREQ_FLAGS_ALL) ||
> + hdr->reserved32 ||
> + memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> + return -EINVAL;
> +
> + if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> + return -EINVAL;
> +
> + breq->ubuffer = ubuffer;
> + breq->icount = 1;
> + breq->startino = hdr->ino;
> + return 0;
> +}
> +
> +/*
> + * Update the userspace singleton request @hdr to reflect the end state of the
> + * internal bulk request @breq. If @error is negative then we return just
> + * that; otherwise we copy the state so that userspace can discover what
> + * happened.
> + */
> +static int
> +xfs_ireq_teardown(
> + struct xfs_ireq *hdr,
> + struct xfs_ibulk *breq,
> + int error)
> +{
> + if (error < 0)
> + return error;
Similar nit about the error code here too. This function is so small,
it might make sense to just factor it up into the calling function,
sense I don't see it used else where in the set. I suppose it's good to
keep things consistent too though, it seem to be the pattern that the
other ioctls use.
I think the rest is ok though
Allison
> +
> + hdr->ino = breq->startino;
> + return 0;
> +}
> +
> +/* Handle the v5 bulkstat_single ioctl. */
> +STATIC int
> +xfs_ioc_bulkstat_single(
> + struct xfs_mount *mp,
> + unsigned int cmd,
> + struct xfs_bulkstat_single_req __user *arg)
> +{
> + struct xfs_ireq hdr;
> + struct xfs_ibulk breq = {
> + .mp = mp,
> + };
> + int error;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return -EIO;
> +
> + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + error = xfs_ireq_setup(mp, &hdr, &breq, &arg->bulkstat);
> + if (error)
> + return error;
> +
> + error = xfs_bulkstat_one(&breq, xfs_bulkstat_fmt);
> +
> + error = xfs_ireq_teardown(&hdr, &breq, error);
> + if (error)
> + return error;
> +
> + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> STATIC int
> xfs_ioc_fsgeometry(
> struct xfs_mount *mp,
> @@ -2089,6 +2172,8 @@ xfs_file_ioctl(
>
> case XFS_IOC_BULKSTAT:
> return xfs_ioc_bulkstat(mp, cmd, arg);
> + case XFS_IOC_BULKSTAT_SINGLE:
> + return xfs_ioc_bulkstat_single(mp, cmd, arg);
>
> case XFS_IOC_FSGEOMETRY_V1:
> return xfs_ioc_fsgeometry(mp, arg, 3);
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 84c342be4536..08a90e3459d1 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -577,6 +577,7 @@ xfs_file_compat_ioctl(
> case FS_IOC_GETFSMAP:
> case XFS_IOC_SCRUB_METADATA:
> case XFS_IOC_BULKSTAT:
> + case XFS_IOC_BULKSTAT_SINGLE:
> return xfs_file_ioctl(filp, cmd, p);
> #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
> /*
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 954484c6eb96..fa1252657b08 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -150,6 +150,7 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192);
> XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_single_req, 224);
> }
>
> #endif /* __XFS_ONDISK_H */
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 7/9] xfs: wire up the v5 INUMBERS ioctl
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (5 preceding siblings ...)
2019-05-29 22:28 ` [PATCH 6/9] xfs: wire up the new v5 bulkstat_single ioctl Darrick J. Wong
@ 2019-05-29 22:28 ` Darrick J. Wong
2019-06-05 22:30 ` Allison Collins
2019-05-29 22:28 ` [PATCH 8/9] xfs: specify AG in bulk req Darrick J. Wong
2019-05-29 22:28 ` [PATCH 9/9] xfs: allow bulkstat_single of special inodes Darrick J. Wong
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:28 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Wire up the v5 INUMBERS ioctl and rename the old one to v1.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 8 ++++++++
fs/xfs/xfs_ioctl.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_ioctl32.c | 1 +
fs/xfs/xfs_ondisk.h | 1 +
4 files changed, 61 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 95d0411dae9b..f9f35139d4b7 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -493,6 +493,13 @@ struct xfs_bulkstat_single_req {
struct xfs_bulkstat bulkstat;
};
+struct xfs_inumbers_req {
+ struct xfs_bulk_ireq hdr;
+ struct xfs_inumbers inumbers[];
+};
+#define XFS_INUMBERS_REQ_SIZE(nr) (sizeof(struct xfs_inumbers_req) + \
+ (nr) * sizeof(struct xfs_inumbers))
+
/*
* Error injection.
*/
@@ -796,6 +803,7 @@ struct xfs_scrub_metadata {
#define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
#define XFS_IOC_BULKSTAT_SINGLE _IOR ('X', 128, struct xfs_bulkstat_single_req)
+#define XFS_IOC_INUMBERS _IOR ('X', 129, struct xfs_inumbers_req)
/* XFS_IOC_GETFSUUID ---------- deprecated 140 */
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index f6971eb9561e..294039c2ea75 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1006,6 +1006,55 @@ xfs_ioc_bulkstat_single(
return 0;
}
+STATIC int
+xfs_inumbers_fmt(
+ struct xfs_ibulk *breq,
+ const struct xfs_inumbers *igrp)
+{
+ if (copy_to_user(breq->ubuffer, igrp, sizeof(struct xfs_inumbers)))
+ return -EFAULT;
+ return xfs_ibulk_advance(breq, sizeof(struct xfs_inumbers));
+}
+
+/* Handle the v5 inumbers ioctl. */
+STATIC int
+xfs_ioc_inumbers(
+ struct xfs_mount *mp,
+ unsigned int cmd,
+ struct xfs_inumbers_req __user *arg)
+{
+ struct xfs_bulk_ireq hdr;
+ struct xfs_ibulk breq = {
+ .mp = mp,
+ };
+ int error;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->inumbers);
+ if (error < 0)
+ return error;
+
+ if (!error)
+ error = xfs_inumbers(&breq, xfs_inumbers_fmt);
+
+ error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
+ if (error)
+ return error;
+
+ if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
+ return -EFAULT;
+
+ return 0;
+}
+
STATIC int
xfs_ioc_fsgeometry(
struct xfs_mount *mp,
@@ -2174,6 +2223,8 @@ xfs_file_ioctl(
return xfs_ioc_bulkstat(mp, cmd, arg);
case XFS_IOC_BULKSTAT_SINGLE:
return xfs_ioc_bulkstat_single(mp, cmd, arg);
+ case XFS_IOC_INUMBERS:
+ return xfs_ioc_inumbers(mp, cmd, arg);
case XFS_IOC_FSGEOMETRY_V1:
return xfs_ioc_fsgeometry(mp, arg, 3);
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 08a90e3459d1..772086998ab3 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -578,6 +578,7 @@ xfs_file_compat_ioctl(
case XFS_IOC_SCRUB_METADATA:
case XFS_IOC_BULKSTAT:
case XFS_IOC_BULKSTAT_SINGLE:
+ case XFS_IOC_INUMBERS:
return xfs_file_ioctl(filp, cmd, p);
#if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
/*
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index fa1252657b08..e390e65d2438 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -151,6 +151,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_single_req, 224);
+ XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers_req, 64);
}
#endif /* __XFS_ONDISK_H */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 7/9] xfs: wire up the v5 INUMBERS ioctl
2019-05-29 22:28 ` [PATCH 7/9] xfs: wire up the v5 INUMBERS ioctl Darrick J. Wong
@ 2019-06-05 22:30 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Wire up the v5 INUMBERS ioctl and rename the old one to v1.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 8 ++++++++
> fs/xfs/xfs_ioctl.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_ioctl32.c | 1 +
> fs/xfs/xfs_ondisk.h | 1 +
> 4 files changed, 61 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 95d0411dae9b..f9f35139d4b7 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -493,6 +493,13 @@ struct xfs_bulkstat_single_req {
> struct xfs_bulkstat bulkstat;
> };
>
> +struct xfs_inumbers_req {
> + struct xfs_bulk_ireq hdr;
> + struct xfs_inumbers inumbers[];
> +};
> +#define XFS_INUMBERS_REQ_SIZE(nr) (sizeof(struct xfs_inumbers_req) + \
> + (nr) * sizeof(struct xfs_inumbers))
> +
> /*
> * Error injection.
> */
> @@ -796,6 +803,7 @@ struct xfs_scrub_metadata {
> #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> #define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> #define XFS_IOC_BULKSTAT_SINGLE _IOR ('X', 128, struct xfs_bulkstat_single_req)
> +#define XFS_IOC_INUMBERS _IOR ('X', 129, struct xfs_inumbers_req)
> /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
>
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index f6971eb9561e..294039c2ea75 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1006,6 +1006,55 @@ xfs_ioc_bulkstat_single(
> return 0;
> }
>
> +STATIC int
> +xfs_inumbers_fmt(
> + struct xfs_ibulk *breq,
> + const struct xfs_inumbers *igrp)
> +{
> + if (copy_to_user(breq->ubuffer, igrp, sizeof(struct xfs_inumbers)))
> + return -EFAULT;
> + return xfs_ibulk_advance(breq, sizeof(struct xfs_inumbers));
> +}
> +
> +/* Handle the v5 inumbers ioctl. */
> +STATIC int
> +xfs_ioc_inumbers(
> + struct xfs_mount *mp,
> + unsigned int cmd,
> + struct xfs_inumbers_req __user *arg)
> +{
> + struct xfs_bulk_ireq hdr;
> + struct xfs_ibulk breq = {
> + .mp = mp,
> + };
> + int error;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return -EIO;
> +
> + if (copy_from_user(&hdr, &arg->hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + error = xfs_bulk_ireq_setup(mp, &hdr, &breq, arg->inumbers);
> + if (error < 0)
> + return error;
> +
> + if (!error)
> + error = xfs_inumbers(&breq, xfs_inumbers_fmt);
> +
Ditto on comments from patch 5 if you choose to change the teardown
params. Otherwise the rest looks ok.
Allison
> + error = xfs_bulk_ireq_teardown(&hdr, &breq, error);
> + if (error)
> + return error;
> +
> + if (copy_to_user(&arg->hdr, &hdr, sizeof(hdr)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> STATIC int
> xfs_ioc_fsgeometry(
> struct xfs_mount *mp,
> @@ -2174,6 +2223,8 @@ xfs_file_ioctl(
> return xfs_ioc_bulkstat(mp, cmd, arg);
> case XFS_IOC_BULKSTAT_SINGLE:
> return xfs_ioc_bulkstat_single(mp, cmd, arg);
> + case XFS_IOC_INUMBERS:
> + return xfs_ioc_inumbers(mp, cmd, arg);
>
> case XFS_IOC_FSGEOMETRY_V1:
> return xfs_ioc_fsgeometry(mp, arg, 3);
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 08a90e3459d1..772086998ab3 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -578,6 +578,7 @@ xfs_file_compat_ioctl(
> case XFS_IOC_SCRUB_METADATA:
> case XFS_IOC_BULKSTAT:
> case XFS_IOC_BULKSTAT_SINGLE:
> + case XFS_IOC_INUMBERS:
> return xfs_file_ioctl(filp, cmd, p);
> #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
> /*
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index fa1252657b08..e390e65d2438 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -151,6 +151,7 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_single_req, 224);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers_req, 64);
> }
>
> #endif /* __XFS_ONDISK_H */
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 8/9] xfs: specify AG in bulk req
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (6 preceding siblings ...)
2019-05-29 22:28 ` [PATCH 7/9] xfs: wire up the v5 INUMBERS ioctl Darrick J. Wong
@ 2019-05-29 22:28 ` Darrick J. Wong
2019-06-05 22:30 ` Allison Collins
2019-05-29 22:28 ` [PATCH 9/9] xfs: allow bulkstat_single of special inodes Darrick J. Wong
8 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:28 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Add a new xfs_bulk_ireq flag to constrain the iteration to a single AG.
If the passed-in startino value is zero then we start with the first
inode in the AG that the user passes in; otherwise, we iterate only
within the same AG as the passed-in inode.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 10 ++++++++--
fs/xfs/xfs_ioctl.c | 25 ++++++++++++++++++++++---
fs/xfs/xfs_itable.c | 6 +++---
fs/xfs/xfs_itable.h | 4 ++++
fs/xfs/xfs_iwalk.c | 12 ++++++++++++
fs/xfs/xfs_iwalk.h | 22 +++++++++++++++++-----
fs/xfs/xfs_qm.c | 3 ++-
7 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index f9f35139d4b7..77c06850ac52 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -462,11 +462,17 @@ struct xfs_bulk_ireq {
uint32_t flags; /* I/O: operation flags */
uint32_t icount; /* I: count of entries in buffer */
uint32_t ocount; /* O: count of entries filled out */
- uint32_t reserved32; /* must be zero */
+ uint32_t agno; /* I: see comment for IREQ_AGNO */
uint64_t reserved[5]; /* must be zero */
};
-#define XFS_BULK_IREQ_FLAGS_ALL (0)
+/*
+ * Only return results from the specified @agno. If @ino is zero, start
+ * with the first inode of @agno.
+ */
+#define XFS_BULK_IREQ_AGNO (1 << 0)
+
+#define XFS_BULK_IREQ_FLAGS_ALL (XFS_BULK_IREQ_AGNO)
/* Header for a single inode request. */
struct xfs_ireq {
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 294039c2ea75..cf48a2bad325 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -14,6 +14,7 @@
#include "xfs_ioctl.h"
#include "xfs_alloc.h"
#include "xfs_rtalloc.h"
+#include "xfs_iwalk.h"
#include "xfs_itable.h"
#include "xfs_error.h"
#include "xfs_attr.h"
@@ -846,18 +847,36 @@ xfs_bulk_ireq_setup(
struct xfs_ibulk *breq,
void __user *ubuffer)
{
- if (hdr->icount == 0 ||
- (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
- hdr->reserved32 ||
+ if (hdr->icount == 0 || (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
return -EINVAL;
+ /*
+ * The IREQ_AGNO flag means that we only want results from a given AG.
+ * If @hdr->ino is zero, we start iterating in that AG. If @hdr->ino is
+ * beyond the specified AG then we return no results.
+ */
+ if (hdr->flags & XFS_BULK_IREQ_AGNO) {
+ if (hdr->agno >= mp->m_sb.sb_agcount)
+ return -EINVAL;
+
+ if (hdr->ino == 0)
+ hdr->ino = XFS_AGINO_TO_INO(mp, hdr->agno, 0);
+ else if (XFS_INO_TO_AGNO(mp, hdr->ino) < hdr->agno)
+ return -EINVAL;
+ else if (XFS_INO_TO_AGNO(mp, hdr->ino) > hdr->agno)
+ goto no_results;
+ } else if (hdr->agno)
+ return -EINVAL;
+
if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
goto no_results;
breq->ubuffer = ubuffer;
breq->icount = hdr->icount;
breq->startino = hdr->ino;
+ if (hdr->flags & XFS_BULK_IREQ_AGNO)
+ breq->flags |= XFS_IBULK_SAME_AG;
return 0;
no_results:
hdr->ocount = 0;
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 8701596976bb..08a8a827d204 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -241,8 +241,8 @@ xfs_bulkstat(
if (xfs_bulkstat_already_done(breq->mp, breq->startino))
return 0;
- error = xfs_iwalk(breq->mp, NULL, breq->startino, xfs_bulkstat_iwalk,
- breq->icount, &bc);
+ error = xfs_iwalk(breq->mp, NULL, breq->startino, breq->flags,
+ xfs_bulkstat_iwalk, breq->icount, &bc);
/*
* We found some inodes, so clear the error status and return them.
@@ -362,7 +362,7 @@ xfs_inumbers(
if (xfs_bulkstat_already_done(breq->mp, breq->startino))
return 0;
- error = xfs_inobt_walk(breq->mp, NULL, breq->startino,
+ error = xfs_inobt_walk(breq->mp, NULL, breq->startino, breq->flags,
xfs_inumbers_walk, breq->icount, &ic);
/*
diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
index 2987f3eb335f..ae8e9cfca8ad 100644
--- a/fs/xfs/xfs_itable.h
+++ b/fs/xfs/xfs_itable.h
@@ -12,8 +12,12 @@ struct xfs_ibulk {
xfs_ino_t startino; /* start with this inode */
unsigned int icount; /* number of elements in ubuffer */
unsigned int ocount; /* number of records returned */
+ unsigned int flags; /* see XFS_IBULK_FLAG_* */
};
+/* Only iterate within the same AG as startino */
+#define XFS_IBULK_SAME_AG (XFS_IWALK_SAME_AG)
+
/* Return value that means we want to abort the walk. */
#define XFS_IBULK_ABORT (XFS_IWALK_ABORT)
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index afa4b22ffb3d..d21537e0bfbb 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -479,6 +479,7 @@ xfs_iwalk(
struct xfs_mount *mp,
struct xfs_trans *tp,
xfs_ino_t startino,
+ unsigned int flags,
xfs_iwalk_fn iwalk_fn,
unsigned int max_prefetch,
void *data)
@@ -495,6 +496,7 @@ xfs_iwalk(
int error;
ASSERT(agno < mp->m_sb.sb_agcount);
+ ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL));
xfs_iwalk_set_prefetch(&iwag, max_prefetch);
error = xfs_iwalk_allocbuf(&iwag);
@@ -506,6 +508,8 @@ xfs_iwalk(
if (error)
break;
iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
+ if (flags & XFS_INOBT_WALK_SAME_AG)
+ break;
}
xfs_iwalk_freebuf(&iwag);
@@ -541,6 +545,7 @@ int
xfs_iwalk_threaded(
struct xfs_mount *mp,
xfs_ino_t startino,
+ unsigned int flags,
xfs_iwalk_fn iwalk_fn,
unsigned int max_prefetch,
bool polled,
@@ -552,6 +557,7 @@ xfs_iwalk_threaded(
int error;
ASSERT(agno < mp->m_sb.sb_agcount);
+ ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL));
nr_threads = xfs_pwork_guess_datadev_parallelism(mp);
error = xfs_pwork_init(mp, &pctl, xfs_iwalk_ag_work, "xfs_iwalk",
@@ -572,6 +578,8 @@ xfs_iwalk_threaded(
xfs_iwalk_set_prefetch(iwag, max_prefetch);
xfs_pwork_queue(&pctl, &iwag->pwork);
startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
+ if (flags & XFS_INOBT_WALK_SAME_AG)
+ break;
}
if (polled)
@@ -672,6 +680,7 @@ xfs_inobt_walk(
struct xfs_mount *mp,
struct xfs_trans *tp,
xfs_ino_t startino,
+ unsigned int flags,
xfs_inobt_walk_fn inobt_walk_fn,
unsigned int max_prefetch,
void *data)
@@ -688,6 +697,7 @@ xfs_inobt_walk(
int error;
ASSERT(agno < mp->m_sb.sb_agcount);
+ ASSERT(!(flags & ~XFS_INOBT_WALK_FLAGS_ALL));
xfs_iwalk_set_prefetch(&iwag, max_prefetch * XFS_INODES_PER_CHUNK);
error = xfs_iwalk_allocbuf(&iwag);
@@ -699,6 +709,8 @@ xfs_inobt_walk(
if (error)
break;
iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
+ if (flags & XFS_INOBT_WALK_SAME_AG)
+ break;
}
xfs_iwalk_freebuf(&iwag);
diff --git a/fs/xfs/xfs_iwalk.h b/fs/xfs/xfs_iwalk.h
index 20bee93d4676..a0a1bae44362 100644
--- a/fs/xfs/xfs_iwalk.h
+++ b/fs/xfs/xfs_iwalk.h
@@ -13,10 +13,16 @@ typedef int (*xfs_iwalk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
#define XFS_IWALK_ABORT (1)
int xfs_iwalk(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t startino,
- xfs_iwalk_fn iwalk_fn, unsigned int max_prefetch, void *data);
+ unsigned int flags, xfs_iwalk_fn iwalk_fn,
+ unsigned int max_prefetch, void *data);
int xfs_iwalk_threaded(struct xfs_mount *mp, xfs_ino_t startino,
- xfs_iwalk_fn iwalk_fn, unsigned int max_prefetch, bool poll,
- void *data);
+ unsigned int flags, xfs_iwalk_fn iwalk_fn,
+ unsigned int max_prefetch, bool poll, void *data);
+
+/* Only iterate inodes within the same AG as @startino. */
+#define XFS_IWALK_SAME_AG (0x1)
+
+#define XFS_IWALK_FLAGS_ALL (XFS_IWALK_SAME_AG)
/* Walk all inode btree records in the filesystem starting from @startino. */
typedef int (*xfs_inobt_walk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
@@ -27,7 +33,13 @@ typedef int (*xfs_inobt_walk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
#define XFS_INOBT_WALK_ABORT (XFS_IWALK_ABORT)
int xfs_inobt_walk(struct xfs_mount *mp, struct xfs_trans *tp,
- xfs_ino_t startino, xfs_inobt_walk_fn inobt_walk_fn,
- unsigned int max_prefetch, void *data);
+ xfs_ino_t startino, unsigned int flags,
+ xfs_inobt_walk_fn inobt_walk_fn, unsigned int max_prefetch,
+ void *data);
+
+/* Only iterate inobt records within the same AG as @startino. */
+#define XFS_INOBT_WALK_SAME_AG (XFS_IWALK_SAME_AG)
+
+#define XFS_INOBT_WALK_FLAGS_ALL (XFS_INOBT_WALK_SAME_AG)
#endif /* __XFS_IWALK_H__ */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index de6a623ada02..3540238b6130 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1305,7 +1305,8 @@ xfs_qm_quotacheck(
flags |= XFS_PQUOTA_CHKD;
}
- error = xfs_iwalk_threaded(mp, 0, xfs_qm_dqusage_adjust, 0, true, NULL);
+ error = xfs_iwalk_threaded(mp, 0, 0, xfs_qm_dqusage_adjust, 0, true,
+ NULL);
if (error)
goto error_return;
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 8/9] xfs: specify AG in bulk req
2019-05-29 22:28 ` [PATCH 8/9] xfs: specify AG in bulk req Darrick J. Wong
@ 2019-06-05 22:30 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Add a new xfs_bulk_ireq flag to constrain the iteration to a single AG.
> If the passed-in startino value is zero then we start with the first
> inode in the AG that the user passes in; otherwise, we iterate only
> within the same AG as the passed-in inode.
>
Looks ok, you can add my review:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 10 ++++++++--
> fs/xfs/xfs_ioctl.c | 25 ++++++++++++++++++++++---
> fs/xfs/xfs_itable.c | 6 +++---
> fs/xfs/xfs_itable.h | 4 ++++
> fs/xfs/xfs_iwalk.c | 12 ++++++++++++
> fs/xfs/xfs_iwalk.h | 22 +++++++++++++++++-----
> fs/xfs/xfs_qm.c | 3 ++-
> 7 files changed, 68 insertions(+), 14 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index f9f35139d4b7..77c06850ac52 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -462,11 +462,17 @@ struct xfs_bulk_ireq {
> uint32_t flags; /* I/O: operation flags */
> uint32_t icount; /* I: count of entries in buffer */
> uint32_t ocount; /* O: count of entries filled out */
> - uint32_t reserved32; /* must be zero */
> + uint32_t agno; /* I: see comment for IREQ_AGNO */
> uint64_t reserved[5]; /* must be zero */
> };
>
> -#define XFS_BULK_IREQ_FLAGS_ALL (0)
> +/*
> + * Only return results from the specified @agno. If @ino is zero, start
> + * with the first inode of @agno.
> + */
> +#define XFS_BULK_IREQ_AGNO (1 << 0)
> +
> +#define XFS_BULK_IREQ_FLAGS_ALL (XFS_BULK_IREQ_AGNO)
>
> /* Header for a single inode request. */
> struct xfs_ireq {
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 294039c2ea75..cf48a2bad325 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -14,6 +14,7 @@
> #include "xfs_ioctl.h"
> #include "xfs_alloc.h"
> #include "xfs_rtalloc.h"
> +#include "xfs_iwalk.h"
> #include "xfs_itable.h"
> #include "xfs_error.h"
> #include "xfs_attr.h"
> @@ -846,18 +847,36 @@ xfs_bulk_ireq_setup(
> struct xfs_ibulk *breq,
> void __user *ubuffer)
> {
> - if (hdr->icount == 0 ||
> - (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
> - hdr->reserved32 ||
> + if (hdr->icount == 0 || (hdr->flags & ~XFS_BULK_IREQ_FLAGS_ALL) ||
> memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> return -EINVAL;
>
> + /*
> + * The IREQ_AGNO flag means that we only want results from a given AG.
> + * If @hdr->ino is zero, we start iterating in that AG. If @hdr->ino is
> + * beyond the specified AG then we return no results.
> + */
> + if (hdr->flags & XFS_BULK_IREQ_AGNO) {
> + if (hdr->agno >= mp->m_sb.sb_agcount)
> + return -EINVAL;
> +
> + if (hdr->ino == 0)
> + hdr->ino = XFS_AGINO_TO_INO(mp, hdr->agno, 0);
> + else if (XFS_INO_TO_AGNO(mp, hdr->ino) < hdr->agno)
> + return -EINVAL;
> + else if (XFS_INO_TO_AGNO(mp, hdr->ino) > hdr->agno)
> + goto no_results;
> + } else if (hdr->agno)
> + return -EINVAL;
> +
> if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> goto no_results;
>
> breq->ubuffer = ubuffer;
> breq->icount = hdr->icount;
> breq->startino = hdr->ino;
> + if (hdr->flags & XFS_BULK_IREQ_AGNO)
> + breq->flags |= XFS_IBULK_SAME_AG;
> return 0;
> no_results:
> hdr->ocount = 0;
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 8701596976bb..08a8a827d204 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -241,8 +241,8 @@ xfs_bulkstat(
> if (xfs_bulkstat_already_done(breq->mp, breq->startino))
> return 0;
>
> - error = xfs_iwalk(breq->mp, NULL, breq->startino, xfs_bulkstat_iwalk,
> - breq->icount, &bc);
> + error = xfs_iwalk(breq->mp, NULL, breq->startino, breq->flags,
> + xfs_bulkstat_iwalk, breq->icount, &bc);
>
> /*
> * We found some inodes, so clear the error status and return them.
> @@ -362,7 +362,7 @@ xfs_inumbers(
> if (xfs_bulkstat_already_done(breq->mp, breq->startino))
> return 0;
>
> - error = xfs_inobt_walk(breq->mp, NULL, breq->startino,
> + error = xfs_inobt_walk(breq->mp, NULL, breq->startino, breq->flags,
> xfs_inumbers_walk, breq->icount, &ic);
>
> /*
> diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> index 2987f3eb335f..ae8e9cfca8ad 100644
> --- a/fs/xfs/xfs_itable.h
> +++ b/fs/xfs/xfs_itable.h
> @@ -12,8 +12,12 @@ struct xfs_ibulk {
> xfs_ino_t startino; /* start with this inode */
> unsigned int icount; /* number of elements in ubuffer */
> unsigned int ocount; /* number of records returned */
> + unsigned int flags; /* see XFS_IBULK_FLAG_* */
> };
>
> +/* Only iterate within the same AG as startino */
> +#define XFS_IBULK_SAME_AG (XFS_IWALK_SAME_AG)
> +
> /* Return value that means we want to abort the walk. */
> #define XFS_IBULK_ABORT (XFS_IWALK_ABORT)
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index afa4b22ffb3d..d21537e0bfbb 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -479,6 +479,7 @@ xfs_iwalk(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> xfs_ino_t startino,
> + unsigned int flags,
> xfs_iwalk_fn iwalk_fn,
> unsigned int max_prefetch,
> void *data)
> @@ -495,6 +496,7 @@ xfs_iwalk(
> int error;
>
> ASSERT(agno < mp->m_sb.sb_agcount);
> + ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL));
>
> xfs_iwalk_set_prefetch(&iwag, max_prefetch);
> error = xfs_iwalk_allocbuf(&iwag);
> @@ -506,6 +508,8 @@ xfs_iwalk(
> if (error)
> break;
> iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
> + if (flags & XFS_INOBT_WALK_SAME_AG)
> + break;
> }
>
> xfs_iwalk_freebuf(&iwag);
> @@ -541,6 +545,7 @@ int
> xfs_iwalk_threaded(
> struct xfs_mount *mp,
> xfs_ino_t startino,
> + unsigned int flags,
> xfs_iwalk_fn iwalk_fn,
> unsigned int max_prefetch,
> bool polled,
> @@ -552,6 +557,7 @@ xfs_iwalk_threaded(
> int error;
>
> ASSERT(agno < mp->m_sb.sb_agcount);
> + ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL));
>
> nr_threads = xfs_pwork_guess_datadev_parallelism(mp);
> error = xfs_pwork_init(mp, &pctl, xfs_iwalk_ag_work, "xfs_iwalk",
> @@ -572,6 +578,8 @@ xfs_iwalk_threaded(
> xfs_iwalk_set_prefetch(iwag, max_prefetch);
> xfs_pwork_queue(&pctl, &iwag->pwork);
> startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
> + if (flags & XFS_INOBT_WALK_SAME_AG)
> + break;
> }
>
> if (polled)
> @@ -672,6 +680,7 @@ xfs_inobt_walk(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> xfs_ino_t startino,
> + unsigned int flags,
> xfs_inobt_walk_fn inobt_walk_fn,
> unsigned int max_prefetch,
> void *data)
> @@ -688,6 +697,7 @@ xfs_inobt_walk(
> int error;
>
> ASSERT(agno < mp->m_sb.sb_agcount);
> + ASSERT(!(flags & ~XFS_INOBT_WALK_FLAGS_ALL));
>
> xfs_iwalk_set_prefetch(&iwag, max_prefetch * XFS_INODES_PER_CHUNK);
> error = xfs_iwalk_allocbuf(&iwag);
> @@ -699,6 +709,8 @@ xfs_inobt_walk(
> if (error)
> break;
> iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0);
> + if (flags & XFS_INOBT_WALK_SAME_AG)
> + break;
> }
>
> xfs_iwalk_freebuf(&iwag);
> diff --git a/fs/xfs/xfs_iwalk.h b/fs/xfs/xfs_iwalk.h
> index 20bee93d4676..a0a1bae44362 100644
> --- a/fs/xfs/xfs_iwalk.h
> +++ b/fs/xfs/xfs_iwalk.h
> @@ -13,10 +13,16 @@ typedef int (*xfs_iwalk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
> #define XFS_IWALK_ABORT (1)
>
> int xfs_iwalk(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t startino,
> - xfs_iwalk_fn iwalk_fn, unsigned int max_prefetch, void *data);
> + unsigned int flags, xfs_iwalk_fn iwalk_fn,
> + unsigned int max_prefetch, void *data);
> int xfs_iwalk_threaded(struct xfs_mount *mp, xfs_ino_t startino,
> - xfs_iwalk_fn iwalk_fn, unsigned int max_prefetch, bool poll,
> - void *data);
> + unsigned int flags, xfs_iwalk_fn iwalk_fn,
> + unsigned int max_prefetch, bool poll, void *data);
> +
> +/* Only iterate inodes within the same AG as @startino. */
> +#define XFS_IWALK_SAME_AG (0x1)
> +
> +#define XFS_IWALK_FLAGS_ALL (XFS_IWALK_SAME_AG)
>
> /* Walk all inode btree records in the filesystem starting from @startino. */
> typedef int (*xfs_inobt_walk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
> @@ -27,7 +33,13 @@ typedef int (*xfs_inobt_walk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,
> #define XFS_INOBT_WALK_ABORT (XFS_IWALK_ABORT)
>
> int xfs_inobt_walk(struct xfs_mount *mp, struct xfs_trans *tp,
> - xfs_ino_t startino, xfs_inobt_walk_fn inobt_walk_fn,
> - unsigned int max_prefetch, void *data);
> + xfs_ino_t startino, unsigned int flags,
> + xfs_inobt_walk_fn inobt_walk_fn, unsigned int max_prefetch,
> + void *data);
> +
> +/* Only iterate inobt records within the same AG as @startino. */
> +#define XFS_INOBT_WALK_SAME_AG (XFS_IWALK_SAME_AG)
> +
> +#define XFS_INOBT_WALK_FLAGS_ALL (XFS_INOBT_WALK_SAME_AG)
>
> #endif /* __XFS_IWALK_H__ */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index de6a623ada02..3540238b6130 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1305,7 +1305,8 @@ xfs_qm_quotacheck(
> flags |= XFS_PQUOTA_CHKD;
> }
>
> - error = xfs_iwalk_threaded(mp, 0, xfs_qm_dqusage_adjust, 0, true, NULL);
> + error = xfs_iwalk_threaded(mp, 0, 0, xfs_qm_dqusage_adjust, 0, true,
> + NULL);
> if (error)
> goto error_return;
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 9/9] xfs: allow bulkstat_single of special inodes
2019-05-29 22:27 [PATCH 0/9] xfs: introduce new BULKSTAT and INUMBERS ioctls Darrick J. Wong
` (7 preceding siblings ...)
2019-05-29 22:28 ` [PATCH 8/9] xfs: specify AG in bulk req Darrick J. Wong
@ 2019-05-29 22:28 ` Darrick J. Wong
2019-06-05 22:31 ` Allison Collins
2019-06-06 21:15 ` Darrick J. Wong
8 siblings, 2 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-29 22:28 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Create a new ireq flag (for single bulkstats) that enables userspace to
ask us for a special inode number instead of interpreting @ino as a
literal inode number. This enables us to query the root inode easily.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_fs.h | 11 ++++++++++-
fs/xfs/xfs_ioctl.c | 10 ++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 77c06850ac52..1826aa11b585 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -482,7 +482,16 @@ struct xfs_ireq {
uint64_t reserved[2]; /* must be zero */
};
-#define XFS_IREQ_FLAGS_ALL (0)
+/*
+ * The @ino value is a special value, not a literal inode number. See the
+ * XFS_IREQ_SPECIAL_* values below.
+ */
+#define XFS_IREQ_SPECIAL (1 << 0)
+
+#define XFS_IREQ_FLAGS_ALL (XFS_IREQ_SPECIAL)
+
+/* Return the root directory inode. */
+#define XFS_IREQ_SPECIAL_ROOT (1)
/*
* ioctl structures for v5 bulkstat and inumbers requests
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index cf48a2bad325..605bfff3011f 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -959,6 +959,16 @@ xfs_ireq_setup(
memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
return -EINVAL;
+ if (hdr->flags & XFS_IREQ_SPECIAL) {
+ switch (hdr->ino) {
+ case XFS_IREQ_SPECIAL_ROOT:
+ hdr->ino = mp->m_sb.sb_rootino;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
return -EINVAL;
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 9/9] xfs: allow bulkstat_single of special inodes
2019-05-29 22:28 ` [PATCH 9/9] xfs: allow bulkstat_single of special inodes Darrick J. Wong
@ 2019-06-05 22:31 ` Allison Collins
2019-06-06 21:15 ` Darrick J. Wong
1 sibling, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-06-05 22:31 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/29/19 3:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Create a new ireq flag (for single bulkstats) that enables userspace to
> ask us for a special inode number instead of interpreting @ino as a
> literal inode number. This enables us to query the root inode easily.
>
Looks good. You can add my review:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 11 ++++++++++-
> fs/xfs/xfs_ioctl.c | 10 ++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 77c06850ac52..1826aa11b585 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -482,7 +482,16 @@ struct xfs_ireq {
> uint64_t reserved[2]; /* must be zero */
> };
>
> -#define XFS_IREQ_FLAGS_ALL (0)
> +/*
> + * The @ino value is a special value, not a literal inode number. See the
> + * XFS_IREQ_SPECIAL_* values below.
> + */
> +#define XFS_IREQ_SPECIAL (1 << 0)
> +
> +#define XFS_IREQ_FLAGS_ALL (XFS_IREQ_SPECIAL)
> +
> +/* Return the root directory inode. */
> +#define XFS_IREQ_SPECIAL_ROOT (1)
>
> /*
> * ioctl structures for v5 bulkstat and inumbers requests
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index cf48a2bad325..605bfff3011f 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -959,6 +959,16 @@ xfs_ireq_setup(
> memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> return -EINVAL;
>
> + if (hdr->flags & XFS_IREQ_SPECIAL) {
> + switch (hdr->ino) {
> + case XFS_IREQ_SPECIAL_ROOT:
> + hdr->ino = mp->m_sb.sb_rootino;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 9/9] xfs: allow bulkstat_single of special inodes
2019-05-29 22:28 ` [PATCH 9/9] xfs: allow bulkstat_single of special inodes Darrick J. Wong
2019-06-05 22:31 ` Allison Collins
@ 2019-06-06 21:15 ` Darrick J. Wong
1 sibling, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-06-06 21:15 UTC (permalink / raw)
To: linux-xfs
On Wed, May 29, 2019 at 03:28:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Create a new ireq flag (for single bulkstats) that enables userspace to
> ask us for a special inode number instead of interpreting @ino as a
> literal inode number. This enables us to query the root inode easily.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_fs.h | 11 ++++++++++-
> fs/xfs/xfs_ioctl.c | 10 ++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 77c06850ac52..1826aa11b585 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -482,7 +482,16 @@ struct xfs_ireq {
> uint64_t reserved[2]; /* must be zero */
> };
>
> -#define XFS_IREQ_FLAGS_ALL (0)
> +/*
> + * The @ino value is a special value, not a literal inode number. See the
> + * XFS_IREQ_SPECIAL_* values below.
> + */
> +#define XFS_IREQ_SPECIAL (1 << 0)
> +
> +#define XFS_IREQ_FLAGS_ALL (XFS_IREQ_SPECIAL)
> +
> +/* Return the root directory inode. */
> +#define XFS_IREQ_SPECIAL_ROOT (1)
>
> /*
> * ioctl structures for v5 bulkstat and inumbers requests
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index cf48a2bad325..605bfff3011f 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -959,6 +959,16 @@ xfs_ireq_setup(
> memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
> return -EINVAL;
>
> + if (hdr->flags & XFS_IREQ_SPECIAL) {
> + switch (hdr->ino) {
> + case XFS_IREQ_SPECIAL_ROOT:
Hmmm sooo... this doesn't solve Eric's immediate problem where xfsdump
needs to be fed the root inode. We assumed for years that the first
inode returned by bulkstat would always be the root dir, but it turned
out that mkfs puts the root dir at the start of the first possible inode
chunk past the end of the (AG headers + AG btree roots + AGFL blocks).
If all those blocks get freed then inodes can sneak into AG 0 before the
"first" inode chunk, invalidating the assumption.
IOWS, do we need to retrofit SPECIAL_ROOT into the old FSBULKSTAT_SINGLE
somehow? Maybe if you pass in bulkreq.lastip == NULL then you get root?
--D
> + hdr->ino = mp->m_sb.sb_rootino;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> if (XFS_INO_TO_AGNO(mp, hdr->ino) >= mp->m_sb.sb_agcount)
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread