diff mbox series

[8/8] btrfs: sysfs: Add bdi link to the fsid dir

Message ID 20200702122335.9117-9-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Corrupt counter improvement | expand

Commit Message

Nikolay Borisov July 2, 2020, 12:23 p.m. UTC
Since BTRFS uses a private bdi it makes sense to create a link to this
bdi under /sys/fs/btrfs/<UUID>/bdi. This allows size of read ahead to
be controlled. Without this patch it's not possible to uniquely identify
which bdi pertains to which btrfs filesystem in the fase of multiple
btrfs filesystem.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h |  2 ++
 fs/btrfs/sysfs.c | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Josef Bacik July 2, 2020, 1:25 p.m. UTC | #1
On 7/2/20 8:23 AM, Nikolay Borisov wrote:
> Since BTRFS uses a private bdi it makes sense to create a link to this
> bdi under /sys/fs/btrfs/<UUID>/bdi. This allows size of read ahead to
> be controlled. Without this patch it's not possible to uniquely identify
> which bdi pertains to which btrfs filesystem in the fase of multiple
> btrfs filesystem.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Was confused why we needed to make sure the link existed before removing it, 
since other things sysfs is smart enough to figure out.  Apparently it has a 
WARN_ON() if the parent isn't initialized, so the check is necessary, albeit 
annoying.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
David Sterba July 2, 2020, 1:36 p.m. UTC | #2
On Thu, Jul 02, 2020 at 09:25:30AM -0400, Josef Bacik wrote:
> On 7/2/20 8:23 AM, Nikolay Borisov wrote:
> > Since BTRFS uses a private bdi it makes sense to create a link to this
> > bdi under /sys/fs/btrfs/<UUID>/bdi. This allows size of read ahead to
> > be controlled. Without this patch it's not possible to uniquely identify
> > which bdi pertains to which btrfs filesystem in the fase of multiple
> > btrfs filesystem.
> > 
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Was confused why we needed to make sure the link existed before removing it, 
> since other things sysfs is smart enough to figure out.  Apparently it has a 
> WARN_ON() if the parent isn't initialized, so the check is necessary, albeit 
> annoying.

There must be a better way, this is just too weird. We can check if
objects have been initialized by peeking to kobject::state_initialized
and we use that already for fsid_kobj in __btrfs_sysfs_remove_fsid or
btrfs_sysfs_feature_update.
Nikolay Borisov July 2, 2020, 2:41 p.m. UTC | #3
On 2.07.20 г. 16:36 ч., David Sterba wrote:
> On Thu, Jul 02, 2020 at 09:25:30AM -0400, Josef Bacik wrote:
>> On 7/2/20 8:23 AM, Nikolay Borisov wrote:
>>> Since BTRFS uses a private bdi it makes sense to create a link to this
>>> bdi under /sys/fs/btrfs/<UUID>/bdi. This allows size of read ahead to
>>> be controlled. Without this patch it's not possible to uniquely identify
>>> which bdi pertains to which btrfs filesystem in the fase of multiple
>>> btrfs filesystem.
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>
>> Was confused why we needed to make sure the link existed before removing it, 
>> since other things sysfs is smart enough to figure out.  Apparently it has a 
>> WARN_ON() if the parent isn't initialized, so the check is necessary, albeit 
>> annoying.
> 
> There must be a better way, this is just too weird. We can check if
> objects have been initialized by peeking to kobject::state_initialized
> and we use that already for fsid_kobj in __btrfs_sysfs_remove_fsid or
> btrfs_sysfs_feature_update.
> 

Awesome, will use this for v2.
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4dd478b4fe3a..eb61f89e9e85 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -928,6 +928,8 @@  struct btrfs_fs_info {
 	u32 sectorsize;
 	u32 stripesize;
 
+	bool bdi_link_created;
+
 	/* Block groups and devices containing active swapfiles. */
 	spinlock_t swapfile_pins_lock;
 	struct rb_root swapfile_pins;
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 5885abe57c3e..e167ec584627 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -937,8 +937,13 @@  void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
 
 void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 {
+	struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
+
 	btrfs_reset_fs_info_ptr(fs_info);
 
+	if (fs_info->bdi_link_created)
+		sysfs_remove_link(fsid_kobj, "bdi");
+
 	if (fs_info->space_info_kobj) {
 		sysfs_remove_files(fs_info->space_info_kobj, allocation_attrs);
 		kobject_del(fs_info->space_info_kobj);
@@ -958,8 +963,8 @@  void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 	}
 #endif
 	addrm_unknown_feature_attrs(fs_info, false);
-	sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group);
-	sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs);
+	sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
+	sysfs_remove_files(fsid_kobj, btrfs_attrs);
 	btrfs_sysfs_remove_devices_dir(fs_info->fs_devices, NULL);
 }
 
@@ -1410,6 +1415,13 @@  int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 	if (error)
 		goto failure;
 
+	error = sysfs_create_link(fsid_kobj, &fs_info->sb->s_bdi->dev->kobj,
+				  "bdi");
+	if (error)
+		goto failure;
+
+	fs_info->bdi_link_created = true;
+
 #ifdef CONFIG_BTRFS_DEBUG
 	fs_info->debug_kobj = kobject_create_and_add("debug", fsid_kobj);
 	if (!fs_info->debug_kobj) {