diff mbox series

[07/15] btrfs: handle fail path for btrfs_sysfs_add_fs_devices

Message ID 32779bc42ae40eea707cc585624724ac91cd967e.1599091832.git.anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series btrfs: seed fix null ptr, use only main device_list_mutex, and cleanups | expand

Commit Message

Anand Jain Sept. 3, 2020, 12:57 a.m. UTC
btrfs_sysfs_add_fs_devices() is called by btrfs_sysfs_add_mounted().
btrfs_sysfs_add_mounted() assumes that btrfs_sysfs_add_fs_devices() will
either add sysfs entries for all the devices or none. So this patch keeps up
to its caller expecatation and cleans up the created sysfs entries if it
has to fail at some device in the list.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Nikolay Borisov Sept. 3, 2020, 9:13 a.m. UTC | #1
On 3.09.20 г. 3:57 ч., Anand Jain wrote:
> btrfs_sysfs_add_fs_devices() is called by btrfs_sysfs_add_mounted().
> btrfs_sysfs_add_mounted() assumes that btrfs_sysfs_add_fs_devices() will
> either add sysfs entries for all the devices or none. So this patch keeps up
> to its caller expecatation and cleans up the created sysfs entries if it
> has to fail at some device in the list.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

I though calling sysfs_remove_link for an uninitialized link could be
problematic but it's not - it will simply return -ENOENT. So

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/sysfs.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 98ce955a0879..a5a73b21d4af 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -1182,10 +1182,12 @@ void btrfs_sysfs_remove_device(struct btrfs_device *device)
>  		sysfs_remove_link(devices_kobj, disk_kobj->name);
>  	}
>  
> -	kobject_del(&device->devid_kobj);
> -	kobject_put(&device->devid_kobj);
> +	if (device->devid_kobj.state_initialized) {
> +		kobject_del(&device->devid_kobj);
> +		kobject_put(&device->devid_kobj);
> +		wait_for_completion(&device->kobj_unregister);
> +	}
>  
> -	wait_for_completion(&device->kobj_unregister);
>  }
>  
>  static ssize_t btrfs_devinfo_in_fs_metadata_show(struct kobject *kobj,
> @@ -1324,19 +1326,21 @@ int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices)
>  
>  	list_for_each_entry(device, &fs_devices->devices, dev_list) {
>  		ret = btrfs_sysfs_add_device(device);
> -		if (ret)
> -			return ret;
> +		goto fail;
>  	}
>  
>  	list_for_each_entry(seed, &fs_devices->seed_list, seed_list) {
>  		list_for_each_entry(device, &seed->devices, dev_list) {
>  			ret = btrfs_sysfs_add_device(device);
> -			if (ret)
> -				return ret;
> +			goto fail;
>  		}
>  	}
>  
>  	return 0;
> +
> +fail:
> +	btrfs_sysfs_remove_fs_devices(fs_devices);
> +	return ret;
>  }
>  
>  void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
>
diff mbox series

Patch

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 98ce955a0879..a5a73b21d4af 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1182,10 +1182,12 @@  void btrfs_sysfs_remove_device(struct btrfs_device *device)
 		sysfs_remove_link(devices_kobj, disk_kobj->name);
 	}
 
-	kobject_del(&device->devid_kobj);
-	kobject_put(&device->devid_kobj);
+	if (device->devid_kobj.state_initialized) {
+		kobject_del(&device->devid_kobj);
+		kobject_put(&device->devid_kobj);
+		wait_for_completion(&device->kobj_unregister);
+	}
 
-	wait_for_completion(&device->kobj_unregister);
 }
 
 static ssize_t btrfs_devinfo_in_fs_metadata_show(struct kobject *kobj,
@@ -1324,19 +1326,21 @@  int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices)
 
 	list_for_each_entry(device, &fs_devices->devices, dev_list) {
 		ret = btrfs_sysfs_add_device(device);
-		if (ret)
-			return ret;
+		goto fail;
 	}
 
 	list_for_each_entry(seed, &fs_devices->seed_list, seed_list) {
 		list_for_each_entry(device, &seed->devices, dev_list) {
 			ret = btrfs_sysfs_add_device(device);
-			if (ret)
-				return ret;
+			goto fail;
 		}
 	}
 
 	return 0;
+
+fail:
+	btrfs_sysfs_remove_fs_devices(fs_devices);
+	return ret;
 }
 
 void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)