Re: [PATCH v4 2/3] Btrfs: add ioctl to get and reset the device stats
On Tue, May 22, 2012 at 12:53:48PM +0200, Stefan Behrens wrote:
> An ioctl interface is added to get the device statistic counters.
> A second ioctl is added to atomically get and reset these counters.
>
> Signed-off-by: Stefan Behrens
> ---
> fs/btrfs/ioctl.c | 26
> fs/btrfs/ioctl.h | 28 +
> fs/btrfs/volumes.c | 69
>
> fs/btrfs/volumes.h | 13 ++
> 4 files changed, 136 insertions(+)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 14f8e1f..19d2244 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3424,6 +3446,10 @@ long btrfs_ioctl(struct file *file, unsigned int
> return btrfs_ioctl_balance_ctl(root, arg);
> case BTRFS_IOC_BALANCE_PROGRESS:
> return btrfs_ioctl_balance_progress(root, argp);
> + case BTRFS_IOC_GET_DEVICE_STATS:
> + return btrfs_ioctl_get_device_stats(root, argp, 0);
> + case BTRFS_IOC_GET_AND_RESET_DEVICE_STATS:
> + return btrfs_ioctl_get_device_stats(root, argp, 1);
> }
>
> return -ENOTTY;
> diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
> index 086e6bd..f1c1196 100644
> --- a/fs/btrfs/ioctl.h
> +++ b/fs/btrfs/ioctl.h
> @@ -266,6 +266,30 @@ struct btrfs_ioctl_logical_ino_args {
> __u64 inodes;
> };
>
> +#define BTRFS_IOCTL_GET_DEVICE_STATS_MAX_NR_ITEMS5
The check at the end of btrfs_get_device_stats() should use this number:
> + if (stats->nr_items > 5)
> + stats->nr_items = 5;
> + return 0;
> +}
> +struct btrfs_ioctl_get_device_stats {
> + __u64 devid;/* in */
> + __u64 nr_items; /* in/out */
> +
> + /* out values: */
> +
> + /* disk I/O failure stats */
> + __u64 cnt_write_io_errs; /* EIO or EREMOTEIO from lower layers */
> + __u64 cnt_read_io_errs; /* EIO or EREMOTEIO from lower layers */
> + __u64 cnt_flush_io_errs; /* EIO or EREMOTEIO from lower layers */
> +
> + /* stats for indirect indications for I/O failures */
> + __u64 cnt_corruption_errs; /* checksum error, bytenr error or
> + * contents is illegal: this is an
> + * indication that the block was damaged
> + * during read or write, or written to
> + * wrong location or read from wrong
> + * location */
> + __u64 cnt_generation_errs; /* an indication that blocks have not
> + * been written */
> + __u64 unused[121]; /* pad to 1k */
> +};
Padding has landed, and thanks for the explanation in the V3, I see how
the compatibility works.
> +
> #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
> struct btrfs_ioctl_vol_args)
> #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
> @@ -330,5 +354,9 @@ struct btrfs_ioctl_logical_ino_args {
> struct btrfs_ioctl_ino_path_args)
> #define BTRFS_IOC_LOGICAL_INO _IOWR(BTRFS_IOCTL_MAGIC, 36, \
> struct btrfs_ioctl_ino_path_args)
> +#define BTRFS_IOC_GET_DEVICE_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
> + struct btrfs_ioctl_get_device_stats)
> +#define BTRFS_IOC_GET_AND_RESET_DEVICE_STATS _IOWR(BTRFS_IOCTL_MAGIC, 53, \
> + struct btrfs_ioctl_get_device_stats)
I think that 'reset device' should go into the structure, besides the
ioctl number there is no difference.
Then the permission check will look like:
> @@ -3042,6 +3042,28 @@ static long btrfs_ioctl_scrub_progress(struct
> btrfs_root *root,
> return ret;
> }
>
> +static long btrfs_ioctl_get_device_stats(struct btrfs_root *root,
> + void __user *arg, int reset_after_read)
void __user *arg)
> +{
> + struct btrfs_ioctl_get_device_stats *sa;
> + int ret;
> +
> + sa = memdup_user(arg, sizeof(*sa));
> + if (IS_ERR(sa))
> + return PTR_ERR(sa);
> +
> + if (reset_after_read && !capable(CAP_SYS_ADMIN))
if (sa->reset_after_read && !capable(CAP_SYS_ADMIN)) {
kfree(sa);
> + return -EPERM;
}
> +
> + ret = btrfs_get_device_stats(root, sa, reset_after_read);
> +
> + if (copy_to_user(arg, sa, sizeof(*sa)))
> + ret = -EFAULT;
> +
> + kfree(sa);
> + return ret;
> +}
So the EINVAL could come earlier than EPERM, but I don't think it's a
problem.
david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/3] Btrfs: add ioctl to get and reset the device stats
An ioctl interface is added to get the device statistic counters.
A second ioctl is added to atomically get and reset these counters.
Signed-off-by: Stefan Behrens
---
fs/btrfs/ioctl.c | 26
fs/btrfs/ioctl.h | 28 +
fs/btrfs/volumes.c | 69
fs/btrfs/volumes.h | 13 ++
4 files changed, 136 insertions(+)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 14f8e1f..19d2244 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3042,6 +3042,28 @@ static long btrfs_ioctl_scrub_progress(struct btrfs_root
*root,
return ret;
}
+static long btrfs_ioctl_get_device_stats(struct btrfs_root *root,
+void __user *arg, int reset_after_read)
+{
+ struct btrfs_ioctl_get_device_stats *sa;
+ int ret;
+
+ if (reset_after_read && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ sa = memdup_user(arg, sizeof(*sa));
+ if (IS_ERR(sa))
+ return PTR_ERR(sa);
+
+ ret = btrfs_get_device_stats(root, sa, reset_after_read);
+
+ if (copy_to_user(arg, sa, sizeof(*sa)))
+ ret = -EFAULT;
+
+ kfree(sa);
+ return ret;
+}
+
static long btrfs_ioctl_ino_to_path(struct btrfs_root *root, void __user *arg)
{
int ret = 0;
@@ -3424,6 +3446,10 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_balance_ctl(root, arg);
case BTRFS_IOC_BALANCE_PROGRESS:
return btrfs_ioctl_balance_progress(root, argp);
+ case BTRFS_IOC_GET_DEVICE_STATS:
+ return btrfs_ioctl_get_device_stats(root, argp, 0);
+ case BTRFS_IOC_GET_AND_RESET_DEVICE_STATS:
+ return btrfs_ioctl_get_device_stats(root, argp, 1);
}
return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 086e6bd..f1c1196 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -266,6 +266,30 @@ struct btrfs_ioctl_logical_ino_args {
__u64 inodes;
};
+#define BTRFS_IOCTL_GET_DEVICE_STATS_MAX_NR_ITEMS 5
+struct btrfs_ioctl_get_device_stats {
+ __u64 devid;/* in */
+ __u64 nr_items; /* in/out */
+
+ /* out values: */
+
+ /* disk I/O failure stats */
+ __u64 cnt_write_io_errs; /* EIO or EREMOTEIO from lower layers */
+ __u64 cnt_read_io_errs; /* EIO or EREMOTEIO from lower layers */
+ __u64 cnt_flush_io_errs; /* EIO or EREMOTEIO from lower layers */
+
+ /* stats for indirect indications for I/O failures */
+ __u64 cnt_corruption_errs; /* checksum error, bytenr error or
+ * contents is illegal: this is an
+ * indication that the block was damaged
+ * during read or write, or written to
+ * wrong location or read from wrong
+ * location */
+ __u64 cnt_generation_errs; /* an indication that blocks have not
+ * been written */
+ __u64 unused[121]; /* pad to 1k */
+};
+
#define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
struct btrfs_ioctl_vol_args)
#define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -330,5 +354,9 @@ struct btrfs_ioctl_logical_ino_args {
struct btrfs_ioctl_ino_path_args)
#define BTRFS_IOC_LOGICAL_INO _IOWR(BTRFS_IOCTL_MAGIC, 36, \
struct btrfs_ioctl_ino_path_args)
+#define BTRFS_IOC_GET_DEVICE_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
+struct btrfs_ioctl_get_device_stats)
+#define BTRFS_IOC_GET_AND_RESET_DEVICE_STATS _IOWR(BTRFS_IOCTL_MAGIC, 53, \
+struct btrfs_ioctl_get_device_stats)
#endif
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c458c74..5f5a6ce 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4638,3 +4638,72 @@ void btrfs_device_stat_print_on_error(struct
btrfs_device *device)
btrfs_device_stat_read(
&device->cnt_generation_errs));
}
+
+int btrfs_get_device_stats(struct btrfs_root *root,
+ struct btrfs_ioctl_get_device_stats *stats,
+ int reset_after_read)
+{
+ struct btrfs_device *dev;
+ struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
+
+ mutex_lock(&fs_devices->device_list_mutex);
+ dev = btrfs_find_device(root, stats->devid, NULL, NULL);
+ mutex_unlock(&fs_devices->device_list_mutex);
+
+ if (!dev) {
+ printk(KERN_WARNING
+ "btrfs: get device_stats failed, device not found\n");
+ return -ENODEV;
+
