rbd: split up rbd_get_segment()

There are two places where rbd_get_segment() is called.  One, in
rbd_rq_fn(), only needs to know the length within a segment that an
I/O request should be.  The other, in rbd_do_op(), also needs the
name of the object and the offset within it for the I/O request.

Split out rbd_segment_name() into three dedicated functions:
    - rbd_segment_name() allocates and formats the name of the
      object for a segment containing a given rbd image offset
    - rbd_segment_offset() computes the offset within a segment for
      a given rbd image offset
    - rbd_segment_length() computes the length to use for I/O within
      a segment for a request, not to exceed the end of a segment
      object.

In the new functions be a bit more careful, checking for possible
error conditions:
    - watch for errors or overflows returned by snprintf()
    - catch (using BUG_ON()) potential overflow conditions
      when computing segment length

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Yehuda Sadeh <yehuda@inktank.com>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 6da6990a7b57148b156edd7b68deb3a9bcbfb512..7ba70c49bbdbf1321b8e2372db1426aebbf216bf 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -669,27 +669,47 @@ static void rbd_header_free(struct rbd_image_header *header)
        header->snapc = NULL;
 }
 
-/*
- * get the actual striped segment name, offset and length
- */
-static u64 rbd_get_segment(struct rbd_image_header *header,
-                          const char *object_prefix,
-                          u64 ofs, u64 len,
-                          char *seg_name, u64 *segofs)
+static char *rbd_segment_name(struct rbd_device *rbd_dev, u64 offset)
+{
+       char *name;
+       u64 segment;
+       int ret;
+
+       name = kmalloc(RBD_MAX_SEG_NAME_LEN + 1, GFP_NOIO);
+       if (!name)
+               return NULL;
+       segment = offset >> rbd_dev->header.obj_order;
+       ret = snprintf(name, RBD_MAX_SEG_NAME_LEN, "%s.%012llx",
+                       rbd_dev->header.object_prefix, segment);
+       if (ret < 0 || ret >= RBD_MAX_SEG_NAME_LEN) {
+               pr_err("error formatting segment name for #%llu (%d)\n",
+                       segment, ret);
+               kfree(name);
+               name = NULL;
+       }
+
+       return name;
+}
+
+static u64 rbd_segment_offset(struct rbd_device *rbd_dev, u64 offset)
 {
-       u64 seg = ofs >> header->obj_order;
+       u64 segment_size = (u64) 1 << rbd_dev->header.obj_order;
 
-       if (seg_name)
-               snprintf(seg_name, RBD_MAX_SEG_NAME_LEN,
-                        "%s.%012llx", object_prefix, seg);
+       return offset & (segment_size - 1);
+}
+
+static u64 rbd_segment_length(struct rbd_device *rbd_dev,
+                               u64 offset, u64 length)
+{
+       u64 segment_size = (u64) 1 << rbd_dev->header.obj_order;
 
-       ofs = ofs & ((1 << header->obj_order) - 1);
-       len = min_t(u64, len, (1 << header->obj_order) - ofs);
+       offset &= segment_size - 1;
 
-       if (segofs)
-               *segofs = ofs;
+       BUG_ON(length > U64_MAX - offset);
+       if (offset + length > segment_size)
+               length = segment_size - offset;
 
-       return len;
+       return length;
 }
 
 static int rbd_get_num_segments(struct rbd_image_header *header,
@@ -1127,14 +1147,11 @@ static int rbd_do_op(struct request *rq,
        struct ceph_osd_req_op *ops;
        u32 payload_len;
 
-       seg_name = kmalloc(RBD_MAX_SEG_NAME_LEN + 1, GFP_NOIO);
+       seg_name = rbd_segment_name(rbd_dev, ofs);
        if (!seg_name)
                return -ENOMEM;
-
-       seg_len = rbd_get_segment(&rbd_dev->header,
-                                 rbd_dev->header.object_prefix,
-                                 ofs, len,
-                                 seg_name, &seg_ofs);
+       seg_len = rbd_segment_length(rbd_dev, ofs, len);
+       seg_ofs = rbd_segment_offset(rbd_dev, ofs);
 
        payload_len = (flags & CEPH_OSD_FLAG_WRITE ? seg_len : 0);
 
@@ -1545,10 +1562,7 @@ static void rbd_rq_fn(struct request_queue *q)
                do {
                        /* a bio clone to be passed down to OSD req */
                        dout("rq->bio->bi_vcnt=%hu\n", rq->bio->bi_vcnt);
-                       op_size = rbd_get_segment(&rbd_dev->header,
-                                                 rbd_dev->header.object_prefix,
-                                                 ofs, size,
-                                                 NULL, NULL);
+                       op_size = rbd_segment_length(rbd_dev, ofs, size);
                        kref_get(&coll->kref);
                        bio = bio_chain_clone(&rq_bio, &next_bio, &bp,
                                              op_size, GFP_ATOMIC);