Skip to content

Commit

Permalink
libceph: let osd ops determine request data length
Browse files Browse the repository at this point in the history
The length of outgoing data in an osd request is dependent on the
osd ops that are embedded in that request.  Each op is encoded into
a request message using osd_req_encode_op(), so that should be used
to determine the amount of outgoing data implied by the op as it
is encoded.

Have osd_req_encode_op() return the number of bytes of outgoing data
implied by the op being encoded, and accumulate and use that in
ceph_osdc_build_request().

As a result, ceph_osdc_build_request() no longer requires its "len"
parameter, so get rid of it.

Using the sum of the op lengths rather than the length provided is
a valid change because:
    - The only callers of osd ceph_osdc_build_request() are
      rbd and the osd client (in ceph_osdc_new_request() on
      behalf of the file system).
    - When rbd calls it, the length provided is only non-zero for
      write requests, and in that case the single op has the
      same length value as what was passed here.
    - When called from ceph_osdc_new_request(), (it's not all that
      easy to see, but) the length passed is also always the same
      as the extent length encoded in its (single) write op if
      present.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
  • Loading branch information
Alex Elder authored and Sage Weil committed May 2, 2013
1 parent e766d7b commit 175face
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 17 deletions.
2 changes: 1 addition & 1 deletion drivers/block/rbd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ static struct ceph_osd_request *rbd_osd_req_create(

/* osd_req will get its own reference to snapc (if non-null) */

ceph_osdc_build_request(osd_req, offset, length, 1, op,
ceph_osdc_build_request(osd_req, offset, 1, op,
snapc, snap_id, mtime);

return osd_req;
Expand Down
3 changes: 1 addition & 2 deletions include/linux/ceph/osd_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,7 @@ extern struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *
bool use_mempool,
gfp_t gfp_flags);

extern void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len,
extern void ceph_osdc_build_request(struct ceph_osd_request *req, u64 off,
unsigned int num_op,
struct ceph_osd_req_op *src_ops,
struct ceph_snap_context *snapc,
Expand Down
33 changes: 19 additions & 14 deletions net/ceph/osd_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,21 +222,24 @@ struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
}
EXPORT_SYMBOL(ceph_osdc_alloc_request);

static void osd_req_encode_op(struct ceph_osd_request *req,
static u64 osd_req_encode_op(struct ceph_osd_request *req,
struct ceph_osd_op *dst,
struct ceph_osd_req_op *src)
{
u64 out_data_len = 0;
u64 tmp;

dst->op = cpu_to_le16(src->op);

switch (src->op) {
case CEPH_OSD_OP_STAT:
break;
case CEPH_OSD_OP_READ:
case CEPH_OSD_OP_WRITE:
dst->extent.offset =
cpu_to_le64(src->extent.offset);
dst->extent.length =
cpu_to_le64(src->extent.length);
if (src->op == CEPH_OSD_OP_WRITE)
out_data_len = src->extent.length;
dst->extent.offset = cpu_to_le64(src->extent.offset);
dst->extent.length = cpu_to_le64(src->extent.length);
dst->extent.truncate_size =
cpu_to_le64(src->extent.truncate_size);
dst->extent.truncate_seq =
Expand All @@ -247,12 +250,14 @@ static void osd_req_encode_op(struct ceph_osd_request *req,
dst->cls.method_len = src->cls.method_len;
dst->cls.indata_len = cpu_to_le32(src->cls.indata_len);

tmp = req->r_trail.length;
ceph_pagelist_append(&req->r_trail, src->cls.class_name,
src->cls.class_len);
ceph_pagelist_append(&req->r_trail, src->cls.method_name,
src->cls.method_len);
ceph_pagelist_append(&req->r_trail, src->cls.indata,
src->cls.indata_len);
out_data_len = req->r_trail.length - tmp;
break;
case CEPH_OSD_OP_STARTSYNC:
break;
Expand Down Expand Up @@ -326,14 +331,16 @@ static void osd_req_encode_op(struct ceph_osd_request *req,
break;
}
dst->payload_len = cpu_to_le32(src->payload_len);

return out_data_len;
}

/*
* build new request AND message
*
*/
void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len, unsigned int num_ops,
u64 off, unsigned int num_ops,
struct ceph_osd_req_op *src_ops,
struct ceph_snap_context *snapc, u64 snap_id,
struct timespec *mtime)
Expand Down Expand Up @@ -385,12 +392,13 @@ void ceph_osdc_build_request(struct ceph_osd_request *req,
dout("oid '%.*s' len %d\n", req->r_oid_len, req->r_oid, req->r_oid_len);
p += req->r_oid_len;

/* ops */
/* ops--can imply data */
ceph_encode_16(&p, num_ops);
src_op = src_ops;
req->r_request_ops = p;
data_len = 0;
for (i = 0; i < num_ops; i++, src_op++) {
osd_req_encode_op(req, p, src_op);
data_len += osd_req_encode_op(req, p, src_op);
p += sizeof(struct ceph_osd_op);
}

Expand All @@ -407,11 +415,9 @@ void ceph_osdc_build_request(struct ceph_osd_request *req,
req->r_request_attempts = p;
p += 4;

data_len = req->r_trail.length;
if (flags & CEPH_OSD_FLAG_WRITE) {
/* data */
if (flags & CEPH_OSD_FLAG_WRITE)
req->r_request->hdr.data_off = cpu_to_le16(off);
data_len += len;
}
req->r_request->hdr.data_len = cpu_to_le32(data_len);

BUG_ON(p > msg->front.iov_base + msg->front.iov_len);
Expand Down Expand Up @@ -477,13 +483,12 @@ struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *osdc,
ceph_osdc_put_request(req);
return ERR_PTR(r);
}

req->r_file_layout = *layout; /* keep a copy */

snprintf(req->r_oid, sizeof(req->r_oid), "%llx.%08llx", vino.ino, bno);
req->r_oid_len = strlen(req->r_oid);

ceph_osdc_build_request(req, off, *plen, num_op, ops,
ceph_osdc_build_request(req, off, num_op, ops,
snapc, vino.snap, mtime);

return req;
Expand Down

0 comments on commit 175face

Please sign in to comment.