Commit 3e83b65bb9a9f3a4d7f0200139bd947c940ec3ab

Authored by Alex Elder
Committed by Sage Weil
1 parent 9ef1ee5a1b

rbd: don't create sysfs entries for non-mapped snapshots

When an rbd image gets mapped a device entry gets created for it
under /sys/bus/rbd/devices/<id>/.  Inside that directory there are
sysfs files that contain information about the image: its size,
feature bits, major device number, and so on.

Additionally, if that image has any snapshots, a device entry gets
created for each of those as a "child" of the mapped device.  Each
of these is a subdirectory of the mapped device, and each directory
contains a few files with information about the snapshot (its
snapshot id, size, and feature mask).

There is no clear benefit to having those device entries for the
snapshots.  The information provided via sysfs of of little real
value--and all of it is available via rbd CLI commands.  If we
still wanted to see the kernel's view of this information it could
be done much more simply by including it in a single sysfs file for
the mapped image.

But there *is* a clear cost to supporting them.  Every time a snapshot
context changes, these entries need to be updated (deleted snapshots
removed, new snapshots created).  The rbd driver is notified of
changes to the snapshot context via callbacks from an osd, and care
must be taken to coordinate removal of snapshot data structures
with the possibility of one these notifications occurring.

Things would be considerably simpler if we just didn't have to
maintain device entries for the snapshots.

So get rid of them.

The ability to map a snapshot of an rbd image will remain; the only
thing lost will be the ability to query these sysfs directories for
information about snapshots of mapped images.

This resolves:
    http://tracker.ceph.com/issues/4796

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

Showing 2 changed files with 4 additions and 153 deletions Side-by-side Diff

Documentation/ABI/testing/sysfs-bus-rbd
... ... @@ -66,27 +66,8 @@
66 66  
67 67 The current snapshot for which the device is mapped.
68 68  
69   -snap_*
70   -
71   - A directory per each snapshot
72   -
73 69 parent
74 70  
75 71 Information identifying the pool, image, and snapshot id for
76 72 the parent image in a layered rbd image (format 2 only).
77   -
78   -Entries under /sys/bus/rbd/devices/<dev-id>/snap_<snap-name>
79   --------------------------------------------------------------
80   -
81   -snap_id
82   -
83   - The rados internal snapshot id assigned for this snapshot
84   -
85   -snap_size
86   -
87   - The size of the image when this snapshot was taken.
88   -
89   -snap_features
90   -
91   - A hexadecimal encoding of the feature bits for this snapshot.
... ... @@ -272,7 +272,6 @@
272 272 list_for_each_entry_safe_reverse(oreq, n, &(ireq)->obj_requests, links)
273 273  
274 274 struct rbd_snap {
275   - struct device dev;
276 275 const char *name;
277 276 u64 size;
278 277 struct list_head node;
... ... @@ -358,7 +357,6 @@
358 357 static int rbd_img_request_submit(struct rbd_img_request *img_request);
359 358  
360 359 static int rbd_dev_snaps_update(struct rbd_device *rbd_dev);
361   -static int rbd_dev_snaps_register(struct rbd_device *rbd_dev);
362 360  
363 361 static void rbd_dev_release(struct device *dev);
364 362 static void rbd_remove_snap_dev(struct rbd_snap *snap);
... ... @@ -3069,8 +3067,6 @@
3069 3067 kfree(h.object_prefix);
3070 3068  
3071 3069 ret = rbd_dev_snaps_update(rbd_dev);
3072   - if (!ret)
3073   - ret = rbd_dev_snaps_register(rbd_dev);
3074 3070  
3075 3071 up_write(&rbd_dev->header_rwsem);
3076 3072  
... ... @@ -3344,71 +3340,6 @@
3344 3340 .release = rbd_sysfs_dev_release,
3345 3341 };
3346 3342  
3347   -
3348   -/*
3349   - sysfs - snapshots
3350   -*/
3351   -
3352   -static ssize_t rbd_snap_size_show(struct device *dev,
3353   - struct device_attribute *attr,
3354   - char *buf)
3355   -{
3356   - struct rbd_snap *snap = container_of(dev, struct rbd_snap, dev);
3357   -
3358   - return sprintf(buf, "%llu\n", (unsigned long long)snap->size);
3359   -}
3360   -
3361   -static ssize_t rbd_snap_id_show(struct device *dev,
3362   - struct device_attribute *attr,
3363   - char *buf)
3364   -{
3365   - struct rbd_snap *snap = container_of(dev, struct rbd_snap, dev);
3366   -
3367   - return sprintf(buf, "%llu\n", (unsigned long long)snap->id);
3368   -}
3369   -
3370   -static ssize_t rbd_snap_features_show(struct device *dev,
3371   - struct device_attribute *attr,
3372   - char *buf)
3373   -{
3374   - struct rbd_snap *snap = container_of(dev, struct rbd_snap, dev);
3375   -
3376   - return sprintf(buf, "0x%016llx\n",
3377   - (unsigned long long) snap->features);
3378   -}
3379   -
3380   -static DEVICE_ATTR(snap_size, S_IRUGO, rbd_snap_size_show, NULL);
3381   -static DEVICE_ATTR(snap_id, S_IRUGO, rbd_snap_id_show, NULL);
3382   -static DEVICE_ATTR(snap_features, S_IRUGO, rbd_snap_features_show, NULL);
3383   -
3384   -static struct attribute *rbd_snap_attrs[] = {
3385   - &dev_attr_snap_size.attr,
3386   - &dev_attr_snap_id.attr,
3387   - &dev_attr_snap_features.attr,
3388   - NULL,
3389   -};
3390   -
3391   -static struct attribute_group rbd_snap_attr_group = {
3392   - .attrs = rbd_snap_attrs,
3393   -};
3394   -
3395   -static void rbd_snap_dev_release(struct device *dev)
3396   -{
3397   - struct rbd_snap *snap = container_of(dev, struct rbd_snap, dev);
3398   - kfree(snap->name);
3399   - kfree(snap);
3400   -}
3401   -
3402   -static const struct attribute_group *rbd_snap_attr_groups[] = {
3403   - &rbd_snap_attr_group,
3404   - NULL
3405   -};
3406   -
3407   -static struct device_type rbd_snap_device_type = {
3408   - .groups = rbd_snap_attr_groups,
3409   - .release = rbd_snap_dev_release,
3410   -};
3411   -
3412 3343 static struct rbd_spec *rbd_spec_get(struct rbd_spec *spec)
3413 3344 {
3414 3345 kref_get(&spec->kref);
3415 3346  
3416 3347  
... ... @@ -3483,40 +3414,13 @@
3483 3414 kfree(rbd_dev);
3484 3415 }
3485 3416  
3486   -static bool rbd_snap_registered(struct rbd_snap *snap)
3487   -{
3488   - bool ret = snap->dev.type == &rbd_snap_device_type;
3489   - bool reg = device_is_registered(&snap->dev);
3490   -
3491   - rbd_assert(!ret ^ reg);
3492   -
3493   - return ret;
3494   -}
3495   -
3496 3417 static void rbd_remove_snap_dev(struct rbd_snap *snap)
3497 3418 {
3498 3419 list_del(&snap->node);
3499   - if (device_is_registered(&snap->dev))
3500   - device_unregister(&snap->dev);
  3420 + kfree(snap->name);
  3421 + kfree(snap);
3501 3422 }
3502 3423  
3503   -static int rbd_register_snap_dev(struct rbd_snap *snap,
3504   - struct device *parent)
3505   -{
3506   - struct device *dev = &snap->dev;
3507   - int ret;
3508   -
3509   - dev->type = &rbd_snap_device_type;
3510   - dev->parent = parent;
3511   - dev->release = rbd_snap_dev_release;
3512   - dev_set_name(dev, "%s%s", RBD_SNAP_DEV_NAME_PREFIX, snap->name);
3513   - dout("%s: registering device for snapshot %s\n", __func__, snap->name);
3514   -
3515   - ret = device_register(dev);
3516   -
3517   - return ret;
3518   -}
3519   -
3520 3424 static struct rbd_snap *__rbd_add_snap_dev(struct rbd_device *rbd_dev,
3521 3425 const char *snap_name,
3522 3426 u64 snap_id, u64 snap_size,
... ... @@ -4089,8 +3993,6 @@
4089 3993 dout("rbd_dev_snaps_update returned %d\n", ret);
4090 3994 if (ret)
4091 3995 goto out;
4092   - ret = rbd_dev_snaps_register(rbd_dev);
4093   - dout("rbd_dev_snaps_register returned %d\n", ret);
4094 3996 out:
4095 3997 up_write(&rbd_dev->header_rwsem);
4096 3998  
4097 3999  
... ... @@ -4145,11 +4047,11 @@
4145 4047 */
4146 4048 if (rbd_dev->spec->snap_id == snap->id)
4147 4049 clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
4148   - rbd_remove_snap_dev(snap);
4149   - dout("%ssnap id %llu has been removed\n",
  4050 + dout("removing %ssnap id %llu\n",
4150 4051 rbd_dev->spec->snap_id == snap->id ?
4151 4052 "mapped " : "",
4152 4053 (unsigned long long) snap->id);
  4054 + rbd_remove_snap_dev(snap);
4153 4055  
4154 4056 /* Done with this list entry; advance */
4155 4057  
... ... @@ -4209,31 +4111,6 @@
4209 4111 return 0;
4210 4112 }
4211 4113  
4212   -/*
4213   - * Scan the list of snapshots and register the devices for any that
4214   - * have not already been registered.
4215   - */
4216   -static int rbd_dev_snaps_register(struct rbd_device *rbd_dev)
4217   -{
4218   - struct rbd_snap *snap;
4219   - int ret = 0;
4220   -
4221   - dout("%s:\n", __func__);
4222   - if (WARN_ON(!device_is_registered(&rbd_dev->dev)))
4223   - return -EIO;
4224   -
4225   - list_for_each_entry(snap, &rbd_dev->snaps, node) {
4226   - if (!rbd_snap_registered(snap)) {
4227   - ret = rbd_register_snap_dev(snap, &rbd_dev->dev);
4228   - if (ret < 0)
4229   - break;
4230   - }
4231   - }
4232   - dout("%s: returning %d\n", __func__, ret);
4233   -
4234   - return ret;
4235   -}
4236   -
4237 4114 static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
4238 4115 {
4239 4116 struct device *dev;
... ... @@ -4839,12 +4716,6 @@
4839 4716 goto err_out_parent;
4840 4717 rbd_dev->parent = parent;
4841 4718 }
4842   -
4843   - down_write(&rbd_dev->header_rwsem);
4844   - ret = rbd_dev_snaps_register(rbd_dev);
4845   - up_write(&rbd_dev->header_rwsem);
4846   - if (ret)
4847   - goto err_out_bus;
4848 4719  
4849 4720 ret = rbd_dev_header_watch_sync(rbd_dev, 1);
4850 4721 if (ret)