* [PATCH REPOST 1/4] rbd: pass num_op with ops array
2013-01-04 14:43 [PATCH REPOST 0/4] rbd: explicitly support only one osd op Alex Elder
@ 2013-01-04 14:45 ` Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 2/4] libceph: pass num_op with ops Alex Elder
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-01-04 14:45 UTC (permalink / raw)
To: ceph-devel@vger.kernel.org
Add a num_op parameter to rbd_do_request() and rbd_req_sync_op() to
indicate the number of entries in the array. The callers of these
functions always know how many entries are in the array, so just
pass that information down.
This is in anticipation of eliminating the extra zero-filled entry
in these ops arrays.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 6b9643a..cee1334 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1119,6 +1119,7 @@ static int rbd_do_request(struct request *rq,
struct page **pages,
int num_pages,
int flags,
+ unsigned int num_op,
struct ceph_osd_req_op *ops,
struct rbd_req_coll *coll,
int coll_index,
@@ -1259,6 +1260,7 @@ static void rbd_simple_req_cb(struct
ceph_osd_request *osd_req,
*/
static int rbd_req_sync_op(struct rbd_device *rbd_dev,
int flags,
+ unsigned int num_op,
struct ceph_osd_req_op *ops,
const char *object_name,
u64 ofs, u64 inbound_size,
@@ -1281,7 +1283,7 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
object_name, ofs, inbound_size, NULL,
pages, num_pages,
flags,
- ops,
+ num_op, ops,
NULL, 0,
NULL,
linger_req, ver);
@@ -1351,7 +1353,7 @@ static int rbd_do_op(struct request *rq,
bio,
NULL, 0,
flags,
- ops,
+ 1, ops,
coll, coll_index,
rbd_req_cb, 0, NULL);
if (ret < 0)
@@ -1380,7 +1382,7 @@ static int rbd_req_sync_read(struct rbd_device
*rbd_dev,
return -ENOMEM;
ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ,
- ops, object_name, ofs, len, buf, NULL, ver);
+ 1, ops, object_name, ofs, len, buf, NULL, ver);
rbd_destroy_ops(ops);
return ret;
@@ -1408,7 +1410,7 @@ static int rbd_req_sync_notify_ack(struct
rbd_device *rbd_dev,
rbd_dev->header_name, 0, 0, NULL,
NULL, 0,
CEPH_OSD_FLAG_READ,
- ops,
+ 1, ops,
NULL, 0,
rbd_simple_req_cb, 0, NULL);
@@ -1460,7 +1462,7 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev)
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- ops,
+ 1, ops,
rbd_dev->header_name,
0, 0, NULL,
&rbd_dev->watch_request, NULL);
@@ -1497,7 +1499,7 @@ static int rbd_req_sync_unwatch(struct rbd_device
*rbd_dev)
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- ops,
+ 1, ops,
rbd_dev->header_name,
0, 0, NULL, NULL, NULL);
@@ -1548,7 +1550,7 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
ops[0].cls.indata = outbound;
ops[0].cls.indata_len = outbound_size;
- ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, ops,
+ ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, 1, ops,
object_name, 0, inbound_size, inbound,
NULL, ver);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH REPOST 2/4] libceph: pass num_op with ops
2013-01-04 14:43 [PATCH REPOST 0/4] rbd: explicitly support only one osd op Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 1/4] rbd: pass num_op with ops array Alex Elder
@ 2013-01-04 14:45 ` Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 3/4] rbd: there is really only one op Alex Elder
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-01-04 14:45 UTC (permalink / raw)
To: ceph-devel@vger.kernel.org
Both ceph_osdc_alloc_request() and ceph_osdc_build_request() are
provided an array of ceph osd request operations. Rather than just
passing the number of operations in the array, the caller is
required append an additional zeroed operation structure to signal
the end of the array.
All callers know the number of operations at the time these
functions are called, so drop the silly zero entry and supply that
number directly. As a result, get_num_ops() is no longer needed.
This also means that ceph_osdc_alloc_request() never uses its ops
argument, so that can be dropped.
Also rbd_create_rw_ops() no longer needs to add one to reserve room
for the additional op.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 9 ++++----
include/linux/ceph/osd_client.h | 3 ++-
net/ceph/osd_client.c | 43
++++++++++++++-------------------------
3 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index cee1334..0e6cc67 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1026,12 +1026,12 @@ out_err:
/*
* helpers for osd request op vectors.
*/
-static struct ceph_osd_req_op *rbd_create_rw_ops(int num_ops,
+static struct ceph_osd_req_op *rbd_create_rw_ops(int num_op,
int opcode, u32 payload_len)
{
struct ceph_osd_req_op *ops;
- ops = kzalloc(sizeof (*ops) * (num_ops + 1), GFP_NOIO);
+ ops = kzalloc(num_op * sizeof (*ops), GFP_NOIO);
if (!ops)
return NULL;
@@ -1149,7 +1149,7 @@ static int rbd_do_request(struct request *rq,
(unsigned long long) len, coll, coll_index);
osdc = &rbd_dev->rbd_client->client->osdc;
- osd_req = ceph_osdc_alloc_request(osdc, snapc, ops, false, GFP_NOIO);
+ osd_req = ceph_osdc_alloc_request(osdc, snapc, num_op, false, GFP_NOIO);
if (!osd_req) {
ret = -ENOMEM;
goto done_pages;
@@ -1178,7 +1178,8 @@ static int rbd_do_request(struct request *rq,
ofs, &len, &bno, osd_req, ops);
rbd_assert(ret == 0);
- ceph_osdc_build_request(osd_req, ofs, len, ops, snapc, snapid, &mtime);
+ ceph_osdc_build_request(osd_req, ofs, len, num_op, ops,
+ snapc, snapid, &mtime);
if (linger_req) {
ceph_osdc_set_request_linger(osdc, osd_req);
diff --git a/include/linux/ceph/osd_client.h
b/include/linux/ceph/osd_client.h
index 75f56d3..2b04d05 100644
--- a/include/linux/ceph/osd_client.h
+++ b/include/linux/ceph/osd_client.h
@@ -214,12 +214,13 @@ extern int ceph_calc_raw_layout(struct
ceph_file_layout *layout,
extern struct ceph_osd_request *ceph_osdc_alloc_request(struct
ceph_osd_client *osdc,
struct ceph_snap_context *snapc,
- struct ceph_osd_req_op *ops,
+ unsigned int num_op,
bool use_mempool,
gfp_t gfp_flags);
extern void ceph_osdc_build_request(struct ceph_osd_request *req,
u64 off, u64 len,
+ unsigned int num_op,
struct ceph_osd_req_op *src_ops,
struct ceph_snap_context *snapc,
u64 snap_id,
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 150166b..06625fa 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -152,25 +152,14 @@ void ceph_osdc_release_request(struct kref *kref)
}
EXPORT_SYMBOL(ceph_osdc_release_request);
-static int get_num_ops(struct ceph_osd_req_op *ops)
-{
- int i = 0;
-
- while (ops[i].op)
- i++;
-
- return i;
-}
-
struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client
*osdc,
struct ceph_snap_context *snapc,
- struct ceph_osd_req_op *ops,
+ unsigned int num_op,
bool use_mempool,
gfp_t gfp_flags)
{
struct ceph_osd_request *req;
struct ceph_msg *msg;
- int num_op = get_num_ops(ops);
size_t msg_size = sizeof(struct ceph_osd_request_head);
msg_size += num_op*sizeof(struct ceph_osd_op);
@@ -309,7 +298,7 @@ static void osd_req_encode_op(struct
ceph_osd_request *req,
*
*/
void ceph_osdc_build_request(struct ceph_osd_request *req,
- u64 off, u64 len,
+ u64 off, u64 len, unsigned int num_op,
struct ceph_osd_req_op *src_ops,
struct ceph_snap_context *snapc, u64 snap_id,
struct timespec *mtime)
@@ -319,7 +308,6 @@ void ceph_osdc_build_request(struct ceph_osd_request
*req,
struct ceph_osd_req_op *src_op;
struct ceph_osd_op *op;
void *p;
- int num_op = get_num_ops(src_ops);
size_t msg_size = sizeof(*head) + num_op*sizeof(*op);
int flags = req->r_flags;
u64 data_len = 0;
@@ -338,20 +326,17 @@ void ceph_osdc_build_request(struct
ceph_osd_request *req,
head->flags = cpu_to_le32(flags);
if (flags & CEPH_OSD_FLAG_WRITE)
ceph_encode_timespec(&head->mtime, mtime);
+ BUG_ON(num_op > (unsigned int) ((u16) -1));
head->num_ops = cpu_to_le16(num_op);
-
/* fill in oid */
head->object_len = cpu_to_le32(req->r_oid_len);
memcpy(p, req->r_oid, req->r_oid_len);
p += req->r_oid_len;
src_op = src_ops;
- while (src_op->op) {
- osd_req_encode_op(req, op, src_op);
- src_op++;
- op++;
- }
+ while (num_op--)
+ osd_req_encode_op(req, op++, src_op++);
data_len += req->r_trail.length;
@@ -406,23 +391,24 @@ struct ceph_osd_request
*ceph_osdc_new_request(struct ceph_osd_client *osdc,
bool use_mempool, int num_reply,
int page_align)
{
- struct ceph_osd_req_op ops[3];
+ struct ceph_osd_req_op ops[2];
struct ceph_osd_request *req;
+ unsigned int num_op = 1;
int r;
+ memset(&ops, 0, sizeof ops);
+
ops[0].op = opcode;
ops[0].extent.truncate_seq = truncate_seq;
ops[0].extent.truncate_size = truncate_size;
- ops[0].payload_len = 0;
if (do_sync) {
ops[1].op = CEPH_OSD_OP_STARTSYNC;
- ops[1].payload_len = 0;
- ops[2].op = 0;
- } else
- ops[1].op = 0;
+ num_op++;
+ }
- req = ceph_osdc_alloc_request(osdc, snapc, ops, use_mempool, GFP_NOFS);
+ req = ceph_osdc_alloc_request(osdc, snapc, num_op, use_mempool,
+ GFP_NOFS);
if (!req)
return ERR_PTR(-ENOMEM);
req->r_flags = flags;
@@ -438,7 +424,8 @@ struct ceph_osd_request
*ceph_osdc_new_request(struct ceph_osd_client *osdc,
req->r_num_pages = calc_pages_for(page_align, *plen);
req->r_page_alignment = page_align;
- ceph_osdc_build_request(req, off, *plen, ops, snapc, vino.snap, mtime);
+ ceph_osdc_build_request(req, off, *plen, num_op, ops,
+ snapc, vino.snap, mtime);
return req;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH REPOST 3/4] rbd: there is really only one op
2013-01-04 14:43 [PATCH REPOST 0/4] rbd: explicitly support only one osd op Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 1/4] rbd: pass num_op with ops array Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 2/4] libceph: pass num_op with ops Alex Elder
@ 2013-01-04 14:45 ` Alex Elder
2013-01-04 14:45 ` [PATCH REPOST 4/4] rbd: assume single op in a request Alex Elder
2013-01-17 2:27 ` [PATCH REPOST 0/4] rbd: explicitly support only one osd op Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-01-04 14:45 UTC (permalink / raw)
To: ceph-devel@vger.kernel.org
Throughout the rbd code there are spots where it appears we can
handle an osd request containing more than one osd request op.
But that is only the way it appears. In fact, currently only one
operation at a time can be supported, and supporting more than
one will require much more than fleshing out the support that's
there now.
This patch changes names to make it perfectly clear that anywhere
we're dealing with a block of ops, we're in fact dealing with
exactly one of them. We'll be able to simplify some things as
a result.
When multiple op support is implemented, we can update things again
accordingly.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 118
++++++++++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 62 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 0e6cc67..d3472db 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1023,32 +1023,26 @@ out_err:
return NULL;
}
-/*
- * helpers for osd request op vectors.
- */
-static struct ceph_osd_req_op *rbd_create_rw_ops(int num_op,
- int opcode, u32 payload_len)
+static struct ceph_osd_req_op *rbd_create_rw_op(int opcode, u32
payload_len)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
- ops = kzalloc(num_op * sizeof (*ops), GFP_NOIO);
- if (!ops)
+ op = kzalloc(sizeof (*op), GFP_NOIO);
+ if (!op)
return NULL;
-
- ops[0].op = opcode;
-
/*
* op extent offset and length will be set later on
* in calc_raw_layout()
*/
- ops[0].payload_len = payload_len;
+ op->op = opcode;
+ op->payload_len = payload_len;
- return ops;
+ return op;
}
-static void rbd_destroy_ops(struct ceph_osd_req_op *ops)
+static void rbd_destroy_op(struct ceph_osd_req_op *op)
{
- kfree(ops);
+ kfree(op);
}
static void rbd_coll_end_req_index(struct request *rq,
@@ -1314,7 +1308,7 @@ static int rbd_do_op(struct request *rq,
u64 seg_ofs;
u64 seg_len;
int ret;
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
u32 payload_len;
int opcode;
int flags;
@@ -1340,8 +1334,8 @@ static int rbd_do_op(struct request *rq,
}
ret = -ENOMEM;
- ops = rbd_create_rw_ops(1, opcode, payload_len);
- if (!ops)
+ op = rbd_create_rw_op(opcode, payload_len);
+ if (!op)
goto done;
/* we've taken care of segment sizes earlier when we
@@ -1354,13 +1348,13 @@ static int rbd_do_op(struct request *rq,
bio,
NULL, 0,
flags,
- 1, ops,
+ 1, op,
coll, coll_index,
rbd_req_cb, 0, NULL);
if (ret < 0)
rbd_coll_end_req_index(rq, coll, coll_index,
(s32) ret, seg_len);
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
done:
kfree(seg_name);
return ret;
@@ -1375,16 +1369,16 @@ static int rbd_req_sync_read(struct rbd_device
*rbd_dev,
char *buf,
u64 *ver)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
int ret;
- ops = rbd_create_rw_ops(1, CEPH_OSD_OP_READ, 0);
- if (!ops)
+ op = rbd_create_rw_op(CEPH_OSD_OP_READ, 0);
+ if (!op)
return -ENOMEM;
ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ,
- 1, ops, object_name, ofs, len, buf, NULL, ver);
- rbd_destroy_ops(ops);
+ 1, op, object_name, ofs, len, buf, NULL, ver);
+ rbd_destroy_op(op);
return ret;
}
@@ -1396,26 +1390,26 @@ static int rbd_req_sync_notify_ack(struct
rbd_device *rbd_dev,
u64 ver,
u64 notify_id)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
int ret;
- ops = rbd_create_rw_ops(1, CEPH_OSD_OP_NOTIFY_ACK, 0);
- if (!ops)
+ op = rbd_create_rw_op(CEPH_OSD_OP_NOTIFY_ACK, 0);
+ if (!op)
return -ENOMEM;
- ops[0].watch.ver = cpu_to_le64(ver);
- ops[0].watch.cookie = notify_id;
- ops[0].watch.flag = 0;
+ op->watch.ver = cpu_to_le64(ver);
+ op->watch.cookie = notify_id;
+ op->watch.flag = 0;
ret = rbd_do_request(NULL, rbd_dev, NULL, CEPH_NOSNAP,
rbd_dev->header_name, 0, 0, NULL,
NULL, 0,
CEPH_OSD_FLAG_READ,
- 1, ops,
+ 1, op,
NULL, 0,
rbd_simple_req_cb, 0, NULL);
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
return ret;
}
@@ -1444,12 +1438,12 @@ static void rbd_watch_cb(u64 ver, u64 notify_id,
u8 opcode, void *data)
*/
static int rbd_req_sync_watch(struct rbd_device *rbd_dev)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
int ret;
- ops = rbd_create_rw_ops(1, CEPH_OSD_OP_WATCH, 0);
- if (!ops)
+ op = rbd_create_rw_op(CEPH_OSD_OP_WATCH, 0);
+ if (!op)
return -ENOMEM;
ret = ceph_osdc_create_event(osdc, rbd_watch_cb, 0,
@@ -1457,13 +1451,13 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev)
if (ret < 0)
goto fail;
- ops[0].watch.ver = cpu_to_le64(rbd_dev->header.obj_version);
- ops[0].watch.cookie = cpu_to_le64(rbd_dev->watch_event->cookie);
- ops[0].watch.flag = 1;
+ op->watch.ver = cpu_to_le64(rbd_dev->header.obj_version);
+ op->watch.cookie = cpu_to_le64(rbd_dev->watch_event->cookie);
+ op->watch.flag = 1;
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- 1, ops,
+ 1, op,
rbd_dev->header_name,
0, 0, NULL,
&rbd_dev->watch_request, NULL);
@@ -1471,14 +1465,14 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev)
if (ret < 0)
goto fail_event;
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
return 0;
fail_event:
ceph_osdc_cancel_event(rbd_dev->watch_event);
rbd_dev->watch_event = NULL;
fail:
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
return ret;
}
@@ -1487,25 +1481,25 @@ fail:
*/
static int rbd_req_sync_unwatch(struct rbd_device *rbd_dev)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
int ret;
- ops = rbd_create_rw_ops(1, CEPH_OSD_OP_WATCH, 0);
- if (!ops)
+ op = rbd_create_rw_op(CEPH_OSD_OP_WATCH, 0);
+ if (!op)
return -ENOMEM;
- ops[0].watch.ver = 0;
- ops[0].watch.cookie = cpu_to_le64(rbd_dev->watch_event->cookie);
- ops[0].watch.flag = 0;
+ op->watch.ver = 0;
+ op->watch.cookie = cpu_to_le64(rbd_dev->watch_event->cookie);
+ op->watch.flag = 0;
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- 1, ops,
+ 1, op,
rbd_dev->header_name,
0, 0, NULL, NULL, NULL);
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
ceph_osdc_cancel_event(rbd_dev->watch_event);
rbd_dev->watch_event = NULL;
return ret;
@@ -1524,7 +1518,7 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
size_t inbound_size,
u64 *ver)
{
- struct ceph_osd_req_op *ops;
+ struct ceph_osd_req_op *op;
int class_name_len = strlen(class_name);
int method_name_len = strlen(method_name);
int payload_size;
@@ -1539,23 +1533,23 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
* operation.
*/
payload_size = class_name_len + method_name_len + outbound_size;
- ops = rbd_create_rw_ops(1, CEPH_OSD_OP_CALL, payload_size);
- if (!ops)
+ op = rbd_create_rw_op(CEPH_OSD_OP_CALL, payload_size);
+ if (!op)
return -ENOMEM;
- ops[0].cls.class_name = class_name;
- ops[0].cls.class_len = (__u8) class_name_len;
- ops[0].cls.method_name = method_name;
- ops[0].cls.method_len = (__u8) method_name_len;
- ops[0].cls.argc = 0;
- ops[0].cls.indata = outbound;
- ops[0].cls.indata_len = outbound_size;
+ op->cls.class_name = class_name;
+ op->cls.class_len = (__u8) class_name_len;
+ op->cls.method_name = method_name;
+ op->cls.method_len = (__u8) method_name_len;
+ op->cls.argc = 0;
+ op->cls.indata = outbound;
+ op->cls.indata_len = outbound_size;
- ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, 1, ops,
+ ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, 1, op,
object_name, 0, inbound_size, inbound,
NULL, ver);
- rbd_destroy_ops(ops);
+ rbd_destroy_op(op);
dout("cls_exec returned %d\n", ret);
return ret;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH REPOST 4/4] rbd: assume single op in a request
2013-01-04 14:43 [PATCH REPOST 0/4] rbd: explicitly support only one osd op Alex Elder
` (2 preceding siblings ...)
2013-01-04 14:45 ` [PATCH REPOST 3/4] rbd: there is really only one op Alex Elder
@ 2013-01-04 14:45 ` Alex Elder
2013-01-17 2:27 ` [PATCH REPOST 0/4] rbd: explicitly support only one osd op Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Alex Elder @ 2013-01-04 14:45 UTC (permalink / raw)
To: ceph-devel@vger.kernel.org
We now know that every of rbd_req_sync_op() passes an array of
exactly one operation, as evidenced by all callers passing 1 as its
num_op argument. So get rid of that argument, assuming a single op.
Similarly, we now know that all callers of rbd_do_request() pass 1
as the num_op value, so that parameter can be eliminated as well.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d3472db..a57d6e9 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1113,8 +1113,7 @@ static int rbd_do_request(struct request *rq,
struct page **pages,
int num_pages,
int flags,
- unsigned int num_op,
- struct ceph_osd_req_op *ops,
+ struct ceph_osd_req_op *op,
struct rbd_req_coll *coll,
int coll_index,
void (*rbd_cb)(struct ceph_osd_request *,
@@ -1143,7 +1142,7 @@ static int rbd_do_request(struct request *rq,
(unsigned long long) len, coll, coll_index);
osdc = &rbd_dev->rbd_client->client->osdc;
- osd_req = ceph_osdc_alloc_request(osdc, snapc, num_op, false, GFP_NOIO);
+ osd_req = ceph_osdc_alloc_request(osdc, snapc, 1, false, GFP_NOIO);
if (!osd_req) {
ret = -ENOMEM;
goto done_pages;
@@ -1169,10 +1168,10 @@ static int rbd_do_request(struct request *rq,
rbd_layout_init(&osd_req->r_file_layout, rbd_dev->spec->pool_id);
ret = ceph_calc_raw_layout(&osd_req->r_file_layout,
- ofs, &len, &bno, osd_req, ops);
+ ofs, &len, &bno, osd_req, op);
rbd_assert(ret == 0);
- ceph_osdc_build_request(osd_req, ofs, len, num_op, ops,
+ ceph_osdc_build_request(osd_req, ofs, len, 1, op,
snapc, snapid, &mtime);
if (linger_req) {
@@ -1255,8 +1254,7 @@ static void rbd_simple_req_cb(struct
ceph_osd_request *osd_req,
*/
static int rbd_req_sync_op(struct rbd_device *rbd_dev,
int flags,
- unsigned int num_op,
- struct ceph_osd_req_op *ops,
+ struct ceph_osd_req_op *op,
const char *object_name,
u64 ofs, u64 inbound_size,
char *inbound,
@@ -1267,7 +1265,7 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
struct page **pages;
int num_pages;
- rbd_assert(ops != NULL);
+ rbd_assert(op != NULL);
num_pages = calc_pages_for(ofs, inbound_size);
pages = ceph_alloc_page_vector(num_pages, GFP_KERNEL);
@@ -1278,7 +1276,7 @@ static int rbd_req_sync_op(struct rbd_device *rbd_dev,
object_name, ofs, inbound_size, NULL,
pages, num_pages,
flags,
- num_op, ops,
+ op,
NULL, 0,
NULL,
linger_req, ver);
@@ -1348,7 +1346,7 @@ static int rbd_do_op(struct request *rq,
bio,
NULL, 0,
flags,
- 1, op,
+ op,
coll, coll_index,
rbd_req_cb, 0, NULL);
if (ret < 0)
@@ -1377,7 +1375,7 @@ static int rbd_req_sync_read(struct rbd_device
*rbd_dev,
return -ENOMEM;
ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ,
- 1, op, object_name, ofs, len, buf, NULL, ver);
+ op, object_name, ofs, len, buf, NULL, ver);
rbd_destroy_op(op);
return ret;
@@ -1405,7 +1403,7 @@ static int rbd_req_sync_notify_ack(struct
rbd_device *rbd_dev,
rbd_dev->header_name, 0, 0, NULL,
NULL, 0,
CEPH_OSD_FLAG_READ,
- 1, op,
+ op,
NULL, 0,
rbd_simple_req_cb, 0, NULL);
@@ -1457,7 +1455,7 @@ static int rbd_req_sync_watch(struct rbd_device
*rbd_dev)
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- 1, op,
+ op,
rbd_dev->header_name,
0, 0, NULL,
&rbd_dev->watch_request, NULL);
@@ -1494,7 +1492,7 @@ static int rbd_req_sync_unwatch(struct rbd_device
*rbd_dev)
ret = rbd_req_sync_op(rbd_dev,
CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- 1, op,
+ op,
rbd_dev->header_name,
0, 0, NULL, NULL, NULL);
@@ -1545,7 +1543,7 @@ static int rbd_req_sync_exec(struct rbd_device
*rbd_dev,
op->cls.indata = outbound;
op->cls.indata_len = outbound_size;
- ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, 1, op,
+ ret = rbd_req_sync_op(rbd_dev, CEPH_OSD_FLAG_READ, op,
object_name, 0, inbound_size, inbound,
NULL, ver);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH REPOST 0/4] rbd: explicitly support only one osd op
2013-01-04 14:43 [PATCH REPOST 0/4] rbd: explicitly support only one osd op Alex Elder
` (3 preceding siblings ...)
2013-01-04 14:45 ` [PATCH REPOST 4/4] rbd: assume single op in a request Alex Elder
@ 2013-01-17 2:27 ` Josh Durgin
4 siblings, 0 replies; 6+ messages in thread
From: Josh Durgin @ 2013-01-17 2:27 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel@vger.kernel.org
On 01/04/2013 06:43 AM, Alex Elder wrote:
> An osd request can be made up of multiple "ops", all of which are
> completed (or not) transactionally. There is partial support for
> multiple ops in an rbd request in the rbd code, but it's incomplete
> and not even supported by the osd client or the messenger right now.
>
> I see three problems with this partial implementation: it gives a
> false impression of how things work; it complicates some code in
> some cases where it's not necessary; and it may constrain how one
> might pursue fully implementing multiple ops in a request to ways
> that don't fit well with how we want to do things.
>
> So this series just simplifies things, making it explicit that there
> is only one op in an kernel osd client request right now.
>
> -Alex
>
> [PATCH REPOST 1/4] rbd: pass num_op with ops array
> [PATCH REPOST 2/4] libceph: pass num_op with ops
> [PATCH REPOST 3/4] rbd: there is really only one op
> [PATCH REPOST 4/4] rbd: assume single op in a request
These look good.
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
^ permalink raw reply [flat|nested] 6+ messages in thread