rbd: acquire header_rwsem just once in rbd_queue_workfn()

Currently header_rwsem is acquired twice: once in rbd_dev_parent_get()
when the image request is being created and then in rbd_queue_workfn()
to capture mapping_size and snapc.  Introduce rbd_img_capture_header()
and move image request allocation so that header_rwsem can be acquired
just once.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index c61c5dd..acda61f 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1601,10 +1601,8 @@ static bool rbd_dev_parent_get(struct rbd_device *rbd_dev)
        if (!rbd_dev->parent_spec)
                return false;
 
        if (!rbd_dev->parent_spec)
                return false;
 

-       down_read(&rbd_dev->header_rwsem);

        if (rbd_dev->parent_overlap)
                counter = atomic_inc_return_safe(&rbd_dev->parent_ref);
        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");
 
        if (counter < 0)
                rbd_warn(rbd_dev, "parent reference overflow");


@@ -1619,8 +1617,7 @@ static bool rbd_dev_parent_get(struct rbd_device *rbd_dev)
  */
 static struct rbd_img_request *rbd_img_request_create(
                                        struct rbd_device *rbd_dev,
  */
 static struct rbd_img_request *rbd_img_request_create(
                                        struct rbd_device *rbd_dev,

-                                       enum obj_operation_type op_type,
-                                       struct ceph_snap_context *snapc)
+                                       enum obj_operation_type op_type)

 {
        struct rbd_img_request *img_request;
 
 {
        struct rbd_img_request *img_request;
 


@@ -1630,13 +1627,6 @@ static struct rbd_img_request *rbd_img_request_create(
 
        img_request->rbd_dev = rbd_dev;
        img_request->op_type = op_type;
 
        img_request->rbd_dev = rbd_dev;
        img_request->op_type = op_type;

-       if (!rbd_img_is_write(img_request))
-               img_request->snap_id = rbd_dev->spec->snap_id;
-       else
-               img_request->snapc = snapc;
-
-       if (rbd_dev_parent_get(rbd_dev))
-               img_request_layered_set(img_request);

 
        INIT_LIST_HEAD(&img_request->lock_item);
        INIT_LIST_HEAD(&img_request->object_extents);
 
        INIT_LIST_HEAD(&img_request->lock_item);
        INIT_LIST_HEAD(&img_request->object_extents);


@@ -1645,6 +1635,21 @@ static struct rbd_img_request *rbd_img_request_create(
        return img_request;
 }
 
        return img_request;
 }
 

+static void rbd_img_capture_header(struct rbd_img_request *img_req)
+{
+       struct rbd_device *rbd_dev = img_req->rbd_dev;
+
+       lockdep_assert_held(&rbd_dev->header_rwsem);
+
+       if (rbd_img_is_write(img_req))
+               img_req->snapc = ceph_get_snap_context(rbd_dev->header.snapc);
+       else
+               img_req->snap_id = rbd_dev->spec->snap_id;
+
+       if (rbd_dev_parent_get(rbd_dev))
+               img_request_layered_set(img_req);
+}
+

 static void rbd_img_request_destroy(struct rbd_img_request *img_request)
 {
        struct rbd_obj_request *obj_request;
 static void rbd_img_request_destroy(struct rbd_img_request *img_request)
 {
        struct rbd_obj_request *obj_request;


@@ -2825,17 +2830,21 @@ static int rbd_obj_read_object(struct rbd_obj_request *obj_req)
 static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req)
 {
        struct rbd_img_request *img_req = obj_req->img_request;
 static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req)
 {
        struct rbd_img_request *img_req = obj_req->img_request;

+       struct rbd_device *parent = img_req->rbd_dev->parent;

        struct rbd_img_request *child_img_req;
        int ret;
 
        struct rbd_img_request *child_img_req;
        int ret;
 

-       child_img_req = rbd_img_request_create(img_req->rbd_dev->parent,
-                                              OBJ_OP_READ, NULL);
+       child_img_req = rbd_img_request_create(parent, OBJ_OP_READ);

        if (!child_img_req)
                return -ENOMEM;
 
        __set_bit(IMG_REQ_CHILD, &child_img_req->flags);
        child_img_req->obj_request = obj_req;
 
        if (!child_img_req)
                return -ENOMEM;
 
        __set_bit(IMG_REQ_CHILD, &child_img_req->flags);
        child_img_req->obj_request = obj_req;
 

+       down_read(&parent->header_rwsem);
+       rbd_img_capture_header(child_img_req);
+       up_read(&parent->header_rwsem);
+

        dout("%s child_img_req %p for obj_req %p\n", __func__, child_img_req,
             obj_req);
 
        dout("%s child_img_req %p for obj_req %p\n", __func__, child_img_req,
             obj_req);
 


@@ -4686,7 +4695,6 @@ static void rbd_queue_workfn(struct work_struct *work)
        struct request *rq = blk_mq_rq_from_pdu(work);
        struct rbd_device *rbd_dev = rq->q->queuedata;
        struct rbd_img_request *img_request;
        struct request *rq = blk_mq_rq_from_pdu(work);
        struct rbd_device *rbd_dev = rq->q->queuedata;
        struct rbd_img_request *img_request;

-       struct ceph_snap_context *snapc = NULL;

        u64 offset = (u64)blk_rq_pos(rq) << SECTOR_SHIFT;
        u64 length = blk_rq_bytes(rq);
        enum obj_operation_type op_type;
        u64 offset = (u64)blk_rq_pos(rq) << SECTOR_SHIFT;
        u64 length = blk_rq_bytes(rq);
        enum obj_operation_type op_type;


@@ -4739,28 +4747,24 @@ static void rbd_queue_workfn(struct work_struct *work)
 
        blk_mq_start_request(rq);
 
 
        blk_mq_start_request(rq);
 

+       img_request = rbd_img_request_create(rbd_dev, op_type);
+       if (!img_request) {
+               result = -ENOMEM;
+               goto err_rq;
+       }
+       img_request->rq = rq;
+

        down_read(&rbd_dev->header_rwsem);
        mapping_size = rbd_dev->mapping.size;
        down_read(&rbd_dev->header_rwsem);
        mapping_size = rbd_dev->mapping.size;

-       if (op_type != OBJ_OP_READ) {
-               snapc = rbd_dev->header.snapc;
-               ceph_get_snap_context(snapc);
-       }
+       rbd_img_capture_header(img_request);

        up_read(&rbd_dev->header_rwsem);
 
        if (offset + length > mapping_size) {
                rbd_warn(rbd_dev, "beyond EOD (%llu~%llu > %llu)", offset,
                         length, mapping_size);
                result = -EIO;
        up_read(&rbd_dev->header_rwsem);
 
        if (offset + length > mapping_size) {
                rbd_warn(rbd_dev, "beyond EOD (%llu~%llu > %llu)", offset,
                         length, mapping_size);
                result = -EIO;

-               goto err_rq;
-       }
-
-       img_request = rbd_img_request_create(rbd_dev, op_type, snapc);
-       if (!img_request) {
-               result = -ENOMEM;
-               goto err_rq;
+               goto err_img_request;

        }
        }

-       img_request->rq = rq;
-       snapc = NULL; /* img_request consumes a ref */

 
        dout("%s rbd_dev %p img_req %p %s %llu~%llu\n", __func__, rbd_dev,
             img_request, obj_op_name(op_type), offset, length);
 
        dout("%s rbd_dev %p img_req %p %s %llu~%llu\n", __func__, rbd_dev,
             img_request, obj_op_name(op_type), offset, length);


@@ -4782,7 +4786,6 @@ err_rq:
        if (result)
                rbd_warn(rbd_dev, "%s %llx at %llx result %d",
                         obj_op_name(op_type), length, offset, result);
        if (result)
                rbd_warn(rbd_dev, "%s %llx at %llx result %d",
                         obj_op_name(op_type), length, offset, result);

-       ceph_put_snap_context(snapc);

 err:
        blk_mq_end_request(rq, errno_to_blk_status(result));
 }
 err:
        blk_mq_end_request(rq, errno_to_blk_status(result));
 }