diff mbox

libceph: add error handling for osd_req_op_cls_init

Message ID 1523505895-99462-1-git-send-email-cgxu519@gmx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chengguang Xu April 12, 2018, 4:04 a.m. UTC 
Add proper error handling for osd_req_op_cls_init() to replace
BUG_ON statement when failing from memory allocation.

Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
---
 drivers/block/rbd.c             |  8 ++++++--
 include/linux/ceph/osd_client.h |  2 +-
 net/ceph/osd_client.c           | 12 +++++++++---
 3 files changed, 16 insertions(+), 6 deletions(-)

Comments

Ilya Dryomov April 16, 2018, 3:40 p.m. UTC | #1 
On Thu, Apr 12, 2018 at 6:04 AM, Chengguang Xu <cgxu519@gmx.com> wrote:
> Add proper error handling for osd_req_op_cls_init() to replace
> BUG_ON statement when failing from memory allocation.
>
> Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
> ---
>  drivers/block/rbd.c             |  8 ++++++--
>  include/linux/ceph/osd_client.h |  2 +-
>  net/ceph/osd_client.c           | 12 +++++++++---
>  3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 7320466..ebd1d79 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2348,6 +2348,7 @@ static bool is_zero_bvecs(struct bio_vec *bvecs, u32 bytes)
>  static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
>  {
>         unsigned int num_osd_ops = obj_req->osd_req->r_num_ops;
> +       int err;
>
>         dout("%s obj_req %p bytes %u\n", __func__, obj_req, bytes);
>         rbd_assert(obj_req->osd_req->r_ops[0].op == CEPH_OSD_OP_STAT);
> @@ -2362,6 +2363,11 @@ static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
>         if (!obj_req->osd_req)
>                 return -ENOMEM;
>
> +       err = osd_req_op_cls_init(obj_req->osd_req, 0, CEPH_OSD_OP_CALL, "rbd",
> +                           "copyup");
> +       if (err)
> +               return err;
> +
>         /*
>          * Only send non-zero copyup data to save some I/O and network
>          * bandwidth -- zero copyup data is equivalent to the object not
> @@ -2372,8 +2378,6 @@ static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
>                 bytes = 0;
>         }
>
> -       osd_req_op_cls_init(obj_req->osd_req, 0, CEPH_OSD_OP_CALL, "rbd",
> -                           "copyup");
>         osd_req_op_cls_request_data_bvecs(obj_req->osd_req, 0,
>                                           obj_req->copyup_bvecs, bytes);
>
> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> index 528ccc9..ba3d8c2 100644
> --- a/include/linux/ceph/osd_client.h
> +++ b/include/linux/ceph/osd_client.h
> @@ -432,7 +432,7 @@ extern void osd_req_op_cls_response_data_pages(struct ceph_osd_request *,
>                                         struct page **pages, u64 length,
>                                         u32 alignment, bool pages_from_pool,
>                                         bool own_pages);
> -extern void osd_req_op_cls_init(struct ceph_osd_request *osd_req,
> +extern int osd_req_op_cls_init(struct ceph_osd_request *osd_req,
>                                         unsigned int which, u16 opcode,
>                                         const char *class, const char *method);
>  extern int osd_req_op_xattr_init(struct ceph_osd_request *osd_req, unsigned int which,
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index ea2a6c9f..a2e2c36 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -748,7 +748,7 @@ void osd_req_op_extent_dup_last(struct ceph_osd_request *osd_req,
>  }
>  EXPORT_SYMBOL(osd_req_op_extent_dup_last);
>
> -void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
> +int osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
>                         u16 opcode, const char *class, const char *method)
>  {
>         struct ceph_osd_req_op *op = _osd_req_op_init(osd_req, which,
> @@ -760,7 +760,9 @@ void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
>         BUG_ON(opcode != CEPH_OSD_OP_CALL);
>
>         pagelist = kmalloc(sizeof (*pagelist), GFP_NOFS);
> -       BUG_ON(!pagelist);
> +       if (!pagelist)
> +               return -ENOMEM;
> +
>         ceph_pagelist_init(pagelist);
>
>         op->cls.class_name = class;
> @@ -780,6 +782,7 @@ void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
>         osd_req_op_cls_request_info_pagelist(osd_req, which, pagelist);
>
>         op->indata_len = payload_len;
> +       return 0;
>  }
>  EXPORT_SYMBOL(osd_req_op_cls_init);
>
> @@ -4909,7 +4912,10 @@ int ceph_osdc_call(struct ceph_osd_client *osdc,
>         if (ret)
>                 goto out_put_req;
>
> -       osd_req_op_cls_init(req, 0, CEPH_OSD_OP_CALL, class, method);
> +       ret = osd_req_op_cls_init(req, 0, CEPH_OSD_OP_CALL, class, method);
> +       if (ret)
> +               goto out_put_req;
> +
>         if (req_page)
>                 osd_req_op_cls_request_data_pages(req, 0, &req_page, req_len,
>                                                   0, false, false);

Applied.

Thanks,

                Ilya
--
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 mbox

Patch

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 7320466..ebd1d79 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2348,6 +2348,7 @@  static bool is_zero_bvecs(struct bio_vec *bvecs, u32 bytes)
 static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
 {
 	unsigned int num_osd_ops = obj_req->osd_req->r_num_ops;
+	int err;
 
 	dout("%s obj_req %p bytes %u\n", __func__, obj_req, bytes);
 	rbd_assert(obj_req->osd_req->r_ops[0].op == CEPH_OSD_OP_STAT);
@@ -2362,6 +2363,11 @@  static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
 	if (!obj_req->osd_req)
 		return -ENOMEM;
 
+	err = osd_req_op_cls_init(obj_req->osd_req, 0, CEPH_OSD_OP_CALL, "rbd",
+			    "copyup");
+	if (err)
+		return err;
+
 	/*
 	 * Only send non-zero copyup data to save some I/O and network
 	 * bandwidth -- zero copyup data is equivalent to the object not
@@ -2372,8 +2378,6 @@  static int rbd_obj_issue_copyup(struct rbd_obj_request *obj_req, u32 bytes)
 		bytes = 0;
 	}
 
-	osd_req_op_cls_init(obj_req->osd_req, 0, CEPH_OSD_OP_CALL, "rbd",
-			    "copyup");
 	osd_req_op_cls_request_data_bvecs(obj_req->osd_req, 0,
 					  obj_req->copyup_bvecs, bytes);
 
diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
index 528ccc9..ba3d8c2 100644
--- a/include/linux/ceph/osd_client.h
+++ b/include/linux/ceph/osd_client.h
@@ -432,7 +432,7 @@  extern void osd_req_op_cls_response_data_pages(struct ceph_osd_request *,
 					struct page **pages, u64 length,
 					u32 alignment, bool pages_from_pool,
 					bool own_pages);
-extern void osd_req_op_cls_init(struct ceph_osd_request *osd_req,
+extern int osd_req_op_cls_init(struct ceph_osd_request *osd_req,
 					unsigned int which, u16 opcode,
 					const char *class, const char *method);
 extern int osd_req_op_xattr_init(struct ceph_osd_request *osd_req, unsigned int which,
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index ea2a6c9f..a2e2c36 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -748,7 +748,7 @@  void osd_req_op_extent_dup_last(struct ceph_osd_request *osd_req,
 }
 EXPORT_SYMBOL(osd_req_op_extent_dup_last);
 
-void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
+int osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
 			u16 opcode, const char *class, const char *method)
 {
 	struct ceph_osd_req_op *op = _osd_req_op_init(osd_req, which,
@@ -760,7 +760,9 @@  void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
 	BUG_ON(opcode != CEPH_OSD_OP_CALL);
 
 	pagelist = kmalloc(sizeof (*pagelist), GFP_NOFS);
-	BUG_ON(!pagelist);
+	if (!pagelist)
+		return -ENOMEM;
+
 	ceph_pagelist_init(pagelist);
 
 	op->cls.class_name = class;
@@ -780,6 +782,7 @@  void osd_req_op_cls_init(struct ceph_osd_request *osd_req, unsigned int which,
 	osd_req_op_cls_request_info_pagelist(osd_req, which, pagelist);
 
 	op->indata_len = payload_len;
+	return 0;
 }
 EXPORT_SYMBOL(osd_req_op_cls_init);
 
@@ -4909,7 +4912,10 @@  int ceph_osdc_call(struct ceph_osd_client *osdc,
 	if (ret)
 		goto out_put_req;
 
-	osd_req_op_cls_init(req, 0, CEPH_OSD_OP_CALL, class, method);
+	ret = osd_req_op_cls_init(req, 0, CEPH_OSD_OP_CALL, class, method);
+	if (ret)
+		goto out_put_req;
+
 	if (req_page)
 		osd_req_op_cls_request_data_pages(req, 0, &req_page, req_len,
 						  0, false, false);