Whamcloud - gitweb
LU-17929 ptlrpc: ptlrpc_request_alloc_pack() always returns an error code 91/55391/6
authorAurelien Degremont <adegremont@nvidia.com>
Tue, 11 Jun 2024 08:33:11 +0000 (10:33 +0200)
committerOleg Drokin <green@whamcloud.com>
Tue, 25 Jun 2024 03:31:38 +0000 (03:31 +0000)
Current code was always considering that when this function
returns NULL it meant ENOMEM error, but this is not always
true, especially when using GSS by example, or when
reconnecting from an IDLE state.
Also, instead of having every caller converting NULL to
ENOMEM, do that directly in the function when
appropriate.

Make ptlrpc_request_alloc_pack() return -errno in case
of error instead of a NULL pointer.

Thanks to that change, error code will be propagated up
and will help error reporting and debugging.

Took the opportunity to simplify related error path
for 2 HSM functions.

Also changed param.status to a signed data, as it can
store -errno.

Signed-off-by: Aurelien Degremont <adegremont@nvidia.com>
Change-Id: Id2b873d5f0c5cb89db070f6db00269545e6c85e8
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55391
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
14 files changed:
lustre/fid/fid_request.c
lustre/fld/fld_request.c
lustre/include/uapi/linux/lustre/lgss.h
lustre/ldlm/ldlm_lockd.c
lustre/ldlm/ldlm_request.c
lustre/mdc/mdc_request.c
lustre/mgc/mgc_request.c
lustre/osc/osc_quota.c
lustre/ptlrpc/client.c
lustre/ptlrpc/gss/gss_cli_upcall.c
lustre/ptlrpc/import.c
lustre/ptlrpc/llog_client.c
lustre/ptlrpc/lproc_ptlrpc.c
lustre/ptlrpc/pinger.c

index 8c94d04..d3bc94a 100644 (file)
@@ -65,8 +65,8 @@ static int seq_client_rpc(struct lu_client_seq *seq,
        LASSERT(exp != NULL && !IS_ERR(exp));
        req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp), &RQF_SEQ_QUERY,
                                        LUSTRE_MDS_VERSION, SEQ_QUERY);
-       if (!req)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        /* Init operation code */
        op = req_capsule_client_get(&req->rq_pill, &RMF_SEQ_OPC);
index ebe1100..19829a7 100644 (file)
@@ -340,8 +340,8 @@ again:
        case FLD_QUERY:
                req = ptlrpc_request_alloc_pack(imp, &RQF_FLD_QUERY,
                                                LUSTRE_MDS_VERSION, FLD_QUERY);
-               if (!req)
-                       RETURN(-ENOMEM);
+               if (IS_ERR(req))
+                       RETURN(PTR_ERR(req));
 
                /*
                 * XXX: only needed when talking to old server(< 2.6), it should
@@ -364,8 +364,8 @@ again:
        case FLD_READ:
                req = ptlrpc_request_alloc_pack(imp, &RQF_FLD_READ,
                                                LUSTRE_MDS_VERSION, FLD_READ);
-               if (!req)
-                       RETURN(-ENOMEM);
+               if (IS_ERR(req))
+                       RETURN(PTR_ERR(req));
 
                req_capsule_set_size(&req->rq_pill, &RMF_GENERIC_DATA,
                                     RCL_SERVER, PAGE_SIZE);
index d353fba..618e5a4 100644 (file)
@@ -67,7 +67,7 @@ struct lgssd_ioctl_param {
        __u64 reply_buf_size;
        char __user *reply_buf;
        /* out */
-       __u64 status;
+       __s64 status;
        __u64 reply_length;
 };
 
index 231da36..d0a4aff 100644 (file)
@@ -971,8 +971,8 @@ int ldlm_server_blocking_ast(struct ldlm_lock *lock,
        req = ptlrpc_request_alloc_pack(lock->l_export->exp_imp_reverse,
                                        &RQF_LDLM_BL_CALLBACK,
                                        LUSTRE_DLM_VERSION, LDLM_BL_CALLBACK);
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        ca = ptlrpc_req_async_args(ca, req);
        ca->ca_set_arg = arg;
@@ -1215,8 +1215,8 @@ int ldlm_server_glimpse_ast(struct ldlm_lock *lock, void *data)
                                        req_fmt, LUSTRE_DLM_VERSION,
                                        LDLM_GL_CALLBACK);
 
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        if (arg->gl_desc != NULL) {
                /* copy the GL descriptor */
index 3d37218..5f7d5ee 100644 (file)
@@ -1245,8 +1245,8 @@ int ldlm_cli_convert_req(struct ldlm_lock *lock, __u32 *flags, __u64 new_bits)
        req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
                                        &RQF_LDLM_CONVERT, LUSTRE_DLM_VERSION,
                                        LDLM_CONVERT);
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        body = req_capsule_client_get(&req->rq_pill, &RMF_DLM_REQ);
        body->lock_handle[0] = lock->l_remote_handle;
@@ -2573,8 +2573,8 @@ static int replay_one_lock(struct obd_import *imp, struct ldlm_lock *lock)
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_LDLM_ENQUEUE,
                                        LUSTRE_DLM_VERSION, LDLM_ENQUEUE);
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        /* We're part of recovery, so don't wait for it. */
        req->rq_send_state = LUSTRE_IMP_REPLAY_LOCKS;
index a6bc5f0..a0ae7ec 100644 (file)
@@ -1602,8 +1602,8 @@ static int mdc_statfs_async(struct obd_export *exp,
 
        req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp), &RQF_MDS_STATFS,
                                        LUSTRE_MDS_VERSION, MDS_STATFS);
-       if (req == NULL)
-               return -ENOMEM;
+       if (IS_ERR(req))
+               return PTR_ERR(req);
 
        ptlrpc_request_set_replen(req);
        req->rq_interpret_reply = mdc_statfs_interpret;
@@ -1643,8 +1643,8 @@ static int mdc_statfs(const struct lu_env *env,
                fmt = &RQF_MDS_STATFS_NEW;
        req = ptlrpc_request_alloc_pack(imp, fmt, LUSTRE_MDS_VERSION,
                                        MDS_STATFS);
-       if (req == NULL)
-               GOTO(output, rc = -ENOMEM);
+       if (IS_ERR(req))
+               GOTO(output, rc = PTR_ERR(req));
        req->rq_allow_intr = 1;
 
        if ((flags & OBD_STATFS_SUM) &&
@@ -1745,8 +1745,8 @@ static int mdc_ioc_hsm_progress(struct obd_export *exp,
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_MDS_HSM_PROGRESS,
                                        LUSTRE_MDS_VERSION, MDS_HSM_PROGRESS);
-       if (req == NULL)
-               GOTO(out, rc = -ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        mdc_pack_body(&req->rq_pill, NULL, 0, 0, -1, 0);
 
@@ -1878,18 +1878,18 @@ static int mdc_ioc_hsm_ct_unregister(struct obd_import *imp)
        req = ptlrpc_request_alloc_pack(imp, &RQF_MDS_HSM_CT_UNREGISTER,
                                        LUSTRE_MDS_VERSION,
                                        MDS_HSM_CT_UNREGISTER);
-       if (req == NULL)
-               GOTO(out, rc = -ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        mdc_pack_body(&req->rq_pill, NULL, 0, 0, -1, 0);
 
        ptlrpc_request_set_replen(req);
 
        rc = mdc_queue_wait(req);
-       GOTO(out, rc);
-out:
+
        ptlrpc_req_put(req);
-       return rc;
+
+       RETURN(rc);
 }
 
 static int mdc_ioc_hsm_state_get(struct obd_export *exp,
index 902a514..8e34cc5 100644 (file)
@@ -976,8 +976,8 @@ static int mgc_enqueue(struct obd_export *exp, enum ldlm_type type,
        req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
                                        &RQF_LDLM_ENQUEUE, LUSTRE_DLM_VERSION,
                                        LDLM_ENQUEUE);
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        req_capsule_set_size(&req->rq_pill, &RMF_DLM_LVB, RCL_SERVER, 0);
        ptlrpc_request_set_replen(req);
index 0acf616..6a94b18 100644 (file)
@@ -215,8 +215,8 @@ int osc_quotactl(struct obd_device *unused, struct obd_export *exp,
        req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
                                        &RQF_OST_QUOTACTL, LUSTRE_OST_VERSION,
                                        OST_QUOTACTL);
-       if (req == NULL)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        if (oqctl->qc_cmd == LUSTRE_Q_ITEROQUOTA)
                req_capsule_set_size(&req->rq_pill, &RMF_OBD_QUOTA_ITER,
index 5ee870a..7efc593 100644 (file)
@@ -1053,26 +1053,30 @@ void ptlrpc_request_free(struct ptlrpc_request *request)
 EXPORT_SYMBOL(ptlrpc_request_free);
 
 /**
- * Allocate new request for operatione \a opcode and immediatelly pack it for
+ * Allocate new request for operation \a opcode and immediatelly pack it for
  * network transfer.
  * Only used for simple requests like OBD_PING where the only important
  * part of the request is operation itself.
- * Returns allocated request or NULL on error.
+ *
+ * Returns allocated request on success, and -errno on failure.
  */
 struct ptlrpc_request *ptlrpc_request_alloc_pack(struct obd_import *imp,
                                                 const struct req_format *format,
                                                 __u32 version, int opcode)
 {
-       struct ptlrpc_request *req = ptlrpc_request_alloc(imp, format);
+       struct ptlrpc_request *req;
        int rc;
 
-       if (req) {
-               rc = ptlrpc_request_pack(req, version, opcode);
-               if (rc) {
-                       ptlrpc_request_free(req);
-                       req = NULL;
-               }
+       req = ptlrpc_request_alloc(imp, format);
+       if (!req)
+               return ERR_PTR(-ENOMEM);
+
+       rc = ptlrpc_request_pack(req, version, opcode);
+       if (rc) {
+               ptlrpc_request_free(req);
+               return ERR_PTR(rc);
        }
+
        return req;
 }
 EXPORT_SYMBOL(ptlrpc_request_alloc_pack);
index 09bf063..7ada030 100644 (file)
@@ -316,7 +316,11 @@ int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count)
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_SEC_CTX, LUSTRE_OBD_VERSION,
                                        SEC_CTX_INIT);
-       if (!req || !req->rq_cli_ctx || !req->rq_cli_ctx->cc_sec) {
+       if (IS_ERR(req)) {
+               param.status = PTR_ERR(req);
+               req = NULL;
+               goto out_copy;
+       } else if (!req->rq_cli_ctx || !req->rq_cli_ctx->cc_sec) {
                param.status = -ENOMEM;
                goto out_copy;
        }
@@ -358,7 +362,7 @@ int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count)
                if (rc != -EACCES)
                        param.status = -ETIMEDOUT;
                CDEBUG(D_SEC,
-                      "%s: ctx init req got %d, returning to userspace status %llu\n",
+                      "%s: ctx init req got %d, returning to userspace status %lld\n",
                       obd->obd_name, rc, param.status);
                goto out_copy;
        }
index f8146d8..edc6791 100644 (file)
@@ -1515,9 +1515,9 @@ static int signal_completed_replay(struct obd_import *imp)
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_OBD_PING, LUSTRE_OBD_VERSION,
                                        OBD_PING);
-       if (req == NULL) {
+       if (IS_ERR(req)) {
                atomic_dec(&imp->imp_replay_inflight);
-               RETURN(-ENOMEM);
+               RETURN(PTR_ERR(req));
        }
 
        ptlrpc_request_set_replen(req);
@@ -1718,8 +1718,8 @@ static struct ptlrpc_request *ptlrpc_disconnect_prep_req(struct obd_import *imp)
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_MDS_DISCONNECT,
                                        LUSTRE_OBD_VERSION, rq_opc);
-       if (req == NULL)
-               RETURN(ERR_PTR(-ENOMEM));
+       if (IS_ERR(req))
+               RETURN(ERR_CAST(req));
 
        /* We are disconnecting, do not retry a failed DISCONNECT rpc if
         * it fails.  We can get through the above with a down server
index 349bc57..474729f 100644 (file)
@@ -167,8 +167,8 @@ static int llog_client_next_block(const struct lu_env *env,
        req = ptlrpc_request_alloc_pack(imp, &RQF_LLOG_ORIGIN_HANDLE_NEXT_BLOCK,
                                        LUSTRE_LOG_VERSION,
                                        LLOG_ORIGIN_HANDLE_NEXT_BLOCK);
-       if (!req)
-               GOTO(err_exit, rc = -ENOMEM);
+       if (IS_ERR(req))
+               GOTO(err_exit, rc = PTR_ERR(req));
 
        body = req_capsule_client_get(&req->rq_pill, &RMF_LLOGD_BODY);
        body->lgd_logid = loghandle->lgh_id;
@@ -242,8 +242,8 @@ static int llog_client_prev_block(const struct lu_env *env,
        req = ptlrpc_request_alloc_pack(imp, &RQF_LLOG_ORIGIN_HANDLE_PREV_BLOCK,
                                        LUSTRE_LOG_VERSION,
                                        LLOG_ORIGIN_HANDLE_PREV_BLOCK);
-       if (!req)
-               GOTO(err_exit, rc = -ENOMEM);
+       if (IS_ERR(req))
+               GOTO(err_exit, rc = PTR_ERR(req));
 
        body = req_capsule_client_get(&req->rq_pill, &RMF_LLOGD_BODY);
        body->lgd_logid = loghandle->lgh_id;
@@ -296,8 +296,8 @@ static int llog_client_read_header(const struct lu_env *env,
                                        &RQF_LLOG_ORIGIN_HANDLE_READ_HEADER,
                                        LUSTRE_LOG_VERSION,
                                        LLOG_ORIGIN_HANDLE_READ_HEADER);
-       if (!req)
-               GOTO(err_exit, rc = -ENOMEM);
+       if (IS_ERR(req))
+               GOTO(err_exit, rc = PTR_ERR(req));
 
        body = req_capsule_client_get(&req->rq_pill, &RMF_LLOGD_BODY);
        body->lgd_logid = handle->lgh_id;
index ae29ac3..e0a7d1d 100644 (file)
@@ -1344,8 +1344,8 @@ ssize_t ping_show(struct kobject *kobj, struct attribute *attr,
 
        if (rc)
                RETURN(rc);
-       if (!req)
-               RETURN(-ENOMEM);
+       if (IS_ERR(req))
+               RETURN(PTR_ERR(req));
 
        req->rq_send_state = LUSTRE_IMP_FULL;
 
index 431b9a1..dfcb17b 100644 (file)
@@ -63,10 +63,12 @@ ptlrpc_prep_ping(struct obd_import *imp)
 
        req = ptlrpc_request_alloc_pack(imp, &RQF_OBD_PING,
                                        LUSTRE_OBD_VERSION, OBD_PING);
-       if (req) {
-               ptlrpc_request_set_replen(req);
-               req->rq_no_resend = req->rq_no_delay = 1;
-       }
+       if (IS_ERR(req))
+               return ERR_CAST(req);
+
+       ptlrpc_request_set_replen(req);
+       req->rq_no_resend = req->rq_no_delay = 1;
+
        return req;
 }
 
@@ -80,8 +82,8 @@ int ptlrpc_obd_ping(struct obd_device *obd)
 
        with_imp_locked(obd, imp, rc) {
                req = ptlrpc_prep_ping(imp);
-               if (!req) {
-                       rc = -ENOMEM;
+               if (IS_ERR(req)) {
+                       rc = PTR_ERR(req);
                        continue;
                }
                req->rq_send_state = LUSTRE_IMP_FULL;
@@ -141,11 +143,10 @@ static int ptlrpc_ping(struct obd_import *imp)
                        RETURN(0);
 
        req = ptlrpc_prep_ping(imp);
-       if (!req) {
-               CERROR("OOM trying to ping %s->%s\n",
-                      imp->imp_obd->obd_uuid.uuid,
-                      obd2cli_tgt(imp->imp_obd));
-               RETURN(-ENOMEM);
+       if (IS_ERR(req)) {
+               CERROR("%s: ping failed: rc = %ld\n",
+                      imp->imp_obd->obd_name, PTR_ERR(req));
+               RETURN(PTR_ERR(req));
        }
 
        DEBUG_REQ(D_INFO, req, "pinging %s->%s",