[PATCH 0/4] rbd: reorder some setup operations

[PATCH 0/4] rbd: reorder some setup operations

[Alex Elder]

The first patch of this series doesn't really belong,
but I threw it in anyway.  The other three defer doing
some initialization of an rbd device until just before
it gets activated by a call to add_disk().

					-Alex

[PATCH 1/4] rbd: fix up some sysfs stuff
[PATCH 2/4] rbd: only set device exists flag when ready
[PATCH 3/4] rbd: defer setting disk capacity
[PATCH 4/4] rbd: defer setting the mapping size and features

[PATCH 1/4] rbd: fix up some sysfs stuff

[Alex Elder]

This just tweaks a few things in the routines that implement
rbd sysfs files.

All of the entries for an rbd device in /sys/bus/rbd/devices/<id>/
will represent information whose valid values are known by the time
they are accessible.

Right now we get the size of the mapped image by a call to
get_capacity().  There's no need to do this, because that will
return what we last set the capacity to, which is just the size
recorded for the mapping.  So just show that value instead.

We also get this under protection of the header semaphore, in order
to provide a precisely correct value.  This isn't really necessary;
these files are really informational only and it's not necessary to
be so careful.

Finally, print a special value in case the major device number is
not recorded.  Right now that won't matter much but soon the parent
images won't have devices associated with them.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 37d9349..b6f32c2 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3170,13 +3170,9 @@ static ssize_t rbd_size_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
-	sector_t size;

-	down_read(&rbd_dev->header_rwsem);
-	size = get_capacity(rbd_dev->disk);
-	up_read(&rbd_dev->header_rwsem);
-
-	return sprintf(buf, "%llu\n", (unsigned long long) size * SECTOR_SIZE);
+	return sprintf(buf, "%llu\n",
+		(unsigned long long)rbd_dev->mapping.size);
 }

 /*
@@ -3189,7 +3185,7 @@ static ssize_t rbd_features_show(struct device *dev,
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);

 	return sprintf(buf, "0x%016llx\n",
-			(unsigned long long) rbd_dev->mapping.features);
+			(unsigned long long)rbd_dev->mapping.features);
 }

 static ssize_t rbd_major_show(struct device *dev,
@@ -3197,7 +3193,11 @@ static ssize_t rbd_major_show(struct device *dev,
 {
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);

-	return sprintf(buf, "%d\n", rbd_dev->major);
+	if (rbd_dev->major)
+		return sprintf(buf, "%d\n", rbd_dev->major);
+
+	return sprintf(buf, "(none)\n");
+
 }

 static ssize_t rbd_client_id_show(struct device *dev,
@@ -3223,7 +3223,7 @@ static ssize_t rbd_pool_id_show(struct device *dev,
 	struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);

 	return sprintf(buf, "%llu\n",
-		(unsigned long long) rbd_dev->spec->pool_id);
+			(unsigned long long) rbd_dev->spec->pool_id);
 }

 static ssize_t rbd_name_show(struct device *dev,
-- 
1.7.9.5

[PATCH 2/4] rbd: only set device exists flag when ready

[Alex Elder]

Hold off setting the EXISTS rbd device flag until just before we
announce the disk as available for use.  There's no point in doing
so any earlier than that, and at that point the device truly is
fully set up and ready to use.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b6f32c2..b04e3e1 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -881,7 +881,6 @@ static int rbd_dev_set_mapping(struct rbd_device
*rbd_dev)
 		rbd_dev->mapping.features = snap->features;
 		rbd_dev->mapping.read_only = true;
 	}
-	set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);

 	return 0;
 }
@@ -4779,6 +4778,7 @@ static int rbd_dev_probe_finish(struct rbd_device
*rbd_dev)

 	/* Everything's ready.  Announce the disk to the world. */

+	set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
 	add_disk(rbd_dev->disk);

 	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
-- 
1.7.9.5

[PATCH 3/4] rbd: defer setting disk capacity

[Alex Elder]

Don't set the disk capacity until right before we announce the
device as available for use.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b04e3e1..12bc89f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3147,8 +3147,6 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)

 	rbd_dev->disk = disk;

-	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
-
 	return 0;
 out_disk:
 	put_disk(disk);
@@ -4778,6 +4776,7 @@ static int rbd_dev_probe_finish(struct rbd_device
*rbd_dev)

 	/* Everything's ready.  Announce the disk to the world. */

+	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
 	set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
 	add_disk(rbd_dev->disk);

-- 
1.7.9.5

[PATCH 4/4] rbd: defer setting the mapping size and features

[Alex Elder]

There's no need to set the size/features for a mapped device until
we're about to make it available for use.  So wait until then to do
it.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 12bc89f..b10348c 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4712,10 +4712,6 @@ static int rbd_dev_probe_finish(struct rbd_device
*rbd_dev)
 	if (ret)
 		goto err_out_snaps;

-	ret = rbd_dev_set_mapping(rbd_dev);
-	if (ret)
-		goto err_out_snaps;
-
 	/* generate unique id: find highest unique id, add one */
 	rbd_dev_id_get(rbd_dev);

@@ -4774,10 +4770,15 @@ static int rbd_dev_probe_finish(struct
rbd_device *rbd_dev)
 	if (ret)
 		goto err_out_bus;

-	/* Everything's ready.  Announce the disk to the world. */
+	ret = rbd_dev_set_mapping(rbd_dev);
+	if (ret)
+		goto err_out_bus;

 	set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
 	set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
+
+	/* Everything's ready.  Announce the disk to the world. */
+
 	add_disk(rbd_dev->disk);

 	pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
-- 
1.7.9.5

Re: [PATCH 0/4] rbd: reorder some setup operations

[Josh Durgin]

On 04/27/2013 05:31 AM, Alex Elder wrote:
> The first patch of this series doesn't really belong,
> but I threw it in anyway.  The other three defer doing
> some initialization of an rbd device until just before
> it gets activated by a call to add_disk().
>
> 					-Alex
>
> [PATCH 1/4] rbd: fix up some sysfs stuff
> [PATCH 2/4] rbd: only set device exists flag when ready
> [PATCH 3/4] rbd: defer setting disk capacity
> [PATCH 4/4] rbd: defer setting the mapping size and features

These all look good.

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>