On 09/19/2016 12:03 PM, Ilya Dryomov wrote:
> Assert once in rbd_img_obj_request_submit().
Generally I prefer validating things as early as possible.
But I also value the simplicity of doing it all in one place.
These are assertions, so the conditions checked should never
really happen anyway. And... now that we're well past doing
huge and rapid changes, the code is showing its maturity, so
the number of assertions is kind of excessive anyway.
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
>
> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
> ---
> drivers/block/rbd.c | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d8b702e3c4d9..027e0817a118 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2739,21 +2739,14 @@ out_err:
> */
> static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request)
> {
> - struct rbd_img_request *img_request = NULL;
> + struct rbd_device *rbd_dev = obj_request->img_request->rbd_dev;
> struct rbd_img_request *parent_request = NULL;
> - struct rbd_device *rbd_dev;
> u64 img_offset;
> u64 length;
> struct page **pages = NULL;
> u32 page_count;
> int result;
>
> - rbd_assert(obj_request_img_data_test(obj_request));
> - rbd_assert(obj_request_type_valid(obj_request->type));
> -
> - img_request = obj_request->img_request;
> - rbd_assert(img_request != NULL);
> - rbd_dev = img_request->rbd_dev;
> rbd_assert(rbd_dev->parent != NULL);
>
> /*
> @@ -2794,10 +2787,11 @@ static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request)
> result = rbd_img_request_fill(parent_request, OBJ_REQUEST_PAGES, pages);
> if (result)
> goto out_err;
> +
> parent_request->copyup_pages = pages;
> parent_request->copyup_page_count = page_count;
> -
> parent_request->callback = rbd_img_obj_parent_read_full_callback;
> +
> result = rbd_img_request_submit(parent_request);
> if (!result)
> return 0;
> @@ -2883,8 +2877,8 @@ out:
>
> static int rbd_img_obj_exists_submit(struct rbd_obj_request *obj_request)
> {
> + struct rbd_device *rbd_dev = obj_request->img_request->rbd_dev;
> struct rbd_obj_request *stat_request;
> - struct rbd_device *rbd_dev;
> struct page **pages = NULL;
> u32 page_count;
> size_t size;
> @@ -2915,8 +2909,6 @@ static int rbd_img_obj_exists_submit(struct rbd_obj_request *obj_request)
> stat_request->pages = pages;
> stat_request->page_count = page_count;
>
> - rbd_assert(obj_request->img_request);
> - rbd_dev = obj_request->img_request->rbd_dev;
> stat_request->osd_req = rbd_osd_req_create(rbd_dev, OBJ_OP_READ, 1,
> stat_request);
> if (!stat_request->osd_req)
> @@ -2940,14 +2932,8 @@ out:
>
> static bool img_obj_request_simple(struct rbd_obj_request *obj_request)
> {
> - struct rbd_img_request *img_request;
> - struct rbd_device *rbd_dev;
> -
> - rbd_assert(obj_request_img_data_test(obj_request));
> -
> - img_request = obj_request->img_request;
> - rbd_assert(img_request);
> - rbd_dev = img_request->rbd_dev;
> + struct rbd_img_request *img_request = obj_request->img_request;
> + struct rbd_device *rbd_dev = img_request->rbd_dev;
>
> /* Reads */
> if (!img_request_write_test(img_request) &&
> @@ -2986,6 +2972,10 @@ static bool img_obj_request_simple(struct rbd_obj_request *obj_request)
>
> static int rbd_img_obj_request_submit(struct rbd_obj_request *obj_request)
> {
> + rbd_assert(obj_request_img_data_test(obj_request));
> + rbd_assert(obj_request_type_valid(obj_request->type));
> + rbd_assert(obj_request->img_request);
> +
> if (img_obj_request_simple(obj_request)) {
> rbd_obj_request_submit(obj_request);
> return 0;
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
