* [PATCH 0/3] rbd: simplify rbd_do_op() et al
@ 2012-10-11 2:17 Alex Elder
2012-10-11 2:19 ` [PATCH 1/3] rbd: kill rbd_req_{read,write}() Alex Elder
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alex Elder @ 2012-10-11 2:17 UTC (permalink / raw)
To: ceph-devel
These three patches simplify a few paths through the code
involving read and write requests.
-Alex
[PATCH 1/3] rbd: kill rbd_req_{read,write}()
[PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags
[PATCH 3/3] rbd: consolidate rbd_do_op() calls
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] rbd: kill rbd_req_{read,write}()
2012-10-11 2:17 [PATCH 0/3] rbd: simplify rbd_do_op() et al Alex Elder
@ 2012-10-11 2:19 ` Alex Elder
2012-10-24 17:24 ` Josh Durgin
2012-10-11 2:19 ` [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags Alex Elder
2012-10-11 2:19 ` [PATCH 3/3] rbd: consolidate rbd_do_op() calls Alex Elder
2 siblings, 1 reply; 8+ messages in thread
From: Alex Elder @ 2012-10-11 2:19 UTC (permalink / raw)
To: ceph-devel
Both rbd_req_read() and rbd_req_write() are simple wrapper routines
for rbd_do_op(), and each is only called once. Replace each wrapper
call with a direct call to rbd_do_op(), and get rid of the wrapper
functions.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 60
+++++++++++----------------------------------------
1 file changed, 13 insertions(+), 47 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 8f56d37..cc74b36 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1211,41 +1211,6 @@ done:
}
/*
- * Request async osd write
- */
-static int rbd_req_write(struct request *rq,
- struct rbd_device *rbd_dev,
- struct ceph_snap_context *snapc,
- u64 ofs, u64 len,
- struct bio *bio,
- struct rbd_req_coll *coll,
- int coll_index)
-{
- return rbd_do_op(rq, rbd_dev, snapc, CEPH_NOSNAP,
- CEPH_OSD_OP_WRITE,
- CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
- ofs, len, bio, coll, coll_index);
-}
-
-/*
- * Request async osd read
- */
-static int rbd_req_read(struct request *rq,
- struct rbd_device *rbd_dev,
- u64 snapid,
- u64 ofs, u64 len,
- struct bio *bio,
- struct rbd_req_coll *coll,
- int coll_index)
-{
- return rbd_do_op(rq, rbd_dev, NULL,
- snapid,
- CEPH_OSD_OP_READ,
- CEPH_OSD_FLAG_READ,
- ofs, len, bio, coll, coll_index);
-}
-
-/*
* Request sync osd read
*/
static int rbd_req_sync_read(struct rbd_device *rbd_dev,
@@ -1550,21 +1515,22 @@ static void rbd_rq_fn(struct request_queue *q)
goto next_seg;
}
-
/* init OSD command: write or read */
if (do_write)
- rbd_req_write(rq, rbd_dev,
- snapc,
- ofs,
- op_size, bio,
- coll, cur_seg);
+ (void) rbd_do_op(rq, rbd_dev,
+ snapc, CEPH_NOSNAP,
+ CEPH_OSD_OP_WRITE,
+ CEPH_OSD_FLAG_WRITE |
+ CEPH_OSD_FLAG_ONDISK,
+ ofs, op_size, bio,
+ coll, cur_seg);
else
- rbd_req_read(rq, rbd_dev,
- rbd_dev->mapping.snap_id,
- ofs,
- op_size, bio,
- coll, cur_seg);
-
+ (void) rbd_do_op(rq, rbd_dev,
+ NULL, rbd_dev->mapping.snap_id,
+ CEPH_OSD_OP_READ,
+ CEPH_OSD_FLAG_READ,
+ ofs, op_size, bio,
+ coll, cur_seg);
next_seg:
size -= op_size;
ofs += op_size;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags
2012-10-11 2:17 [PATCH 0/3] rbd: simplify rbd_do_op() et al Alex Elder
2012-10-11 2:19 ` [PATCH 1/3] rbd: kill rbd_req_{read,write}() Alex Elder
@ 2012-10-11 2:19 ` Alex Elder
2012-10-24 17:26 ` Josh Durgin
2012-10-11 2:19 ` [PATCH 3/3] rbd: consolidate rbd_do_op() calls Alex Elder
2 siblings, 1 reply; 8+ messages in thread
From: Alex Elder @ 2012-10-11 2:19 UTC (permalink / raw)
To: ceph-devel
The only callers of rbd_do_op() are in rbd_rq_fn(), where call one
is used for writes and the other used for reads. The request passed
to rbd_do_op() already encodes the I/O direction, and that
information can be used inside the function to set the opcode and
flags value (rather than passing them in as arguments).
So get rid of the opcode and flags arguments to rbd_do_op().
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index cc74b36..396af14 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1164,7 +1164,6 @@ static int rbd_do_op(struct request *rq,
struct rbd_device *rbd_dev,
struct ceph_snap_context *snapc,
u64 snapid,
- int opcode, int flags,
u64 ofs, u64 len,
struct bio *bio,
struct rbd_req_coll *coll,
@@ -1176,6 +1175,8 @@ static int rbd_do_op(struct request *rq,
int ret;
struct ceph_osd_req_op *ops;
u32 payload_len;
+ int opcode;
+ int flags;
seg_name = rbd_segment_name(rbd_dev, ofs);
if (!seg_name)
@@ -1183,7 +1184,15 @@ static int rbd_do_op(struct request *rq,
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);
+ if (rq_data_dir(rq) == WRITE) {
+ opcode = CEPH_OSD_OP_WRITE;
+ flags = CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_ONDISK;
+ payload_len = seg_len;
+ } else {
+ opcode = CEPH_OSD_OP_READ;
+ flags = CEPH_OSD_FLAG_READ;
+ payload_len = 0;
+ }
ret = -ENOMEM;
ops = rbd_create_rw_ops(1, opcode, payload_len);
@@ -1519,16 +1528,11 @@ static void rbd_rq_fn(struct request_queue *q)
if (do_write)
(void) rbd_do_op(rq, rbd_dev,
snapc, CEPH_NOSNAP,
- CEPH_OSD_OP_WRITE,
- CEPH_OSD_FLAG_WRITE |
- CEPH_OSD_FLAG_ONDISK,
ofs, op_size, bio,
coll, cur_seg);
else
(void) rbd_do_op(rq, rbd_dev,
NULL, rbd_dev->mapping.snap_id,
- CEPH_OSD_OP_READ,
- CEPH_OSD_FLAG_READ,
ofs, op_size, bio,
coll, cur_seg);
next_seg:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] rbd: consolidate rbd_do_op() calls
2012-10-11 2:17 [PATCH 0/3] rbd: simplify rbd_do_op() et al Alex Elder
2012-10-11 2:19 ` [PATCH 1/3] rbd: kill rbd_req_{read,write}() Alex Elder
2012-10-11 2:19 ` [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags Alex Elder
@ 2012-10-11 2:19 ` Alex Elder
2012-10-24 17:31 ` Josh Durgin
2 siblings, 1 reply; 8+ messages in thread
From: Alex Elder @ 2012-10-11 2:19 UTC (permalink / raw)
To: ceph-devel
The two calls to rbd_do_op() from rbd_rq_fn() differ only in the
value passed for the snapshot id and the snapshot context.
For reads the snapshot always comes from the mapping, and for writes
the snapshot id is always CEPH_NOSNAP.
The snapshot context is always null for reads. For writes, the
snapshot context always comes from the rbd header, but it is
acquired under protection of header semaphore and could change
thereafter, so we can't simply use what's available inside
rbd_do_op().
Eliminate the snapid parameter from rbd_do_op(), and set it
based on the I/O direction inside that function instead. Always
pass the snapshot context acquired in the caller, but reset it
to a null pointer inside rbd_do_op() if the operation is a read.
As a result, there is no difference in the read and write calls
to rbd_do_op() made in rbd_rq_fn(), so just call it unconditionally.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 396af14..ca28036 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1163,7 +1163,6 @@ done:
static int rbd_do_op(struct request *rq,
struct rbd_device *rbd_dev,
struct ceph_snap_context *snapc,
- u64 snapid,
u64 ofs, u64 len,
struct bio *bio,
struct rbd_req_coll *coll,
@@ -1177,6 +1176,7 @@ static int rbd_do_op(struct request *rq,
u32 payload_len;
int opcode;
int flags;
+ u64 snapid;
seg_name = rbd_segment_name(rbd_dev, ofs);
if (!seg_name)
@@ -1187,10 +1187,13 @@ static int rbd_do_op(struct request *rq,
if (rq_data_dir(rq) == WRITE) {
opcode = CEPH_OSD_OP_WRITE;
flags = CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_ONDISK;
+ snapid = CEPH_NOSNAP;
payload_len = seg_len;
} else {
opcode = CEPH_OSD_OP_READ;
flags = CEPH_OSD_FLAG_READ;
+ snapc = NULL;
+ snapid = rbd_dev->mapping.snap_id;
payload_len = 0;
}
@@ -1518,24 +1521,13 @@ static void rbd_rq_fn(struct request_queue *q)
kref_get(&coll->kref);
bio = bio_chain_clone(&rq_bio, &next_bio, &bp,
op_size, GFP_ATOMIC);
- if (!bio) {
+ if (bio)
+ (void) rbd_do_op(rq, rbd_dev, snapc,
+ ofs, op_size,
+ bio, coll, cur_seg);
+ else
rbd_coll_end_req_index(rq, coll, cur_seg,
-ENOMEM, op_size);
- goto next_seg;
- }
-
- /* init OSD command: write or read */
- if (do_write)
- (void) rbd_do_op(rq, rbd_dev,
- snapc, CEPH_NOSNAP,
- ofs, op_size, bio,
- coll, cur_seg);
- else
- (void) rbd_do_op(rq, rbd_dev,
- NULL, rbd_dev->mapping.snap_id,
- ofs, op_size, bio,
- coll, cur_seg);
-next_seg:
size -= op_size;
ofs += op_size;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] rbd: kill rbd_req_{read,write}()
2012-10-11 2:19 ` [PATCH 1/3] rbd: kill rbd_req_{read,write}() Alex Elder
@ 2012-10-24 17:24 ` Josh Durgin
0 siblings, 0 replies; 8+ messages in thread
From: Josh Durgin @ 2012-10-24 17:24 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/10/2012 07:19 PM, Alex Elder wrote:
> Both rbd_req_read() and rbd_req_write() are simple wrapper routines
> for rbd_do_op(), and each is only called once. Replace each wrapper
> call with a direct call to rbd_do_op(), and get rid of the wrapper
> functions.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 60
> +++++++++++----------------------------------------
> 1 file changed, 13 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 8f56d37..cc74b36 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1211,41 +1211,6 @@ done:
> }
>
> /*
> - * Request async osd write
> - */
> -static int rbd_req_write(struct request *rq,
> - struct rbd_device *rbd_dev,
> - struct ceph_snap_context *snapc,
> - u64 ofs, u64 len,
> - struct bio *bio,
> - struct rbd_req_coll *coll,
> - int coll_index)
> -{
> - return rbd_do_op(rq, rbd_dev, snapc, CEPH_NOSNAP,
> - CEPH_OSD_OP_WRITE,
> - CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK,
> - ofs, len, bio, coll, coll_index);
> -}
> -
> -/*
> - * Request async osd read
> - */
> -static int rbd_req_read(struct request *rq,
> - struct rbd_device *rbd_dev,
> - u64 snapid,
> - u64 ofs, u64 len,
> - struct bio *bio,
> - struct rbd_req_coll *coll,
> - int coll_index)
> -{
> - return rbd_do_op(rq, rbd_dev, NULL,
> - snapid,
> - CEPH_OSD_OP_READ,
> - CEPH_OSD_FLAG_READ,
> - ofs, len, bio, coll, coll_index);
> -}
> -
> -/*
> * Request sync osd read
> */
> static int rbd_req_sync_read(struct rbd_device *rbd_dev,
> @@ -1550,21 +1515,22 @@ static void rbd_rq_fn(struct request_queue *q)
> goto next_seg;
> }
>
> -
> /* init OSD command: write or read */
> if (do_write)
> - rbd_req_write(rq, rbd_dev,
> - snapc,
> - ofs,
> - op_size, bio,
> - coll, cur_seg);
> + (void) rbd_do_op(rq, rbd_dev,
> + snapc, CEPH_NOSNAP,
> + CEPH_OSD_OP_WRITE,
> + CEPH_OSD_FLAG_WRITE |
> + CEPH_OSD_FLAG_ONDISK,
> + ofs, op_size, bio,
> + coll, cur_seg);
> else
> - rbd_req_read(rq, rbd_dev,
> - rbd_dev->mapping.snap_id,
> - ofs,
> - op_size, bio,
> - coll, cur_seg);
> -
> + (void) rbd_do_op(rq, rbd_dev,
> + NULL, rbd_dev->mapping.snap_id,
> + CEPH_OSD_OP_READ,
> + CEPH_OSD_FLAG_READ,
> + ofs, op_size, bio,
> + coll, cur_seg);
> next_seg:
> size -= op_size;
> ofs += op_size;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags
2012-10-11 2:19 ` [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags Alex Elder
@ 2012-10-24 17:26 ` Josh Durgin
0 siblings, 0 replies; 8+ messages in thread
From: Josh Durgin @ 2012-10-24 17:26 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/10/2012 07:19 PM, Alex Elder wrote:
> The only callers of rbd_do_op() are in rbd_rq_fn(), where call one
> is used for writes and the other used for reads. The request passed
> to rbd_do_op() already encodes the I/O direction, and that
> information can be used inside the function to set the opcode and
> flags value (rather than passing them in as arguments).
>
> So get rid of the opcode and flags arguments to rbd_do_op().
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index cc74b36..396af14 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1164,7 +1164,6 @@ static int rbd_do_op(struct request *rq,
> struct rbd_device *rbd_dev,
> struct ceph_snap_context *snapc,
> u64 snapid,
> - int opcode, int flags,
> u64 ofs, u64 len,
> struct bio *bio,
> struct rbd_req_coll *coll,
> @@ -1176,6 +1175,8 @@ static int rbd_do_op(struct request *rq,
> int ret;
> struct ceph_osd_req_op *ops;
> u32 payload_len;
> + int opcode;
> + int flags;
>
> seg_name = rbd_segment_name(rbd_dev, ofs);
> if (!seg_name)
> @@ -1183,7 +1184,15 @@ static int rbd_do_op(struct request *rq,
> 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);
> + if (rq_data_dir(rq) == WRITE) {
> + opcode = CEPH_OSD_OP_WRITE;
> + flags = CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_ONDISK;
> + payload_len = seg_len;
> + } else {
> + opcode = CEPH_OSD_OP_READ;
> + flags = CEPH_OSD_FLAG_READ;
> + payload_len = 0;
> + }
>
> ret = -ENOMEM;
> ops = rbd_create_rw_ops(1, opcode, payload_len);
> @@ -1519,16 +1528,11 @@ static void rbd_rq_fn(struct request_queue *q)
> if (do_write)
> (void) rbd_do_op(rq, rbd_dev,
> snapc, CEPH_NOSNAP,
> - CEPH_OSD_OP_WRITE,
> - CEPH_OSD_FLAG_WRITE |
> - CEPH_OSD_FLAG_ONDISK,
> ofs, op_size, bio,
> coll, cur_seg);
> else
> (void) rbd_do_op(rq, rbd_dev,
> NULL, rbd_dev->mapping.snap_id,
> - CEPH_OSD_OP_READ,
> - CEPH_OSD_FLAG_READ,
> ofs, op_size, bio,
> coll, cur_seg);
> next_seg:
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] rbd: consolidate rbd_do_op() calls
2012-10-11 2:19 ` [PATCH 3/3] rbd: consolidate rbd_do_op() calls Alex Elder
@ 2012-10-24 17:31 ` Josh Durgin
2012-10-26 21:58 ` Alex Elder
0 siblings, 1 reply; 8+ messages in thread
From: Josh Durgin @ 2012-10-24 17:31 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
This cleanup makes sense. We should probably check
the return code now as well, as I note below.
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 10/10/2012 07:19 PM, Alex Elder wrote:
> The two calls to rbd_do_op() from rbd_rq_fn() differ only in the
> value passed for the snapshot id and the snapshot context.
>
> For reads the snapshot always comes from the mapping, and for writes
> the snapshot id is always CEPH_NOSNAP.
>
> The snapshot context is always null for reads. For writes, the
> snapshot context always comes from the rbd header, but it is
> acquired under protection of header semaphore and could change
> thereafter, so we can't simply use what's available inside
> rbd_do_op().
>
> Eliminate the snapid parameter from rbd_do_op(), and set it
> based on the I/O direction inside that function instead. Always
> pass the snapshot context acquired in the caller, but reset it
> to a null pointer inside rbd_do_op() if the operation is a read.
>
> As a result, there is no difference in the read and write calls
> to rbd_do_op() made in rbd_rq_fn(), so just call it unconditionally.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 396af14..ca28036 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1163,7 +1163,6 @@ done:
> static int rbd_do_op(struct request *rq,
> struct rbd_device *rbd_dev,
> struct ceph_snap_context *snapc,
> - u64 snapid,
> u64 ofs, u64 len,
> struct bio *bio,
> struct rbd_req_coll *coll,
> @@ -1177,6 +1176,7 @@ static int rbd_do_op(struct request *rq,
> u32 payload_len;
> int opcode;
> int flags;
> + u64 snapid;
>
> seg_name = rbd_segment_name(rbd_dev, ofs);
> if (!seg_name)
> @@ -1187,10 +1187,13 @@ static int rbd_do_op(struct request *rq,
> if (rq_data_dir(rq) == WRITE) {
> opcode = CEPH_OSD_OP_WRITE;
> flags = CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_ONDISK;
> + snapid = CEPH_NOSNAP;
> payload_len = seg_len;
> } else {
> opcode = CEPH_OSD_OP_READ;
> flags = CEPH_OSD_FLAG_READ;
> + snapc = NULL;
> + snapid = rbd_dev->mapping.snap_id;
> payload_len = 0;
> }
>
> @@ -1518,24 +1521,13 @@ static void rbd_rq_fn(struct request_queue *q)
> kref_get(&coll->kref);
> bio = bio_chain_clone(&rq_bio, &next_bio, &bp,
> op_size, GFP_ATOMIC);
> - if (!bio) {
> + if (bio)
> + (void) rbd_do_op(rq, rbd_dev, snapc,
> + ofs, op_size,
> + bio, coll, cur_seg);
We could check the error code here pretty easily now.
> + else
> rbd_coll_end_req_index(rq, coll, cur_seg,
> -ENOMEM, op_size);
> - goto next_seg;
> - }
> -
> - /* init OSD command: write or read */
> - if (do_write)
> - (void) rbd_do_op(rq, rbd_dev,
> - snapc, CEPH_NOSNAP,
> - ofs, op_size, bio,
> - coll, cur_seg);
> - else
> - (void) rbd_do_op(rq, rbd_dev,
> - NULL, rbd_dev->mapping.snap_id,
> - ofs, op_size, bio,
> - coll, cur_seg);
> -next_seg:
> size -= op_size;
> ofs += op_size;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] rbd: consolidate rbd_do_op() calls
2012-10-24 17:31 ` Josh Durgin
@ 2012-10-26 21:58 ` Alex Elder
0 siblings, 0 replies; 8+ messages in thread
From: Alex Elder @ 2012-10-26 21:58 UTC (permalink / raw)
To: Josh Durgin; +Cc: ceph-devel
On 10/24/2012 12:31 PM, Josh Durgin wrote:
> This cleanup makes sense. We should probably check
> the return code now as well, as I note below.
>
> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
I'm going to do this as a followup patch, next week.
-Alex
>
> On 10/10/2012 07:19 PM, Alex Elder wrote:
>> The two calls to rbd_do_op() from rbd_rq_fn() differ only in the
>> value passed for the snapshot id and the snapshot context.
>>
>> For reads the snapshot always comes from the mapping, and for writes
>> the snapshot id is always CEPH_NOSNAP.
>>
>> The snapshot context is always null for reads. For writes, the
>> snapshot context always comes from the rbd header, but it is
>> acquired under protection of header semaphore and could change
>> thereafter, so we can't simply use what's available inside
>> rbd_do_op().
>>
>> Eliminate the snapid parameter from rbd_do_op(), and set it
>> based on the I/O direction inside that function instead. Always
>> pass the snapshot context acquired in the caller, but reset it
>> to a null pointer inside rbd_do_op() if the operation is a read.
>>
>> As a result, there is no difference in the read and write calls
>> to rbd_do_op() made in rbd_rq_fn(), so just call it unconditionally.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>> drivers/block/rbd.c | 26 +++++++++-----------------
>> 1 file changed, 9 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 396af14..ca28036 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -1163,7 +1163,6 @@ done:
>> static int rbd_do_op(struct request *rq,
>> struct rbd_device *rbd_dev,
>> struct ceph_snap_context *snapc,
>> - u64 snapid,
>> u64 ofs, u64 len,
>> struct bio *bio,
>> struct rbd_req_coll *coll,
>> @@ -1177,6 +1176,7 @@ static int rbd_do_op(struct request *rq,
>> u32 payload_len;
>> int opcode;
>> int flags;
>> + u64 snapid;
>>
>> seg_name = rbd_segment_name(rbd_dev, ofs);
>> if (!seg_name)
>> @@ -1187,10 +1187,13 @@ static int rbd_do_op(struct request *rq,
>> if (rq_data_dir(rq) == WRITE) {
>> opcode = CEPH_OSD_OP_WRITE;
>> flags = CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_ONDISK;
>> + snapid = CEPH_NOSNAP;
>> payload_len = seg_len;
>> } else {
>> opcode = CEPH_OSD_OP_READ;
>> flags = CEPH_OSD_FLAG_READ;
>> + snapc = NULL;
>> + snapid = rbd_dev->mapping.snap_id;
>> payload_len = 0;
>> }
>>
>> @@ -1518,24 +1521,13 @@ static void rbd_rq_fn(struct request_queue *q)
>> kref_get(&coll->kref);
>> bio = bio_chain_clone(&rq_bio, &next_bio, &bp,
>> op_size, GFP_ATOMIC);
>> - if (!bio) {
>> + if (bio)
>> + (void) rbd_do_op(rq, rbd_dev, snapc,
>> + ofs, op_size,
>> + bio, coll, cur_seg);
>
> We could check the error code here pretty easily now.
>
>> + else
>> rbd_coll_end_req_index(rq, coll, cur_seg,
>> -ENOMEM, op_size);
>> - goto next_seg;
>> - }
>> -
>> - /* init OSD command: write or read */
>> - if (do_write)
>> - (void) rbd_do_op(rq, rbd_dev,
>> - snapc, CEPH_NOSNAP,
>> - ofs, op_size, bio,
>> - coll, cur_seg);
>> - else
>> - (void) rbd_do_op(rq, rbd_dev,
>> - NULL, rbd_dev->mapping.snap_id,
>> - ofs, op_size, bio,
>> - coll, cur_seg);
>> -next_seg:
>> size -= op_size;
>> ofs += op_size;
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-10-26 21:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-11 2:17 [PATCH 0/3] rbd: simplify rbd_do_op() et al Alex Elder
2012-10-11 2:19 ` [PATCH 1/3] rbd: kill rbd_req_{read,write}() Alex Elder
2012-10-24 17:24 ` Josh Durgin
2012-10-11 2:19 ` [PATCH 2/3] rbd: kill drop rbd_do_op() opcode and flags Alex Elder
2012-10-24 17:26 ` Josh Durgin
2012-10-11 2:19 ` [PATCH 3/3] rbd: consolidate rbd_do_op() calls Alex Elder
2012-10-24 17:31 ` Josh Durgin
2012-10-26 21:58 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox