[1/3] block: introduce device_add_disk()
diff mbox

Message ID 20160225200443.21975.77561.stgit@dwillia2-desk3.amr.corp.intel.com
State New
Headers show

Commit Message

Dan Williams Feb. 25, 2016, 8:04 p.m. UTC
In preparation for removing the ->driverfs_dev member of a gendisk, add
an api that takes the parent device as a parameter to add_disk().

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 block/genhd.c         |   22 ++++++++++------------
 include/linux/genhd.h |    8 +++++++-
 2 files changed, 17 insertions(+), 13 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig March 10, 2016, 8:09 a.m. UTC | #1
>  /**
> - * add_disk - add partitioning information to kernel list
> + * device_add_disk - add partitioning information to kernel list
> + * @parent: parent device for the disk
>   * @disk: per-device partitioning information
>   *
>   * This function registers the partitioning information in @disk
> @@ -581,7 +582,7 @@ exit:
>   *
>   * FIXME: error handling
>   */
> -void add_disk(struct gendisk *disk)
> +int device_add_disk(struct device *parent, struct gendisk *disk)
>  {
>  	struct backing_dev_info *bdi;
>  	dev_t devt;
> @@ -597,10 +598,8 @@ void add_disk(struct gendisk *disk)
>  	disk->flags |= GENHD_FL_UP;
>  
>  	retval = blk_alloc_devt(&disk->part0, &devt);
> -	if (retval) {
> -		WARN_ON(1);
> -		return;
> -	}
> +	if (retval)
> +		return retval;
>  	disk_to_dev(disk)->devt = devt;
>  
>  	/* ->major and ->first_minor aren't supposed to be
> @@ -617,7 +616,7 @@ void add_disk(struct gendisk *disk)
>  
>  	blk_register_region(disk_devt(disk), disk->minors, NULL,
>  			    exact_match, exact_lock, disk);
> -	register_disk(disk);
> +	register_disk(parent, disk);
>  	blk_register_queue(disk);
>  
>  	/*
> @@ -628,12 +627,11 @@ void add_disk(struct gendisk *disk)
>  
>  	retval = sysfs_create_link(&disk_to_dev(disk)->kobj, &bdi->dev->kobj,
>  				   "bdi");
> -	WARN_ON(retval);
> -
>  	disk_add_events(disk);
>  	blk_integrity_add(disk);
> +	return retval;

How are the callers expected to unwind after already having added the
disk?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams March 10, 2016, 3:15 p.m. UTC | #2
On Thu, Mar 10, 2016 at 12:09 AM, Christoph Hellwig <hch@lst.de> wrote:
>>  /**
>> - * add_disk - add partitioning information to kernel list
>> + * device_add_disk - add partitioning information to kernel list
>> + * @parent: parent device for the disk
>>   * @disk: per-device partitioning information
>>   *
>>   * This function registers the partitioning information in @disk
>> @@ -581,7 +582,7 @@ exit:
>>   *
>>   * FIXME: error handling
>>   */
>> -void add_disk(struct gendisk *disk)
>> +int device_add_disk(struct device *parent, struct gendisk *disk)
>>  {
>>       struct backing_dev_info *bdi;
>>       dev_t devt;
>> @@ -597,10 +598,8 @@ void add_disk(struct gendisk *disk)
>>       disk->flags |= GENHD_FL_UP;
>>
>>       retval = blk_alloc_devt(&disk->part0, &devt);
>> -     if (retval) {
>> -             WARN_ON(1);
>> -             return;
>> -     }
>> +     if (retval)
>> +             return retval;
>>       disk_to_dev(disk)->devt = devt;
>>
>>       /* ->major and ->first_minor aren't supposed to be
>> @@ -617,7 +616,7 @@ void add_disk(struct gendisk *disk)
>>
>>       blk_register_region(disk_devt(disk), disk->minors, NULL,
>>                           exact_match, exact_lock, disk);
>> -     register_disk(disk);
>> +     register_disk(parent, disk);
>>       blk_register_queue(disk);
>>
>>       /*
>> @@ -628,12 +627,11 @@ void add_disk(struct gendisk *disk)
>>
>>       retval = sysfs_create_link(&disk_to_dev(disk)->kobj, &bdi->dev->kobj,
>>                                  "bdi");
>> -     WARN_ON(retval);
>> -
>>       disk_add_events(disk);
>>       blk_integrity_add(disk);
>> +     return retval;
>
> How are the callers expected to unwind after already having added the
> disk?

True, I overlooked making add_disk() unwind-able, will re-spin...
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig March 10, 2016, 3:18 p.m. UTC | #3
On Thu, Mar 10, 2016 at 07:15:19AM -0800, Dan Williams wrote:
> > How are the callers expected to unwind after already having added the
> > disk?
> 
> True, I overlooked making add_disk() unwind-able, will re-spin...

I think this will take more work than we can finish in the next
days.  It would be good to simply respin it without adding the error
propagation.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams March 10, 2016, 3:27 p.m. UTC | #4
On Thu, Mar 10, 2016 at 7:18 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Mar 10, 2016 at 07:15:19AM -0800, Dan Williams wrote:
>> > How are the callers expected to unwind after already having added the
>> > disk?
>>
>> True, I overlooked making add_disk() unwind-able, will re-spin...
>
> I think this will take more work than we can finish in the next
> days.  It would be good to simply respin it without adding the error
> propagation.

Sounds good.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/block/genhd.c b/block/genhd.c
index eb05dfc1dffe..3fa2654ada52 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -506,7 +506,7 @@  static int exact_lock(dev_t devt, void *data)
 	return 0;
 }
 
-static void register_disk(struct gendisk *disk)
+static void register_disk(struct device *parent, struct gendisk *disk)
 {
 	struct device *ddev = disk_to_dev(disk);
 	struct block_device *bdev;
@@ -514,7 +514,7 @@  static void register_disk(struct gendisk *disk)
 	struct hd_struct *part;
 	int err;
 
-	ddev->parent = disk->driverfs_dev;
+	ddev->parent = parent;
 
 	dev_set_name(ddev, "%s", disk->disk_name);
 
@@ -573,7 +573,8 @@  exit:
 }
 
 /**
- * add_disk - add partitioning information to kernel list
+ * device_add_disk - add partitioning information to kernel list
+ * @parent: parent device for the disk
  * @disk: per-device partitioning information
  *
  * This function registers the partitioning information in @disk
@@ -581,7 +582,7 @@  exit:
  *
  * FIXME: error handling
  */
-void add_disk(struct gendisk *disk)
+int device_add_disk(struct device *parent, struct gendisk *disk)
 {
 	struct backing_dev_info *bdi;
 	dev_t devt;
@@ -597,10 +598,8 @@  void add_disk(struct gendisk *disk)
 	disk->flags |= GENHD_FL_UP;
 
 	retval = blk_alloc_devt(&disk->part0, &devt);
-	if (retval) {
-		WARN_ON(1);
-		return;
-	}
+	if (retval)
+		return retval;
 	disk_to_dev(disk)->devt = devt;
 
 	/* ->major and ->first_minor aren't supposed to be
@@ -617,7 +616,7 @@  void add_disk(struct gendisk *disk)
 
 	blk_register_region(disk_devt(disk), disk->minors, NULL,
 			    exact_match, exact_lock, disk);
-	register_disk(disk);
+	register_disk(parent, disk);
 	blk_register_queue(disk);
 
 	/*
@@ -628,12 +627,11 @@  void add_disk(struct gendisk *disk)
 
 	retval = sysfs_create_link(&disk_to_dev(disk)->kobj, &bdi->dev->kobj,
 				   "bdi");
-	WARN_ON(retval);
-
 	disk_add_events(disk);
 	blk_integrity_add(disk);
+	return retval;
 }
-EXPORT_SYMBOL(add_disk);
+EXPORT_SYMBOL(device_add_disk);
 
 void del_gendisk(struct gendisk *disk)
 {
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 5c706765404a..6fa03fe589ad 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -431,7 +431,13 @@  static inline void free_part_info(struct hd_struct *part)
 extern void part_round_stats(int cpu, struct hd_struct *part);
 
 /* block/genhd.c */
-extern void add_disk(struct gendisk *disk);
+extern int device_add_disk(struct device *parent, struct gendisk *disk);
+static inline void add_disk(struct gendisk *disk)
+{
+	/* temporary while we convert callers to device_add_disk */
+	WARN_ON(device_add_disk(disk->driverfs_dev, disk) != 0);
+}
+
 extern void del_gendisk(struct gendisk *gp);
 extern struct gendisk *get_gendisk(dev_t dev, int *partno);
 extern struct block_device *bdget_disk(struct gendisk *disk, int partno);