On Wed, Jul 1, 2020 at 5:54 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> Turn the request_init helper into a more full-featured initialization
> routine that we can use to initialize an already-allocated request.
> Make it a public and exported function so we can use it from ceph.ko.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> include/linux/ceph/osd_client.h | 4 ++++
> net/ceph/osd_client.c | 28 +++++++++++++++-------------
> 2 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> index 8d63dc22cb36..40a08c4e5d8d 100644
> --- a/include/linux/ceph/osd_client.h
> +++ b/include/linux/ceph/osd_client.h
> @@ -495,6 +495,10 @@ extern struct ceph_osd_request *ceph_osdc_new_request(struct ceph_osd_client *,
>
> extern void ceph_osdc_get_request(struct ceph_osd_request *req);
> extern void ceph_osdc_put_request(struct ceph_osd_request *req);
> +void ceph_osdc_init_request(struct ceph_osd_request *req,
> + struct ceph_osd_client *osdc,
> + struct ceph_snap_context *snapc,
> + unsigned int num_ops);
>
> extern int ceph_osdc_start_request(struct ceph_osd_client *osdc,
> struct ceph_osd_request *req,
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 3cff29d38b9f..4ddf23120b1a 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -523,7 +523,10 @@ void ceph_osdc_put_request(struct ceph_osd_request *req)
> }
> EXPORT_SYMBOL(ceph_osdc_put_request);
>
> -static void request_init(struct ceph_osd_request *req)
> +void ceph_osdc_init_request(struct ceph_osd_request *req,
> + struct ceph_osd_client *osdc,
> + struct ceph_snap_context *snapc,
> + unsigned int num_ops)
> {
> /* req only, each op is zeroed in osd_req_op_init() */
> memset(req, 0, sizeof(*req));
> @@ -535,7 +538,13 @@ static void request_init(struct ceph_osd_request *req)
> INIT_LIST_HEAD(&req->r_private_item);
>
> target_init(&req->r_t);
> +
> + req->r_osdc = osdc;
> + req->r_num_ops = num_ops;
> + req->r_snapid = CEPH_NOSNAP;
> + req->r_snapc = ceph_get_snap_context(snapc);
> }
> +EXPORT_SYMBOL(ceph_osdc_init_request);
>
> /*
> * This is ugly, but it allows us to reuse linger registration and ping
> @@ -563,12 +572,9 @@ static void request_reinit(struct ceph_osd_request *req)
> WARN_ON(kref_read(&reply_msg->kref) != 1);
> target_destroy(&req->r_t);
>
> - request_init(req);
> - req->r_osdc = osdc;
> + ceph_osdc_init_request(req, osdc, snapc, num_ops);
> req->r_mempool = mempool;
> - req->r_num_ops = num_ops;
> req->r_snapid = snapid;
> - req->r_snapc = snapc;
> req->r_linger = linger;
> req->r_request = request_msg;
> req->r_reply = reply_msg;
> @@ -591,15 +597,11 @@ struct ceph_osd_request *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
> BUG_ON(num_ops > CEPH_OSD_MAX_OPS);
> req = kmalloc(struct_size(req, r_ops, num_ops), gfp_flags);
> }
> - if (unlikely(!req))
> - return NULL;
>
> - request_init(req);
> - req->r_osdc = osdc;
> - req->r_mempool = use_mempool;
> - req->r_num_ops = num_ops;
> - req->r_snapid = CEPH_NOSNAP;
> - req->r_snapc = ceph_get_snap_context(snapc);
> + if (likely(req)) {
> + req->r_mempool = use_mempool;
> + ceph_osdc_init_request(req, osdc, snapc, num_ops);
> + }
>
> dout("%s req %p\n", __func__, req);
> return req;
What is going to use ceph_osdc_init_request()?
Given that OSD request allocation is non-trivial, exporting a
routine for initializing already allocated requests doesn't seem
like a good idea. How do you ensure that the OSD request to be
initialized has enough room for passed num_ops? What about calling
this routine on a request that has already been initialized?
None of these issues exist today...
Thanks,
Ilya
