rbd: fix rbd_dev_parent_get() when parent_overlap == 0

The comment for rbd_dev_parent_get() said

    * We must get the reference before checking for the overlap to
    * coordinate properly with zeroing the parent overlap in
    * rbd_dev_v2_parent_info() when an image gets flattened.  We
    * drop it again if there is no overlap.

but the "drop it again if there is no overlap" part was missing from
the implementation.  This lead to absurd parent_ref values for images
with parent_overlap == 0, as parent_ref was incremented for each
img_request and virtually never decremented.

Fix this by leveraging the fact that refresh path calls
rbd_dev_v2_parent_info() under header_rwsem and use it for read in
rbd_dev_parent_get(), instead of messing around with atomics.  Get rid
of barriers in rbd_dev_v2_parent_info() while at it - I don't see what
they'd pair with now and I suspect we are in a pretty miserable
situation as far as proper locking goes regardless.

Cc: stable@vger.kernel.org # 3.11+
Signed-off-by: Ilya Dryomov <idryomov@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Reviewed-by: Alex Elder <elder@linaro.org>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 3ec85dfce12496dd64a8ad2c37984ad61f489e25..d511ecb35144deed195d8db2a7822ffee5098f1f 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2098,32 +2098,26 @@ static void rbd_dev_parent_put(struct rbd_device *rbd_dev)
  * If an image has a non-zero parent overlap, get a reference to its
  * parent.
  *
- * We must get the reference before checking for the overlap to
- * coordinate properly with zeroing the parent overlap in
- * rbd_dev_v2_parent_info() when an image gets flattened.  We
- * drop it again if there is no overlap.
- *
  * Returns true if the rbd device has a parent with a non-zero
  * overlap and a reference for it was successfully taken, or
  * false otherwise.
  */
 static bool rbd_dev_parent_get(struct rbd_device *rbd_dev)
 {
-       int counter;
+       int counter = 0;
 
        if (!rbd_dev->parent_spec)
                return false;
 
-       counter = atomic_inc_return_safe(&rbd_dev->parent_ref);
-       if (counter > 0 && rbd_dev->parent_overlap)
-               return true;
-
-       /* Image was flattened, but parent is not yet torn down */
+       down_read(&rbd_dev->header_rwsem);
+       if (rbd_dev->parent_overlap)
+               counter = atomic_inc_return_safe(&rbd_dev->parent_ref);
+       up_read(&rbd_dev->header_rwsem);
 
        if (counter < 0)
                rbd_warn(rbd_dev, "parent reference overflow");
 
-       return false;
+       return counter > 0;
 }
 
 /*
@@ -4239,7 +4233,6 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
                 */
                if (rbd_dev->parent_overlap) {
                        rbd_dev->parent_overlap = 0;
-                       smp_mb();
                        rbd_dev_parent_put(rbd_dev);
                        pr_info("%s: clone image has been flattened\n",
                                rbd_dev->disk->disk_name);
@@ -4285,7 +4278,6 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
         * treat it specially.
         */
        rbd_dev->parent_overlap = overlap;
-       smp_mb();
        if (!overlap) {
 
                /* A null parent_spec indicates it's the initial probe */