Looks good
On Tue, 28 Feb 2012, Alex Elder wrote:
> Here are a few very simple cleanups:
> - Add a "RBD_" prefix to the two driver name string definitions.
> - Move the definition of struct rbd_request below struct rbd_req_coll
> to avoid the need for an empty declaration of the latter.
> - Move and group the definitions of rbd_root_dev_release() and
> rbd_root_dev, as well as rbd_bus_type and rbd_bus_attrs[],
> close to the top of the file. Arrange the latter so
> rbd_bus_type.bus_attrs can be initialized statically.
> - Get rid of an unnecessary local variable in rbd_open().
> - Rework some hokey logic in rbd_bus_add_dev(), so the value of
> "ret" at the end is either 0 or -ENOENT to avoid the need for
> the code duplication that was there.
> - Rename a goto target in rbd_add().
>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxxx>
> ---
> drivers/block/rbd.c | 107
> +++++++++++++++++++++++++--------------------------
> 1 files changed, 53 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index f6dba36..e7d6dfc 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -41,8 +41,8 @@
>
> #include "rbd_types.h"
>
> -#define DRV_NAME "rbd"
> -#define DRV_NAME_LONG "rbd (rados block device)"
> +#define RBD_DRV_NAME "rbd"
> +#define RBD_DRV_NAME_LONG "rbd (rados block device)"
>
> #define RBD_MINORS_PER_MAJOR 256 /* max minors per blkdev */
>
> @@ -83,7 +83,7 @@ struct rbd_options {
> };
>
> /*
> - * an instance of the client. multiple devices may share a client.
> + * an instance of the client. multiple devices may share an rbd client.
> */
> struct rbd_client {
> struct ceph_client *client;
> @@ -92,20 +92,9 @@ struct rbd_client {
> struct list_head node;
> };
>
> -struct rbd_req_coll;
> -
> /*
> - * a single io request
> + * a request completion status
> */
> -struct rbd_request {
> - struct request *rq; /* blk layer request */
> - struct bio *bio; /* cloned bio */
> - struct page **pages; /* list of used pages */
> - u64 len;
> - int coll_index;
> - struct rbd_req_coll *coll;
> -};
> -
> struct rbd_req_status {
> int done;
> int rc;
> @@ -122,6 +111,18 @@ struct rbd_req_coll {
> struct rbd_req_status status[0];
> };
>
> +/*
> + * a single io request
> + */
> +struct rbd_request {
> + struct request *rq; /* blk layer request */
> + struct bio *bio; /* cloned bio */
> + struct page **pages; /* list of used pages */
> + u64 len;
> + int coll_index;
> + struct rbd_req_coll *coll;
> +};
> +
> struct rbd_snap {
> struct device dev;
> const char *name;
> @@ -170,10 +171,6 @@ struct rbd_device {
> struct device dev;
> };
>
> -static struct bus_type rbd_bus_type = {
> - .name = "rbd",
> -};
> -
> static DEFINE_MUTEX(ctl_mutex); /* Serialize
> open/close/setup/teardown */
>
> static LIST_HEAD(rbd_dev_list); /* devices */
> @@ -191,6 +188,31 @@ static ssize_t rbd_snap_add(struct device *dev,
> static void __rbd_remove_snap_dev(struct rbd_device *rbd_dev,
> struct rbd_snap *snap);
>
> +static ssize_t rbd_add(struct bus_type *bus, const char *buf,
> + size_t count);
> +static ssize_t rbd_remove(struct bus_type *bus, const char *buf,
> + size_t count);
> +
> +static struct bus_attribute rbd_bus_attrs[] = {
> + __ATTR(add, S_IWUSR, NULL, rbd_add),
> + __ATTR(remove, S_IWUSR, NULL, rbd_remove),
> + __ATTR_NULL
> +};
> +
> +static struct bus_type rbd_bus_type = {
> + .name = "rbd",
> + .bus_attrs = rbd_bus_attrs,
> +};
> +
> +static void rbd_root_dev_release(struct device *dev)
> +{
> +}
> +
> +static struct device rbd_root_dev = {
> + .init_name = "rbd",
> + .release = rbd_root_dev_release,
> +};
> +
>
> static struct rbd_device *dev_to_rbd(struct device *dev)
> {
> @@ -211,8 +233,7 @@ static int __rbd_update_snaps(struct rbd_device *rbd_dev);
>
> static int rbd_open(struct block_device *bdev, fmode_t mode)
> {
> - struct gendisk *disk = bdev->bd_disk;
> - struct rbd_device *rbd_dev = disk->private_data;
> + struct rbd_device *rbd_dev = bdev->bd_disk->private_data;
>
> rbd_get_dev(rbd_dev);
>
> @@ -1215,8 +1236,8 @@ static void rbd_watch_cb(u64 ver, u64 notify_id, u8
> opcode, void *data)
> rc = __rbd_update_snaps(dev);
> mutex_unlock(&ctl_mutex);
> if (rc)
> - pr_warning(DRV_NAME "%d got notification but failed to update"
> - " snaps: %d\n", dev->major, rc);
> + pr_warning(RBD_DRV_NAME "%d got notification but failed to "
> + " update snaps: %d\n", dev->major, rc);
>
> rbd_req_sync_notify_ack(dev, ver, notify_id, dev->obj_md_name);
> }
> @@ -1746,7 +1767,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
> if (!disk)
> goto out;
>
> - snprintf(disk->disk_name, sizeof(disk->disk_name), DRV_NAME "%d",
> + snprintf(disk->disk_name, sizeof(disk->disk_name), RBD_DRV_NAME "%d",
> rbd_dev->id);
> disk->major = rbd_dev->major;
> disk->first_minor = 0;
> @@ -2092,19 +2113,9 @@ static int __rbd_init_snaps_header(struct rbd_device
> *rbd_dev)
> return 0;
> }
>
> -
> -static void rbd_root_dev_release(struct device *dev)
> -{
> -}
> -
> -static struct device rbd_root_dev = {
> - .init_name = "rbd",
> - .release = rbd_root_dev_release,
> -};
> -
> static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
> {
> - int ret = -ENOMEM;
> + int ret;
> struct device *dev;
> struct rbd_snap *snap;
>
> @@ -2118,7 +2129,7 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
> dev_set_name(dev, "%d", rbd_dev->id);
> ret = device_register(dev);
> if (ret < 0)
> - goto done_free;
> + goto out;
>
> list_for_each_entry(snap, &rbd_dev->snaps, node) {
> ret = rbd_register_snap_dev(rbd_dev, snap,
> @@ -2126,10 +2137,7 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
> if (ret < 0)
> break;
> }
> -
> - mutex_unlock(&ctl_mutex);
> - return 0;
> -done_free:
> +out:
> mutex_unlock(&ctl_mutex);
> return ret;
> }
> @@ -2265,7 +2273,7 @@ static ssize_t rbd_add(struct bus_type *bus,
> mon_dev_name, options, rbd_dev->pool_name,
> rbd_dev->obj, rbd_dev->snap_name) < 4) {
> rc = -EINVAL;
> - goto err_out_slot;
> + goto err_put_id;
> }
>
> if (rbd_dev->snap_name[0] == 0)
> @@ -2277,11 +2285,11 @@ static ssize_t rbd_add(struct bus_type *bus,
> rbd_dev->obj, RBD_SUFFIX);
>
> /* initialize rest of new object */
> - snprintf(rbd_dev->name, DEV_NAME_LEN, DRV_NAME "%d", rbd_dev->id);
> + snprintf(rbd_dev->name, DEV_NAME_LEN, RBD_DRV_NAME "%d", rbd_dev->id);
>
> rc = rbd_get_client(rbd_dev, mon_dev_name, options);
> if (rc < 0)
> - goto err_out_slot;
> + goto err_put_id;
>
> /* pick the pool */
> osdc = &rbd_dev->rbd_client->client->osdc;
> @@ -2327,9 +2335,8 @@ err_out_blkdev:
> unregister_blkdev(rbd_dev->major, rbd_dev->name);
> err_out_client:
> rbd_put_client(rbd_dev);
> -err_out_slot:
> +err_put_id:
> rbd_id_put(rbd_dev);
> -
> kfree(rbd_dev);
> err_out_opt:
> kfree(options);
> @@ -2460,12 +2467,6 @@ err_unlock:
> return ret;
> }
>
> -static struct bus_attribute rbd_bus_attrs[] = {
> - __ATTR(add, S_IWUSR, NULL, rbd_add),
> - __ATTR(remove, S_IWUSR, NULL, rbd_remove),
> - __ATTR_NULL
> -};
> -
> /*
> * create control files in sysfs
> * /sys/bus/rbd/...
> @@ -2474,8 +2475,6 @@ static int rbd_sysfs_init(void)
> {
> int ret;
>
> - rbd_bus_type.bus_attrs = rbd_bus_attrs;
> -
> ret = bus_register(&rbd_bus_type);
> if (ret < 0)
> return ret;
> @@ -2498,7 +2497,7 @@ int __init rbd_init(void)
> rc = rbd_sysfs_init();
> if (rc)
> return rc;
> - pr_info("loaded " DRV_NAME_LONG "\n");
> + pr_info("loaded " RBD_DRV_NAME_LONG "\n");
> return 0;
> }
>
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
