diff mbox series

[v3] btrfs: add a force_chunk_alloc to space_info's sysfs

Message ID 20190805183153.22712-1-josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series [v3] btrfs: add a force_chunk_alloc to space_info's sysfs | expand

Commit Message

Josef Bacik Aug. 5, 2019, 6:31 p.m. UTC
In testing various things such as the btrfsck patch to detect over
allocation of chunks, empty block group deletion, and balance I've had
various ways to force chunk allocations for debug purposes.  Add a sysfs
file to enable forcing of chunk allocation for the owning space info in
order to enable us to add testcases in the future to test these various
features easier.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
v2->v3:
- as per Qu's suggestion, moved this to sysfs where it's easier to mess with and
  makes more sense.
- added side-effect is mixed bg forced allocation works with this scheme as
  well.
- had to add get_btrfs_kobj() to get to fs_info, not sure this is better than
  just adding the fs_info to the space_info, am open to other opinions here.

 fs/btrfs/sysfs.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

David Sterba Oct. 1, 2019, 6:23 p.m. UTC | #1
On Mon, Aug 05, 2019 at 02:31:53PM -0400, Josef Bacik wrote:
> In testing various things such as the btrfsck patch to detect over
> allocation of chunks, empty block group deletion, and balance I've had
> various ways to force chunk allocations for debug purposes.  Add a sysfs
> file to enable forcing of chunk allocation for the owning space info in
> order to enable us to add testcases in the future to test these various
> features easier.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> v2->v3:
> - as per Qu's suggestion, moved this to sysfs where it's easier to mess with and
>   makes more sense.
> - added side-effect is mixed bg forced allocation works with this scheme as
>   well.
> - had to add get_btrfs_kobj() to get to fs_info, not sure this is better than
>   just adding the fs_info to the space_info, am open to other opinions here.

I believe v3 is the latest version, but I don't see the new file being
added to the debug/ directory (ie. /sys/fs/btrfs/UUID/debug/)

As it has been discussed, whether to make the file always visible or
only with CONFIG_BTRFS_DEBUG, I'd rather keep it debugging-only. The
testing coverage should be sufficient for fstests that are run with the
config option and not giving users a knob to paper over allocator
problems sounds desirable.
Josef Bacik Oct. 1, 2019, 6:28 p.m. UTC | #2
On Tue, Oct 01, 2019 at 08:23:51PM +0200, David Sterba wrote:
> On Mon, Aug 05, 2019 at 02:31:53PM -0400, Josef Bacik wrote:
> > In testing various things such as the btrfsck patch to detect over
> > allocation of chunks, empty block group deletion, and balance I've had
> > various ways to force chunk allocations for debug purposes.  Add a sysfs
> > file to enable forcing of chunk allocation for the owning space info in
> > order to enable us to add testcases in the future to test these various
> > features easier.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> > v2->v3:
> > - as per Qu's suggestion, moved this to sysfs where it's easier to mess with and
> >   makes more sense.
> > - added side-effect is mixed bg forced allocation works with this scheme as
> >   well.
> > - had to add get_btrfs_kobj() to get to fs_info, not sure this is better than
> >   just adding the fs_info to the space_info, am open to other opinions here.
> 
> I believe v3 is the latest version, but I don't see the new file being
> added to the debug/ directory (ie. /sys/fs/btrfs/UUID/debug/)
> 
> As it has been discussed, whether to make the file always visible or
> only with CONFIG_BTRFS_DEBUG, I'd rather keep it debugging-only. The
> testing coverage should be sufficient for fstests that are run with the
> config option and not giving users a knob to paper over allocator
> problems sounds desirable.

Just ignore this one.  The btrfsck patch is more important.

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index e6493b068294..12cff81aa8ea 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -20,6 +20,7 @@ 
 
 static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj);
 static inline struct btrfs_fs_devices *to_fs_devs(struct kobject *kobj);
+static inline struct kobject *get_btrfs_kobj(struct kobject *kobj);
 
 static u64 get_features(struct btrfs_fs_info *fs_info,
 			enum btrfs_feature_set set)
@@ -321,6 +322,58 @@  struct kobj_type btrfs_raid_ktype = {
 	.default_attrs = raid_attributes,
 };
 
+static ssize_t btrfs_space_info_force_chunk_alloc_show(struct kobject *kobj,
+						       struct kobj_attribute *a,
+						       char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0\n");
+}
+
+static ssize_t btrfs_space_info_force_chunk_alloc(struct kobject *kobj,
+						  struct kobj_attribute *a,
+						  const char *buf, size_t len)
+{
+	struct btrfs_space_info *space_info = to_space_info(kobj);
+	struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj));
+	struct btrfs_trans_handle *trans;
+	unsigned long val;
+	int ret;
+
+	if (!fs_info) {
+		printk(KERN_ERR "couldn't get fs_info\n");
+		return -EPERM;
+	}
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (sb_rdonly(fs_info->sb))
+		return -EROFS;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	/*
+	 * We don't really care, but if we echo 0 > force it seems silly to do
+	 * anything.
+	 */
+	if (val == 0)
+		return -EINVAL;
+
+	trans = btrfs_start_transaction(fs_info->extent_root, 0);
+	if (!trans)
+		return PTR_ERR(trans);
+	ret = btrfs_force_chunk_alloc(trans, space_info->flags);
+	btrfs_end_transaction(trans);
+	if (ret == 1)
+		return len;
+	return -ENOSPC;
+}
+BTRFS_ATTR_RW(space_info, force_chunk_alloc,
+	      btrfs_space_info_force_chunk_alloc_show,
+	      btrfs_space_info_force_chunk_alloc);
+
 #define SPACE_INFO_ATTR(field)						\
 static ssize_t btrfs_space_info_show_##field(struct kobject *kobj,	\
 					     struct kobj_attribute *a,	\
@@ -363,6 +416,7 @@  static struct attribute *space_info_attrs[] = {
 	BTRFS_ATTR_PTR(space_info, disk_used),
 	BTRFS_ATTR_PTR(space_info, disk_total),
 	BTRFS_ATTR_PTR(space_info, total_bytes_pinned),
+	BTRFS_ATTR_PTR(space_info, force_chunk_alloc),
 	NULL,
 };
 
@@ -556,6 +610,16 @@  static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj)
 	return to_fs_devs(kobj)->fs_info;
 }
 
+static inline struct kobject *get_btrfs_kobj(struct kobject *kobj)
+{
+	while (kobj) {
+		if (kobj->ktype == &btrfs_ktype)
+			return kobj;
+		kobj = kobj->parent;
+	}
+	return NULL;
+}
+
 #define NUM_FEATURE_BITS 64
 #define BTRFS_FEATURE_NAME_MAX 13
 static char btrfs_unknown_feature_names[FEAT_MAX][NUM_FEATURE_BITS][BTRFS_FEATURE_NAME_MAX];