From: Josh Durgin <josh.durgin@inktank.com>
To: Alex Elder <elder@inktank.com>
Cc: "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>
Subject: Re: [PATCH 07/20] rbd: define rbd_osd_req_format_op()
Date: Fri, 05 Apr 2013 11:18:03 -0700 [thread overview]
Message-ID: <515F155B.5040502@inktank.com> (raw)
In-Reply-To: <515ED9A1.7070809@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 04/05/2013 07:03 AM, Alex Elder wrote:
> Define rbd_osd_req_format_op(), which encapsulates formatting
> an osd op into an object request's osd request message. Only
> one op is supported right now.
>
> Stop calling ceph_osdc_build_request() in rbd_osd_req_create().
> Instead, call rbd_osd_req_format_op() in each of the callers of
> rbd_osd_req_create().
>
> This is to prepare for the next patch, in which the source ops for
> an osd request will be held in the osd request itself. Because of
> that, we won't have the source op to work with until after the
> request is created, so we can't format the op until then.
>
> This an the next patch resolve:
> http://tracker.ceph.com/issues/4656
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 98
> ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 53 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index a9d88a0..fc41675 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1311,29 +1311,47 @@ static void rbd_osd_req_callback(struct
> ceph_osd_request *osd_req,
> rbd_obj_request_complete(obj_request);
> }
>
> +static void rbd_osd_req_format_op(struct rbd_obj_request *obj_request,
> + bool write_request,
> + struct ceph_osd_req_op *op)
> +{
> + struct rbd_img_request *img_request = obj_request->img_request;
> + struct ceph_snap_context *snapc = NULL;
> + u64 snap_id = CEPH_NOSNAP;
> + struct timespec *mtime = NULL;
> + struct timespec now;
> +
> + rbd_assert(obj_request->osd_req != NULL);
> +
> + if (write_request) {
> + now = CURRENT_TIME;
> + mtime = &now;
> + if (img_request)
> + snapc = img_request->snapc;
> + } else if (img_request) {
> + snap_id = img_request->snap_id;
> + }
> +
> + ceph_osdc_build_request(obj_request->osd_req, obj_request->offset,
> + 1, op, snapc, snap_id, mtime);
> +}
> +
> static struct ceph_osd_request *rbd_osd_req_create(
> struct rbd_device *rbd_dev,
> bool write_request,
> - struct rbd_obj_request *obj_request,
> - struct ceph_osd_req_op *op)
> + struct rbd_obj_request *obj_request)
> {
> struct rbd_img_request *img_request = obj_request->img_request;
> struct ceph_snap_context *snapc = NULL;
> struct ceph_osd_client *osdc;
> struct ceph_osd_request *osd_req;
> struct ceph_osd_data *osd_data;
> - struct timespec now;
> - struct timespec *mtime;
> - u64 snap_id = CEPH_NOSNAP;
> u64 offset = obj_request->offset;
> - u64 length = obj_request->length;
>
> if (img_request) {
> rbd_assert(img_request->write_request == write_request);
> if (img_request->write_request)
> snapc = img_request->snapc;
> - else
> - snap_id = img_request->snap_id;
> }
>
> /* Allocate and initialize the request, for the single op */
> @@ -1360,16 +1378,10 @@ static struct ceph_osd_request *rbd_osd_req_create(
> break;
> }
>
> - if (write_request) {
> + if (write_request)
> osd_req->r_flags = CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK;
> - now = CURRENT_TIME;
> - mtime = &now;
> - } else {
> + else
> osd_req->r_flags = CEPH_OSD_FLAG_READ;
> - mtime = NULL; /* not needed for reads */
> - offset = 0; /* These are not used... */
> - length = 0; /* ...for osd read requests */
> - }
>
> osd_req->r_callback = rbd_osd_req_callback;
> osd_req->r_priv = obj_request;
> @@ -1380,11 +1392,6 @@ static struct ceph_osd_request *rbd_osd_req_create(
>
> osd_req->r_file_layout = rbd_dev->layout; /* struct */
>
> - /* osd_req will get its own reference to snapc (if non-null) */
> -
> - ceph_osdc_build_request(osd_req, offset, 1, op,
> - snapc, snap_id, mtime);
> -
> return osd_req;
> }
>
> @@ -1538,6 +1545,7 @@ static int rbd_img_request_fill_bio(struct
> rbd_img_request *img_request,
> struct rbd_device *rbd_dev = img_request->rbd_dev;
> struct rbd_obj_request *obj_request = NULL;
> struct rbd_obj_request *next_obj_request;
> + bool write_request = img_request->write_request;
> unsigned int bio_offset;
> u64 image_offset;
> u64 resid;
> @@ -1545,8 +1553,7 @@ static int rbd_img_request_fill_bio(struct
> rbd_img_request *img_request,
>
> dout("%s: img %p bio %p\n", __func__, img_request, bio_list);
>
> - opcode = img_request->write_request ? CEPH_OSD_OP_WRITE
> - : CEPH_OSD_OP_READ;
> + opcode = write_request ? CEPH_OSD_OP_WRITE : CEPH_OSD_OP_READ;
> bio_offset = 0;
> image_offset = img_request->offset;
> rbd_assert(image_offset == bio_list->bi_sector << SECTOR_SHIFT);
> @@ -1579,17 +1586,14 @@ static int rbd_img_request_fill_bio(struct
> rbd_img_request *img_request,
> if (!obj_request->bio_list)
> goto out_partial;
>
> - /*
> - * Build up the op to use in building the osd
> - * request. Note that the contents of the op are
> - * copied by rbd_osd_req_create().
> - */
> - osd_req_op_extent_init(&op, opcode, offset, length, 0, 0);
> obj_request->osd_req = rbd_osd_req_create(rbd_dev,
> - img_request->write_request,
> - obj_request, &op);
> + write_request, obj_request);
> if (!obj_request->osd_req)
> goto out_partial;
> +
> + osd_req_op_extent_init(&op, opcode, offset, length, 0, 0);
> + rbd_osd_req_format_op(obj_request, write_request, &op);
> +
> /* status and version are initially zero-filled */
>
> rbd_img_obj_request_add(img_request, obj_request);
> @@ -1699,12 +1703,13 @@ static int rbd_obj_notify_ack(struct rbd_device
> *rbd_dev,
> return -ENOMEM;
>
> ret = -ENOMEM;
> - osd_req_op_watch_init(&op, CEPH_OSD_OP_NOTIFY_ACK, notify_id, ver, 0);
> - obj_request->osd_req = rbd_osd_req_create(rbd_dev, false,
> - obj_request, &op);
> + obj_request->osd_req = rbd_osd_req_create(rbd_dev, false, obj_request);
> if (!obj_request->osd_req)
> goto out;
>
> + osd_req_op_watch_init(&op, CEPH_OSD_OP_NOTIFY_ACK, notify_id, ver, 0);
> + rbd_osd_req_format_op(obj_request, false, &op);
> +
> osdc = &rbd_dev->rbd_client->client->osdc;
> obj_request->callback = rbd_obj_request_put;
> ret = rbd_obj_request_submit(osdc, obj_request);
> @@ -1763,13 +1768,14 @@ static int rbd_dev_header_watch_sync(struct
> rbd_device *rbd_dev, int start)
> if (!obj_request)
> goto out_cancel;
>
> + obj_request->osd_req = rbd_osd_req_create(rbd_dev, true, obj_request);
> + if (!obj_request->osd_req)
> + goto out_cancel;
> +
> osd_req_op_watch_init(&op, CEPH_OSD_OP_WATCH,
> rbd_dev->watch_event->cookie,
> rbd_dev->header.obj_version, start);
> - obj_request->osd_req = rbd_osd_req_create(rbd_dev, true,
> - obj_request, &op);
> - if (!obj_request->osd_req)
> - goto out_cancel;
> + rbd_osd_req_format_op(obj_request, true, &op);
>
> if (start)
> ceph_osdc_set_request_linger(osdc, obj_request->osd_req);
> @@ -1855,13 +1861,14 @@ static int rbd_obj_method_sync(struct rbd_device
> *rbd_dev,
> obj_request->pages = pages;
> obj_request->page_count = page_count;
>
> - osd_req_op_cls_init(&op, CEPH_OSD_OP_CALL, class_name, method_name,
> - outbound, outbound_size);
> - obj_request->osd_req = rbd_osd_req_create(rbd_dev, false,
> - obj_request, &op);
> + obj_request->osd_req = rbd_osd_req_create(rbd_dev, false, obj_request);
> if (!obj_request->osd_req)
> goto out;
>
> + osd_req_op_cls_init(&op, CEPH_OSD_OP_CALL, class_name, method_name,
> + outbound, outbound_size);
> + rbd_osd_req_format_op(obj_request, false, &op);
> +
> osdc = &rbd_dev->rbd_client->client->osdc;
> ret = rbd_obj_request_submit(osdc, obj_request);
> if (ret)
> @@ -2060,12 +2067,13 @@ static int rbd_obj_read_sync(struct rbd_device
> *rbd_dev,
> obj_request->pages = pages;
> obj_request->page_count = page_count;
>
> - osd_req_op_extent_init(&op, CEPH_OSD_OP_READ, offset, length, 0, 0);
> - obj_request->osd_req = rbd_osd_req_create(rbd_dev, false,
> - obj_request, &op);
> + obj_request->osd_req = rbd_osd_req_create(rbd_dev, false, obj_request);
> if (!obj_request->osd_req)
> goto out;
>
> + osd_req_op_extent_init(&op, CEPH_OSD_OP_READ, offset, length, 0, 0);
> + rbd_osd_req_format_op(obj_request, false, &op);
> +
> osdc = &rbd_dev->rbd_client->client->osdc;
> ret = rbd_obj_request_submit(osdc, obj_request);
> if (ret)
>
next prev parent reply other threads:[~2013-04-05 18:18 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-05 13:57 [PATCH 00/20] libceph: per-op osd request data Alex Elder
2013-04-05 14:01 ` [PATCH 01/20] rbd: define inbound data size for method ops Alex Elder
2013-04-05 18:13 ` Josh Durgin
2013-04-05 14:01 ` [PATCH 02/20] libceph: compute incoming bytes once Alex Elder
2013-04-05 18:14 ` Josh Durgin
2013-04-05 14:02 ` [PATCH 03/20] libceph: define osd data initialization helpers Alex Elder
2013-04-05 18:16 ` Josh Durgin
2013-04-05 14:02 ` [PATCH 04/20] libceph: define a few more helpers Alex Elder
2013-04-05 18:17 ` Josh Durgin
2013-04-05 14:02 ` [PATCH 05/20] libceph: define ceph_osd_data_length() Alex Elder
2013-04-05 18:17 ` Josh Durgin
2013-04-05 14:02 ` [PATCH 06/20] libceph: a few more osd data cleanups Alex Elder
2013-04-05 18:17 ` Josh Durgin
2013-04-05 14:03 ` [PATCH 07/20] rbd: define rbd_osd_req_format_op() Alex Elder
2013-04-05 18:18 ` Josh Durgin [this message]
2013-04-05 14:03 ` [PATCH 08/20] libceph: keep source rather than message osd op array Alex Elder
2013-04-08 18:12 ` Josh Durgin
2013-04-05 14:03 ` [PATCH 09/20] libceph: rename data out field in osd request op Alex Elder
2013-04-08 18:12 ` Josh Durgin
2013-04-05 14:03 ` [PATCH 10/20] libceph: add data pointers in osd op structures Alex Elder
2013-04-08 18:13 ` Josh Durgin
2013-04-05 14:04 ` [PATCH 11/20] libceph: specify osd op by index in request Alex Elder
2013-04-08 18:14 ` Josh Durgin
2013-04-05 14:04 ` [PATCH 12/20] rbd: don't set data in rbd_osd_req_format_op() Alex Elder
2013-04-08 18:14 ` Josh Durgin
2013-04-05 14:04 ` [PATCH 13/20] rbd: separate initialization of osd data Alex Elder
2013-04-08 18:14 ` Josh Durgin
2013-04-05 14:04 ` [PATCH 14/20] rbd: rearrange some code for consistency Alex Elder
2013-04-08 18:15 ` Josh Durgin
2013-04-05 14:05 ` [PATCH 15/20] libceph: format class info at init time Alex Elder
2013-04-08 18:16 ` Josh Durgin
2013-04-05 14:05 ` [PATCH 16/20] libceph: move ceph_osdc_build_request() Alex Elder
2013-04-08 18:17 ` Josh Durgin
2013-04-05 14:05 ` [PATCH 17/20] libceph: set message data when building osd request Alex Elder
2013-04-08 18:17 ` Josh Durgin
2013-04-05 14:05 ` [PATCH 18/20] libceph: combine initializing and setting osd data Alex Elder
2013-04-08 19:59 ` Josh Durgin
2013-04-05 14:06 ` [PATCH 19/20] libceph: set the data pointers when encoding ops Alex Elder
2013-04-08 20:03 ` Josh Durgin
2013-04-05 14:06 ` [PATCH 20/20] libceph: kill off osd request r_data_in and r_data_out Alex Elder
2013-04-08 20:07 ` Josh Durgin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=515F155B.5040502@inktank.com \
--to=josh.durgin@inktank.com \
--cc=ceph-devel@vger.kernel.org \
--cc=elder@inktank.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.