[PATCH 0/7] rbd: drop unused parameters from functions

[PATCH 0/7] rbd: drop unused parameters from functions

[Alex Elder]

All the patches in this series just drop unused parameters from
their functions.

					-Alex

[PATCH 1/7] rbd: snapc is unused in rbd_req_sync_read()
[PATCH 2/7] rbd: drop rbd_header_from_disk() gfp_flags parameter
[PATCH 3/7] rbd: drop rbd_dev parameter in snap functions
[PATCH 4/7] rbd: drop "object_name" from rbd_req_sync_watch()
[PATCH 5/7] rbd: drop "object_name" from rbd_req_sync_notify()
[PATCH 6/7] rbd: drop "object_name" from rbd_req_sync_notify_ack()
[PATCH 7/7] rbd: drop "object_name" from rbd_req_sync_unwatch()

[PATCH 1/7] rbd: snapc is unused in rbd_req_sync_read()

[Alex Elder]

The "snapc" parameter to in rbd_req_sync_read() is not used, so
get rid of it.

Reported-by: Josh Durgin <josh.durgin@inktank.com>
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 0fda2ed..64d3dbd 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1168,7 +1168,6 @@ static int rbd_req_read(struct request *rq,
  * Request sync osd read
  */
 static int rbd_req_sync_read(struct rbd_device *rbd_dev,
-			  struct ceph_snap_context *snapc,
 			  u64 snapid,
 			  const char *object_name,
 			  u64 ofs, u64 len,
@@ -1615,7 +1614,7 @@ static int rbd_read_header(struct rbd_device *rbd_dev,
 			return -ENOMEM;

 		rc = rbd_req_sync_read(rbd_dev,
-				       NULL, CEPH_NOSNAP,
+				       CEPH_NOSNAP,
 				       rbd_dev->header_name,
 				       0, len,
 				       (char *)dh, &ver);
-- 
1.7.9.5

[PATCH 2/7] rbd: drop rbd_header_from_disk() gfp_flags parameter

[Alex Elder]

The function rbd_header_from_disk() is only called in one spot, and
it passes GFP_KERNEL as its value for the gfp_flags parameter.

Just drop that parameter and substitute GFP_KERNEL everywhere within
that function it had been used.  (If we find we need the parameter
again in the future it's easy enough to add back again.)

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 64d3dbd..597ebaa 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -493,8 +493,7 @@ static bool rbd_dev_ondisk_valid(struct
rbd_image_header_ondisk *ondisk)
  */
 static int rbd_header_from_disk(struct rbd_image_header *header,
 				 struct rbd_image_header_ondisk *ondisk,
-				 u32 allocated_snaps,
-				 gfp_t gfp_flags)
+				 u32 allocated_snaps)
 {
 	u32 i, snap_count;

@@ -507,18 +506,18 @@ static int rbd_header_from_disk(struct
rbd_image_header *header,
 		return -EINVAL;
 	header->snapc = kmalloc(sizeof(struct ceph_snap_context) +
 				snap_count * sizeof(u64),
-				gfp_flags);
+				GFP_KERNEL);
 	if (!header->snapc)
 		return -ENOMEM;

 	header->snap_names_len = le64_to_cpu(ondisk->snap_names_len);
 	if (snap_count) {
 		header->snap_names = kmalloc(header->snap_names_len,
-					     gfp_flags);
+					     GFP_KERNEL);
 		if (!header->snap_names)
 			goto err_snapc;
 		header->snap_sizes = kmalloc(snap_count * sizeof(u64),
-					     gfp_flags);
+					     GFP_KERNEL);
 		if (!header->snap_sizes)
 			goto err_names;
 	} else {
@@ -527,7 +526,7 @@ static int rbd_header_from_disk(struct
rbd_image_header *header,
 	}

 	header->object_prefix = kmalloc(sizeof (ondisk->block_name) + 1,
-					gfp_flags);
+					GFP_KERNEL);
 	if (!header->object_prefix)
 		goto err_sizes;

@@ -1621,7 +1620,7 @@ static int rbd_read_header(struct rbd_device *rbd_dev,
 		if (rc < 0)
 			goto out_dh;

-		rc = rbd_header_from_disk(header, dh, snap_count, GFP_KERNEL);
+		rc = rbd_header_from_disk(header, dh, snap_count);
 		if (rc < 0) {
 			if (rc == -ENXIO)
 				pr_warning("unrecognized header format"
-- 
1.7.9.5

[PATCH 3/7] rbd: drop rbd_dev parameter in snap functions

[Alex Elder]

Both rbd_register_snap_dev() and __rbd_remove_snap_dev() have
rbd_dev parameters that are unused.  Remove them.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 597ebaa..a748448 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -202,8 +202,7 @@ static ssize_t rbd_snap_add(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf,
 			    size_t count);
-static void __rbd_remove_snap_dev(struct rbd_device *rbd_dev,
-				  struct rbd_snap *snap);
+static void __rbd_remove_snap_dev(struct rbd_snap *snap);

 static ssize_t rbd_add(struct bus_type *bus, const char *buf,
 		       size_t count);
@@ -1698,7 +1697,7 @@ static void __rbd_remove_all_snaps(struct
rbd_device *rbd_dev)
 	struct rbd_snap *next;

 	list_for_each_entry_safe(snap, next, &rbd_dev->snaps, node)
-		__rbd_remove_snap_dev(rbd_dev, snap);
+		__rbd_remove_snap_dev(snap);
 }

 /*
@@ -2006,15 +2005,13 @@ static struct device_type rbd_snap_device_type = {
 	.release	= rbd_snap_dev_release,
 };

-static void __rbd_remove_snap_dev(struct rbd_device *rbd_dev,
-				  struct rbd_snap *snap)
+static void __rbd_remove_snap_dev(struct rbd_snap *snap)
 {
 	list_del(&snap->node);
 	device_unregister(&snap->dev);
 }

-static int rbd_register_snap_dev(struct rbd_device *rbd_dev,
-				  struct rbd_snap *snap,
+static int rbd_register_snap_dev(struct rbd_snap *snap,
 				  struct device *parent)
 {
 	struct device *dev = &snap->dev;
@@ -2041,8 +2038,7 @@ static int __rbd_add_snap_dev(struct rbd_device
*rbd_dev,
 	snap->size = rbd_dev->header.snap_sizes[i];
 	snap->id = rbd_dev->header.snapc->snaps[i];
 	if (device_is_registered(&rbd_dev->dev)) {
-		ret = rbd_register_snap_dev(rbd_dev, snap,
-					     &rbd_dev->dev);
+		ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
 		if (ret < 0)
 			goto err;
 	}
@@ -2107,7 +2103,7 @@ static int __rbd_init_snaps_header(struct
rbd_device *rbd_dev)
 			 */
 			if (rbd_dev->snap_id == old_snap->id)
 				rbd_dev->snap_exists = false;
-			__rbd_remove_snap_dev(rbd_dev, old_snap);
+			__rbd_remove_snap_dev(old_snap);
 			continue;
 		}
 		if (old_snap->id == cur_id) {
@@ -2171,8 +2167,7 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
 		goto out;

 	list_for_each_entry(snap, &rbd_dev->snaps, node) {
-		ret = rbd_register_snap_dev(rbd_dev, snap,
-					     &rbd_dev->dev);
+		ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
 		if (ret < 0)
 			break;
 	}
-- 
1.7.9.5

[PATCH 4/7] rbd: drop "object_name" from rbd_req_sync_watch()

[Alex Elder]

rbd_req_sync_watch() is only called in one place, and in that place
it passes rbd_dev->header_name as the value of the "object_name"
parameter.  This value is available within the function already.

Having the extra parameter leaves the impression the object name
could take on different values, but it does not.

So get rid of the parameter.  We can always add it back again if
we find we want to watch some other object in the future.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index a748448..5cfeeaf 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1236,9 +1236,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
 /*
  * Request sync osd watch
  */
-static int rbd_req_sync_watch(struct rbd_device *rbd_dev,
-			      const char *object_name,
-			      u64 ver)
+static int rbd_req_sync_watch(struct rbd_device *rbd_dev)
 {
 	struct ceph_osd_req_op *ops;
 	struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
@@ -1252,7 +1250,7 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev,
 	if (ret < 0)
 		goto fail;

-	ops[0].watch.ver = cpu_to_le64(ver);
+	ops[0].watch.ver = cpu_to_le64(rbd_dev->header.obj_version);
 	ops[0].watch.cookie = cpu_to_le64(rbd_dev->watch_event->cookie);
 	ops[0].watch.flag = 1;

@@ -1261,7 +1259,8 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev,
 			      0,
 			      CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
 			      ops,
-			      object_name, 0, 0, NULL,
+			      rbd_dev->header_name,
+			      0, 0, NULL,
 			      &rbd_dev->watch_request, NULL);

 	if (ret < 0)
@@ -2186,8 +2185,7 @@ static int rbd_init_watch_dev(struct rbd_device
*rbd_dev)
 	int ret, rc;

 	do {
-		ret = rbd_req_sync_watch(rbd_dev, rbd_dev->header_name,
-					 rbd_dev->header.obj_version);
+		ret = rbd_req_sync_watch(rbd_dev);
 		if (ret == -ERANGE) {
 			mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
 			rc = __rbd_refresh_header(rbd_dev);
-- 
1.7.9.5

[PATCH 5/7] rbd: drop "object_name" from rbd_req_sync_notify()

[Alex Elder]

rbd_req_sync_notify() only ever uses rbd_dev->header_name as the
value of its "object_name" parameter, and that value is available
within the function already.  So get rid of the parameter.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 5cfeeaf..b9aa377 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1323,8 +1323,7 @@ static void rbd_notify_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
 /*
  * Request sync osd notify
  */
-static int rbd_req_sync_notify(struct rbd_device *rbd_dev,
-		          const char *object_name)
+static int rbd_req_sync_notify(struct rbd_device *rbd_dev)
 {
 	struct ceph_osd_req_op *ops;
 	struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
@@ -1355,7 +1354,8 @@ static int rbd_req_sync_notify(struct rbd_device
*rbd_dev,
 			       0,
 			       CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
 			       ops,
-			       object_name, 0, 0, NULL, NULL, NULL);
+			       rbd_dev->header_name,
+			       0, 0, NULL, NULL, NULL);
 	if (ret < 0)
 		goto fail_event;

@@ -2647,7 +2647,7 @@ static ssize_t rbd_snap_add(struct device *dev,
 	mutex_unlock(&ctl_mutex);

 	/* make a best effort, don't error if failed */
-	rbd_req_sync_notify(rbd_dev, rbd_dev->header_name);
+	rbd_req_sync_notify(rbd_dev);

 	ret = count;
 	kfree(name);
-- 
1.7.9.5

[PATCH 6/7] rbd: drop "object_name" from rbd_req_sync_notify_ack()

[Alex Elder]

rbd_req_sync_notify_ack() only ever uses rbd_dev->header_name as the
value of its "object_name" parameter, and that value is available
within the function already.  So get rid of the parameter.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b9aa377..48a7927 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1185,8 +1185,7 @@ static int rbd_req_sync_read(struct rbd_device
*rbd_dev,
  */
 static int rbd_req_sync_notify_ack(struct rbd_device *rbd_dev,
 				   u64 ver,
-				   u64 notify_id,
-				   const char *object_name)
+				   u64 notify_id)
 {
 	struct ceph_osd_req_op *ops;
 	int ret;
@@ -1200,7 +1199,7 @@ static int rbd_req_sync_notify_ack(struct
rbd_device *rbd_dev,
 	ops[0].watch.flag = 0;

 	ret = rbd_do_request(NULL, rbd_dev, NULL, CEPH_NOSNAP,
-			  object_name, 0, 0, NULL,
+			  rbd_dev->header_name, 0, 0, NULL,
 			  NULL, 0,
 			  CEPH_OSD_FLAG_READ,
 			  ops,
@@ -1230,7 +1229,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
 		pr_warning(RBD_DRV_NAME "%d got notification but failed to "
 			   " update snaps: %d\n", rbd_dev->major, rc);

-	rbd_req_sync_notify_ack(rbd_dev, hver, notify_id, rbd_dev->header_name);
+	rbd_req_sync_notify_ack(rbd_dev, hver, notify_id);
 }

 /*
-- 
1.7.9.5

[PATCH 7/7] rbd: drop "object_name" from rbd_req_sync_unwatch()

[Alex Elder]

rbd_req_sync_unwatch() only ever uses rbd_dev->header_name as the
value of its "object_name" parameter, and that value is available
within the function already.  So get rid of the parameter.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 48a7927..fd5f3038 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1279,8 +1279,7 @@ fail:
 /*
  * Request sync osd unwatch
  */
-static int rbd_req_sync_unwatch(struct rbd_device *rbd_dev,
-				const char *object_name)
+static int rbd_req_sync_unwatch(struct rbd_device *rbd_dev)
 {
 	struct ceph_osd_req_op *ops;

@@ -1297,7 +1296,9 @@ static int rbd_req_sync_unwatch(struct rbd_device
*rbd_dev,
 			      0,
 			      CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
 			      ops,
-			      object_name, 0, 0, NULL, NULL, NULL);
+			      rbd_dev->header_name,
+			      0, 0, NULL, NULL, NULL);
+

 	rbd_destroy_ops(ops);
 	ceph_osdc_cancel_event(rbd_dev->watch_event);
@@ -2563,7 +2564,7 @@ static void rbd_dev_release(struct device *dev)
 						    rbd_dev->watch_request);
 	}
 	if (rbd_dev->watch_event)
-		rbd_req_sync_unwatch(rbd_dev, rbd_dev->header_name);
+		rbd_req_sync_unwatch(rbd_dev);

 	rbd_put_client(rbd_dev);

-- 
1.7.9.5

Re: [PATCH 0/7] rbd: drop unused parameters from functions

[Josh Durgin]

On 07/26/2012 11:46 AM, Alex Elder wrote:
> All the patches in this series just drop unused parameters from
> their functions.
>
> 					-Alex
>
> [PATCH 1/7] rbd: snapc is unused in rbd_req_sync_read()
> [PATCH 2/7] rbd: drop rbd_header_from_disk() gfp_flags parameter
> [PATCH 3/7] rbd: drop rbd_dev parameter in snap functions
> [PATCH 4/7] rbd: drop "object_name" from rbd_req_sync_watch()
> [PATCH 5/7] rbd: drop "object_name" from rbd_req_sync_notify()
> [PATCH 6/7] rbd: drop "object_name" from rbd_req_sync_notify_ack()
> [PATCH 7/7] rbd: drop "object_name" from rbd_req_sync_unwatch()

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