| Message ID | 5153020B.7050403@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 27 Mar 2013, Alex Elder wrote: > (This patch is available in the branch "review/wip-4559" in the > ceph-client git repository.) > > A result of ENOENT from a read request for an object that's part of > an rbd image indicates that there is a hole in that portion of the > image. Similarly, a short read for such an object indicates that > the remainder of the read should be interpreted a full read with > zeros filling out the end of the request. > > This behavior is not correct for objects that are not backing rbd > image data. Currently rbd_img_obj_request_callback() assumes it > should be done for all objects. > > Change rbd_img_obj_request_callback() so it only does this zeroing > for image objects. Encapsulate that special handling in its own > function. Add an assertion that the image object request is a bio > request, since we assume that (and we currently don't support any > other types). Does this only affect the current -rc or is this problem older than that? We should add some workunit tests that verify that mapping nonexistent images behaves. sage > > Reported-by: Dan van der Ster <dan@vanderster.com> > Signed-off-by: Alex Elder <elder@inktank.com> > --- > drivers/block/rbd.c | 47 ++++++++++++++++++++++++++++++----------------- > 1 file changed, 30 insertions(+), 17 deletions(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index cc74b2c..d54a045 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -1264,6 +1264,32 @@ static bool obj_request_done_test(struct > rbd_obj_request *obj_request) > return atomic_read(&obj_request->done) != 0; > } > > +static void > +rbd_img_obj_request_read_callback(struct rbd_obj_request *obj_request) > +{ > + dout("%s: obj %p img %p result %d %llu/%llu\n", __func__, > + obj_request, obj_request->img_request, obj_request->result, > + obj_request->xferred, obj_request->length); > + /* > + * ENOENT means a hole in the image. We zero-fill the > + * entire length of the request. A short read also implies > + * zero-fill to the end of the request. Either way we > + * update the xferred count to indicate the whole request > + * was satisfied. > + */ > + BUG_ON(obj_request->type != OBJ_REQUEST_BIO); > + if (obj_request->result == -ENOENT) { > + zero_bio_chain(obj_request->bio_list, 0); > + obj_request->result = 0; > + obj_request->xferred = obj_request->length; > + } else if (obj_request->xferred < obj_request->length && > + !obj_request->result) { > + zero_bio_chain(obj_request->bio_list, obj_request->xferred); > + obj_request->xferred = obj_request->length; > + } > + obj_request_done_set(obj_request); > +} > + > static void rbd_obj_request_complete(struct rbd_obj_request *obj_request) > { > dout("%s: obj %p cb %p\n", __func__, obj_request, > @@ -1284,23 +1310,10 @@ static void rbd_osd_read_callback(struct > rbd_obj_request *obj_request) > { > dout("%s: obj %p result %d %llu/%llu\n", __func__, obj_request, > obj_request->result, obj_request->xferred, obj_request->length); > - /* > - * ENOENT means a hole in the object. We zero-fill the > - * entire length of the request. A short read also implies > - * zero-fill to the end of the request. Either way we > - * update the xferred count to indicate the whole request > - * was satisfied. > - */ > - if (obj_request->result == -ENOENT) { > - zero_bio_chain(obj_request->bio_list, 0); > - obj_request->result = 0; > - obj_request->xferred = obj_request->length; > - } else if (obj_request->xferred < obj_request->length && > - !obj_request->result) { > - zero_bio_chain(obj_request->bio_list, obj_request->xferred); > - obj_request->xferred = obj_request->length; > - } > - obj_request_done_set(obj_request); > + if (obj_request->img_request) > + rbd_img_obj_request_read_callback(obj_request); > + else > + obj_request_done_set(obj_request); > } > > static void rbd_osd_write_callback(struct rbd_obj_request *obj_request) > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 27, 2013 at 5:10 PM, Sage Weil <sage@inktank.com> wrote:
> Does this only affect the current -rc or is this problem older than that?
The BUG I reported that triggered this was on 3.8.4.
--
Dan
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/27/2013 11:26 AM, Dan van der Ster wrote: > On Wed, Mar 27, 2013 at 5:10 PM, Sage Weil <sage@inktank.com> wrote: >> Does this only affect the current -rc or is this problem older than that? The bug I found should affect only the new request code, which if I am not mistaken got to sent to Linus during this cycle (3.9-rcX). > The BUG I reported that triggered this was on 3.8.4. OK, now I'm trying to reproduce the problem with 3.8.4 and I'm not seeing any crashes... Hmm. I'll look a little more closely. -Alex > -- > Dan > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index cc74b2c..d54a045 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -1264,6 +1264,32 @@ static bool obj_request_done_test(struct rbd_obj_request *obj_request) return atomic_read(&obj_request->done) != 0; } +static void +rbd_img_obj_request_read_callback(struct rbd_obj_request *obj_request) +{ + dout("%s: obj %p img %p result %d %llu/%llu\n", __func__, + obj_request, obj_request->img_request, obj_request->result, + obj_request->xferred, obj_request->length); + /* + * ENOENT means a hole in the image. We zero-fill the + * entire length of the request. A short read also implies + * zero-fill to the end of the request. Either way we + * update the xferred count to indicate the whole request + * was satisfied. + */ + BUG_ON(obj_request->type != OBJ_REQUEST_BIO); + if (obj_request->result == -ENOENT) { + zero_bio_chain(obj_request->bio_list, 0); + obj_request->result = 0; + obj_request->xferred = obj_request->length; + } else if (obj_request->xferred < obj_request->length && + !obj_request->result) { + zero_bio_chain(obj_request->bio_list, obj_request->xferred); + obj_request->xferred = obj_request->length; + } + obj_request_done_set(obj_request); +} + static void rbd_obj_request_complete(struct rbd_obj_request *obj_request) {
(This patch is available in the branch "review/wip-4559" in the ceph-client git repository.) A result of ENOENT from a read request for an object that's part of an rbd image indicates that there is a hole in that portion of the image. Similarly, a short read for such an object indicates that the remainder of the read should be interpreted a full read with zeros filling out the end of the request. This behavior is not correct for objects that are not backing rbd image data. Currently rbd_img_obj_request_callback() assumes it should be done for all objects. Change rbd_img_obj_request_callback() so it only does this zeroing for image objects. Encapsulate that special handling in its own function. Add an assertion that the image object request is a bio request, since we assume that (and we currently don't support any other types). Reported-by: Dan van der Ster <dan@vanderster.com> Signed-off-by: Alex Elder <elder@inktank.com> --- drivers/block/rbd.c | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) dout("%s: obj %p cb %p\n", __func__, obj_request, @@ -1284,23 +1310,10 @@ static void rbd_osd_read_callback(struct rbd_obj_request *obj_request) { dout("%s: obj %p result %d %llu/%llu\n", __func__, obj_request, obj_request->result, obj_request->xferred, obj_request->length); - /* - * ENOENT means a hole in the object. We zero-fill the - * entire length of the request. A short read also implies - * zero-fill to the end of the request. Either way we - * update the xferred count to indicate the whole request - * was satisfied. - */ - if (obj_request->result == -ENOENT) { - zero_bio_chain(obj_request->bio_list, 0); - obj_request->result = 0; - obj_request->xferred = obj_request->length; - } else if (obj_request->xferred < obj_request->length && - !obj_request->result) { - zero_bio_chain(obj_request->bio_list, obj_request->xferred); - obj_request->xferred = obj_request->length; - } - obj_request_done_set(obj_request); + if (obj_request->img_request) + rbd_img_obj_request_read_callback(obj_request); + else + obj_request_done_set(obj_request); } static void rbd_osd_write_callback(struct rbd_obj_request *obj_request)