From: Mr NeilBrown Date: Sat, 18 Jan 2020 13:41:38 +0000 (-0500) Subject: LU-12678 lnet: me: discard struct lnet_handle_me X-Git-Tag: 2.13.52~52 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=ceeeae4271fdda75a1a064774ea28aca75f347ed LU-12678 lnet: me: discard struct lnet_handle_me The Portals API uses a cookie 'handle' to identify an ME. This is appropriate for a user-space API for objects maintained by the kernel, but it brings no value when the API client and implementation are both in the kernel, as is the case with Lustre and LNet. Instead of using a 'handle', a pointer to the 'struct lnet_me' can be used. This object is not reference counted and is always freed correctly, so there can be no case where the cookie becomes invalid while it is still held - as can be seen by the fact that the return value from LNetMEUnlink() is never used except to assert that it is zero. So use 'struct lnet_me *' directly instead of having indirection through a 'struct lnet_handle_me'. Also: - change LNetMEUnlink() to return void as it cannot fail now. - have LNetMEAttach() return the pointer, using ERR_PTR() to return errors. - discard ln_me_containers and don't store the me there-in. - store an explicit 'cpt' in each me, we no longer store one implicitly via the cookie. Test-Parameters: trivial testlist=sanity-lnet Signed-off-by: Mr NeilBrown Change-Id: I4e73e3217a244d8d15da90a8ba80371d1fd5f61f Reviewed-on: https://review.whamcloud.com/36859 Reviewed-by: Serguei Smirnov Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- diff --git a/lnet/include/lnet/api.h b/lnet/include/lnet/api.h index 6ba7cac..f84016a 100644 --- a/lnet/include/lnet/api.h +++ b/lnet/include/lnet/api.h @@ -95,15 +95,15 @@ bool LNetIsPeerLocal(lnet_nid_t nid); * request. MEs can be dynamically inserted into a match list by LNetMEAttach() * and removed from its list by LNetMEUnlink(). * @{ */ -int LNetMEAttach(unsigned int portal, - struct lnet_process_id match_id_in, - __u64 match_bits_in, - __u64 ignore_bits_in, - enum lnet_unlink unlink_in, - enum lnet_ins_pos pos_in, - struct lnet_handle_me *handle_out); - -int LNetMEUnlink(struct lnet_handle_me current_in); +struct lnet_me * +LNetMEAttach(unsigned int portal, + struct lnet_process_id match_id_in, + __u64 match_bits_in, + __u64 ignore_bits_in, + enum lnet_unlink unlink_in, + enum lnet_ins_pos pos_in); + +void LNetMEUnlink(struct lnet_me *current_in); /** @} lnet_me */ /** \defgroup lnet_md Memory descriptors @@ -118,9 +118,9 @@ int LNetMEUnlink(struct lnet_handle_me current_in); * and LNetMDBind(); one operation to unlink and release the resources * associated with a MD: LNetMDUnlink(). * @{ */ -int LNetMDAttach(struct lnet_handle_me current_in, - struct lnet_md md_in, - enum lnet_unlink unlink_in, +int LNetMDAttach(struct lnet_me *current_in, + struct lnet_md md_in, + enum lnet_unlink unlink_in, struct lnet_handle_md *md_handle_out); int LNetMDBind(struct lnet_md md_in, diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h index 1f3c36e..62ccd52 100644 --- a/lnet/include/lnet/lib-lnet.h +++ b/lnet/include/lnet/lib-lnet.h @@ -371,28 +371,6 @@ lnet_wire_handle2md(struct lnet_handle_wire *wh) } static inline void -lnet_me2handle(struct lnet_handle_me *handle, struct lnet_me *me) -{ - handle->cookie = me->me_lh.lh_cookie; -} - -static inline struct lnet_me * -lnet_handle2me(struct lnet_handle_me *handle) -{ - /* ALWAYS called with resource lock held */ - struct lnet_libhandle *lh; - int cpt; - - cpt = lnet_cpt_of_cookie(handle->cookie); - lh = lnet_res_lh_lookup(the_lnet.ln_me_containers[cpt], - handle->cookie); - if (lh == NULL) - return NULL; - - return lh_entry(lh, struct lnet_me, me_lh); -} - -static inline void lnet_peer_net_addref_locked(struct lnet_peer_net *lpn) { atomic_inc(&lpn->lpn_refcount); diff --git a/lnet/include/lnet/lib-types.h b/lnet/include/lnet/lib-types.h index cff32f1..f41971c 100644 --- a/lnet/include/lnet/lib-types.h +++ b/lnet/include/lnet/lib-types.h @@ -195,7 +195,7 @@ struct lnet_eq { struct lnet_me { struct list_head me_list; - struct lnet_libhandle me_lh; + int me_cpt; struct lnet_process_id me_match_id; unsigned int me_portal; unsigned int me_pos; /* hash offset in mt_hash */ @@ -1007,8 +1007,6 @@ struct lnet { int ln_nportals; /* the vector of portals */ struct lnet_portal **ln_portals; - /* percpt ME containers */ - struct lnet_res_container **ln_me_containers; /* percpt MD container */ struct lnet_res_container **ln_md_containers; diff --git a/lnet/include/uapi/linux/lnet/lnet-types.h b/lnet/include/uapi/linux/lnet/lnet-types.h index c7e779a..8a48f49 100644 --- a/lnet/include/uapi/linux/lnet/lnet-types.h +++ b/lnet/include/uapi/linux/lnet/lnet-types.h @@ -372,10 +372,6 @@ static inline int LNetMDHandleIsInvalid(struct lnet_handle_md h) return (LNET_WIRE_HANDLE_COOKIE_NONE == h.cookie); } -struct lnet_handle_me { - __u64 cookie; -}; - /** * Global process ID. */ diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index 399acdb..485ef06 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -1183,14 +1183,6 @@ lnet_prepare(lnet_pid_t requested_pid) if (rc != 0) goto failed; - recs = lnet_res_containers_create(LNET_COOKIE_TYPE_ME); - if (recs == NULL) { - rc = -ENOMEM; - goto failed; - } - - the_lnet.ln_me_containers = recs; - recs = lnet_res_containers_create(LNET_COOKIE_TYPE_MD); if (recs == NULL) { rc = -ENOMEM; @@ -1252,11 +1244,6 @@ lnet_unprepare (void) the_lnet.ln_md_containers = NULL; } - if (the_lnet.ln_me_containers != NULL) { - lnet_res_containers_destroy(the_lnet.ln_me_containers); - the_lnet.ln_me_containers = NULL; - } - lnet_res_container_cleanup(&the_lnet.ln_eq_container); lnet_msg_containers_destroy(); @@ -1666,7 +1653,7 @@ lnet_ping_target_setup(struct lnet_ping_buffer **ppbuf, .nid = LNET_NID_ANY, .pid = LNET_PID_ANY }; - struct lnet_handle_me me_handle; + struct lnet_me *me; struct lnet_md md = { NULL }; int rc, rc2; @@ -1686,11 +1673,11 @@ lnet_ping_target_setup(struct lnet_ping_buffer **ppbuf, } /* Ping target ME/MD */ - rc = LNetMEAttach(LNET_RESERVED_PORTAL, id, + me = LNetMEAttach(LNET_RESERVED_PORTAL, id, LNET_PROTO_PING_MATCHBITS, 0, - LNET_UNLINK, LNET_INS_AFTER, - &me_handle); - if (rc != 0) { + LNET_UNLINK, LNET_INS_AFTER); + if (IS_ERR(me)) { + rc = PTR_ERR(me); CERROR("Can't create ping target ME: %d\n", rc); goto fail_decref_ping_buffer; } @@ -1705,7 +1692,7 @@ lnet_ping_target_setup(struct lnet_ping_buffer **ppbuf, md.eq_handle = the_lnet.ln_ping_target_eq; md.user_ptr = *ppbuf; - rc = LNetMDAttach(me_handle, md, LNET_RETAIN, ping_mdh); + rc = LNetMDAttach(me, md, LNET_RETAIN, ping_mdh); if (rc != 0) { CERROR("Can't attach ping target MD: %d\n", rc); goto fail_unlink_ping_me; @@ -1715,8 +1702,7 @@ lnet_ping_target_setup(struct lnet_ping_buffer **ppbuf, return 0; fail_unlink_ping_me: - rc2 = LNetMEUnlink(me_handle); - LASSERT(rc2 == 0); + LNetMEUnlink(me); fail_decref_ping_buffer: LASSERT(lnet_ping_buffer_numref(*ppbuf) == 1); lnet_ping_buffer_decref(*ppbuf); @@ -1848,7 +1834,7 @@ int lnet_push_target_resize(void) { struct lnet_process_id id = { LNET_NID_ANY, LNET_PID_ANY }; struct lnet_md md = { NULL }; - struct lnet_handle_me meh; + struct lnet_me *me; struct lnet_handle_md mdh; struct lnet_handle_md old_mdh; struct lnet_ping_buffer *pbuf; @@ -1867,11 +1853,12 @@ again: goto fail_return; } - rc = LNetMEAttach(LNET_RESERVED_PORTAL, id, + me = LNetMEAttach(LNET_RESERVED_PORTAL, id, LNET_PROTO_PING_MATCHBITS, 0, - LNET_UNLINK, LNET_INS_AFTER, - &meh); - if (rc) { + LNET_UNLINK, LNET_INS_AFTER); + + if (IS_ERR(me)) { + rc = PTR_ERR(me); CERROR("Can't create push target ME: %d\n", rc); goto fail_decref_pbuf; } @@ -1886,10 +1873,10 @@ again: md.user_ptr = pbuf; md.eq_handle = the_lnet.ln_push_target_eq; - rc = LNetMDAttach(meh, md, LNET_RETAIN, &mdh); + rc = LNetMDAttach(me, md, LNET_RETAIN, &mdh); if (rc) { CERROR("Can't attach push MD: %d\n", rc); - goto fail_unlink_meh; + goto fail_unlink_me; } lnet_ping_buffer_addref(pbuf); @@ -1912,8 +1899,8 @@ again: return 0; -fail_unlink_meh: - LNetMEUnlink(meh); +fail_unlink_me: + LNetMEUnlink(me); fail_decref_pbuf: lnet_ping_buffer_decref(pbuf); fail_return: diff --git a/lnet/lnet/lib-md.c b/lnet/lnet/lib-md.c index ced8a77..f13868b 100644 --- a/lnet/lnet/lib-md.c +++ b/lnet/lnet/lib-md.c @@ -347,7 +347,7 @@ lnet_md_validate(struct lnet_md *umd) /** * Create a memory descriptor and attach it to a ME * - * \param meh A handle for a ME to associate the new MD with. + * \param me An ME to associate the new MD with. * \param umd Provides initial values for the user-visible parts of a MD. * Other than its use for initialization, there is no linkage between this * structure and the MD maintained by the LNet. @@ -370,12 +370,11 @@ lnet_md_validate(struct lnet_md *umd) * a MD. */ int -LNetMDAttach(struct lnet_handle_me meh, struct lnet_md umd, +LNetMDAttach(struct lnet_me *me, struct lnet_md umd, enum lnet_unlink unlink, struct lnet_handle_md *handle) { LIST_HEAD(matches); LIST_HEAD(drops); - struct lnet_me *me; struct lnet_libmd *md; int cpt; int rc; @@ -398,14 +397,11 @@ LNetMDAttach(struct lnet_handle_me meh, struct lnet_md umd, if (rc != 0) goto out_free; - cpt = lnet_cpt_of_cookie(meh.cookie); + cpt = me->me_cpt; lnet_res_lock(cpt); - me = lnet_handle2me(&meh); - if (me == NULL) - rc = -ENOENT; - else if (me->me_md != NULL) + if (me->me_md) rc = -EBUSY; else rc = lnet_md_link(md, umd.eq_handle, cpt); diff --git a/lnet/lnet/lib-me.c b/lnet/lnet/lib-me.c index 578e353..1ec83db 100644 --- a/lnet/lnet/lib-me.c +++ b/lnet/lnet/lib-me.c @@ -60,20 +60,16 @@ * \param pos Indicates whether the new ME should be prepended or * appended to the match list. Allowed constants: LNET_INS_BEFORE, * LNET_INS_AFTER. - * \param handle On successful returns, a handle to the newly created ME - * object is saved here. This handle can be used later in LNetMEUnlink(), - * or LNetMDAttach() functions. * - * \retval 0 On success. - * \retval -EINVAL If \a portal is invalid. - * \retval -ENOMEM If new ME object cannot be allocated. + * \retval A handle to the newly created ME is returned on success + * \retval ERR_PTR(-EINVAL) If \a portal is invalid. + * \retval ERR_PTR(-ENOMEM) If new ME object cannot be allocated. */ -int +struct lnet_me * LNetMEAttach(unsigned int portal, struct lnet_process_id match_id, __u64 match_bits, __u64 ignore_bits, - enum lnet_unlink unlink, enum lnet_ins_pos pos, - struct lnet_handle_me *handle) + enum lnet_unlink unlink, enum lnet_ins_pos pos) { struct lnet_match_table *mtable; struct lnet_me *me; @@ -82,16 +78,16 @@ LNetMEAttach(unsigned int portal, LASSERT(the_lnet.ln_refcount > 0); if ((int)portal >= the_lnet.ln_nportals) - return -EINVAL; + return ERR_PTR(-EINVAL); mtable = lnet_mt_of_attach(portal, match_id, match_bits, ignore_bits, pos); if (mtable == NULL) /* can't match portal type */ - return -EPERM; + return ERR_PTR(-EPERM); me = lnet_me_alloc(); if (me == NULL) - return -ENOMEM; + return ERR_PTR(-ENOMEM); lnet_res_lock(mtable->mt_cpt); @@ -102,8 +98,8 @@ LNetMEAttach(unsigned int portal, me->me_unlink = unlink; me->me_md = NULL; - lnet_res_lh_initialize(the_lnet.ln_me_containers[mtable->mt_cpt], - &me->me_lh); + me->me_cpt = mtable->mt_cpt; + if (ignore_bits != 0) head = &mtable->mt_mhash[LNET_MT_HASH_IGNORE]; else @@ -115,10 +111,8 @@ LNetMEAttach(unsigned int portal, else list_add(&me->me_list, head); - lnet_me2handle(handle, me); - lnet_res_unlock(mtable->mt_cpt); - return 0; + return me; } EXPORT_SYMBOL(LNetMEAttach); @@ -136,25 +130,18 @@ EXPORT_SYMBOL(LNetMEAttach); * \retval -ENOENT If \a meh does not point to a valid ME. * \see LNetMDUnlink() for the discussion on delivering unlink event. */ -int -LNetMEUnlink(struct lnet_handle_me meh) +void +LNetMEUnlink(struct lnet_me *me) { - struct lnet_me *me; struct lnet_libmd *md; struct lnet_event ev; int cpt; LASSERT(the_lnet.ln_refcount > 0); - cpt = lnet_cpt_of_cookie(meh.cookie); + cpt = me->me_cpt; lnet_res_lock(cpt); - me = lnet_handle2me(&meh); - if (me == NULL) { - lnet_res_unlock(cpt); - return -ENOENT; - } - md = me->me_md; if (md != NULL) { md->md_flags |= LNET_MD_FLAG_ABORTED; @@ -167,7 +154,6 @@ LNetMEUnlink(struct lnet_handle_me meh) lnet_me_unlink(me); lnet_res_unlock(cpt); - return 0; } EXPORT_SYMBOL(LNetMEUnlink); @@ -185,7 +171,6 @@ lnet_me_unlink(struct lnet_me *me) lnet_md_unlink(md); } - lnet_res_lh_invalidate(&me->me_lh); lnet_me_free(me); } diff --git a/lnet/selftest/rpc.c b/lnet/selftest/rpc.c index 52a788c..6a2ba3a 100644 --- a/lnet/selftest/rpc.c +++ b/lnet/selftest/rpc.c @@ -357,11 +357,12 @@ srpc_post_passive_rdma(int portal, int local, __u64 matchbits, void *buf, { int rc; struct lnet_md md; - struct lnet_handle_me meh; + struct lnet_me *me; - rc = LNetMEAttach(portal, peer, matchbits, 0, LNET_UNLINK, - local ? LNET_INS_LOCAL : LNET_INS_AFTER, &meh); - if (rc != 0) { + me = LNetMEAttach(portal, peer, matchbits, 0, LNET_UNLINK, + local ? LNET_INS_LOCAL : LNET_INS_AFTER); + if (IS_ERR(me)) { + rc = PTR_ERR(me); CERROR("LNetMEAttach failed: %d\n", rc); LASSERT(rc == -ENOMEM); return -ENOMEM; @@ -374,13 +375,12 @@ srpc_post_passive_rdma(int portal, int local, __u64 matchbits, void *buf, md.options = options; md.eq_handle = srpc_data.rpc_lnet_eq; - rc = LNetMDAttach(meh, md, LNET_UNLINK, mdh); + rc = LNetMDAttach(me, md, LNET_UNLINK, mdh); if (rc != 0) { CERROR("LNetMDAttach failed: %d\n", rc); LASSERT(rc == -ENOMEM); - rc = LNetMEUnlink(meh); - LASSERT(rc == 0); + LNetMEUnlink(me); return -ENOMEM; } diff --git a/lustre/ptlrpc/niobuf.c b/lustre/ptlrpc/niobuf.c index 4720592..915e092 100644 --- a/lustre/ptlrpc/niobuf.c +++ b/lustre/ptlrpc/niobuf.c @@ -312,11 +312,10 @@ int ptlrpc_register_bulk(struct ptlrpc_request *req) struct ptlrpc_bulk_desc *desc = req->rq_bulk; struct lnet_process_id peer; int rc = 0; - int rc2; int posted_md; int total_md; __u64 mbits; - struct lnet_handle_me me_h; + struct lnet_me *me; struct lnet_md md; ENTRY; @@ -377,8 +376,9 @@ int ptlrpc_register_bulk(struct ptlrpc_request *req) OBD_FAIL_CHECK(OBD_FAIL_PTLRPC_BULK_ATTACH)) { rc = -ENOMEM; } else { - rc = LNetMEAttach(desc->bd_portal, peer, mbits, 0, - LNET_UNLINK, LNET_INS_AFTER, &me_h); + me = LNetMEAttach(desc->bd_portal, peer, mbits, 0, + LNET_UNLINK, LNET_INS_AFTER); + rc = PTR_ERR_OR_ZERO(me); } if (rc != 0) { CERROR("%s: LNetMEAttach failed x%llu/%d: rc = %d\n", @@ -388,14 +388,13 @@ int ptlrpc_register_bulk(struct ptlrpc_request *req) } /* About to let the network at it... */ - rc = LNetMDAttach(me_h, md, LNET_UNLINK, + rc = LNetMDAttach(me, md, LNET_UNLINK, &desc->bd_mds[posted_md]); if (rc != 0) { CERROR("%s: LNetMDAttach failed x%llu/%d: rc = %d\n", desc->bd_import->imp_obd->obd_name, mbits, posted_md, rc); - rc2 = LNetMEUnlink(me_h); - LASSERT(rc2 == 0); + LNetMEUnlink(me); break; } } @@ -687,11 +686,10 @@ int ptlrpc_error(struct ptlrpc_request *req) int ptl_send_rpc(struct ptlrpc_request *request, int noreply) { int rc; - int rc2; int mpflag = 0; struct lnet_handle_md bulk_cookie; struct ptlrpc_connection *connection; - struct lnet_handle_me reply_me_h; + struct lnet_me *reply_me = NULL; struct lnet_md reply_md; struct obd_import *imp = request->rq_import; struct obd_device *obd = imp->imp_obd; @@ -817,15 +815,16 @@ int ptl_send_rpc(struct ptlrpc_request *request, int noreply) request->rq_repmsg = NULL; } - rc = LNetMEAttach(request->rq_reply_portal,/*XXX FIXME bug 249*/ - connection->c_peer, request->rq_xid, 0, - LNET_UNLINK, LNET_INS_AFTER, &reply_me_h); - if (rc != 0) { - CERROR("LNetMEAttach failed: %d\n", rc); - LASSERT (rc == -ENOMEM); - GOTO(cleanup_bulk, rc = -ENOMEM); - } - } + reply_me = LNetMEAttach(request->rq_reply_portal, + connection->c_peer, request->rq_xid, 0, + LNET_UNLINK, LNET_INS_AFTER); + if (IS_ERR(reply_me)) { + rc = PTR_ERR(reply_me); + CERROR("LNetMEAttach failed: %d\n", rc); + LASSERT(rc == -ENOMEM); + GOTO(cleanup_bulk, rc = -ENOMEM); + } + } spin_lock(&request->rq_lock); /* We are responsible for unlinking the reply buffer */ @@ -856,11 +855,11 @@ int ptl_send_rpc(struct ptlrpc_request *request, int noreply) /* We must see the unlink callback to set rq_reply_unlinked, * so we can't auto-unlink */ - rc = LNetMDAttach(reply_me_h, reply_md, LNET_RETAIN, - &request->rq_reply_md_h); - if (rc != 0) { - CERROR("LNetMDAttach failed: %d\n", rc); - LASSERT (rc == -ENOMEM); + rc = LNetMDAttach(reply_me, reply_md, LNET_RETAIN, + &request->rq_reply_md_h); + if (rc != 0) { + CERROR("LNetMDAttach failed: %d\n", rc); + LASSERT(rc == -ENOMEM); spin_lock(&request->rq_lock); /* ...but the MD attach didn't succeed... */ request->rq_receiving_reply = 0; @@ -910,9 +909,9 @@ int ptl_send_rpc(struct ptlrpc_request *request, int noreply) cleanup_me: /* MEUnlink is safe; the PUT didn't even get off the ground, and * nobody apart from the PUT's target has the right nid+XID to - * access the reply buffer. */ - rc2 = LNetMEUnlink(reply_me_h); - LASSERT (rc2 == 0); + * access the reply buffer. + */ + LNetMEUnlink(reply_me); /* UNLINKED callback called synchronously */ LASSERT(!request->rq_receiving_reply); @@ -948,7 +947,7 @@ int ptlrpc_register_rqbd(struct ptlrpc_request_buffer_desc *rqbd) }; int rc; struct lnet_md md; - struct lnet_handle_me me_h; + struct lnet_me *me; CDEBUG(D_NET, "LNetMEAttach: portal %d\n", service->srv_req_portal); @@ -959,14 +958,14 @@ int ptlrpc_register_rqbd(struct ptlrpc_request_buffer_desc *rqbd) /* NB: CPT affinity service should use new LNet flag LNET_INS_LOCAL, * which means buffer can only be attached on local CPT, and LND * threads can find it by grabbing a local lock */ - rc = LNetMEAttach(service->srv_req_portal, + me = LNetMEAttach(service->srv_req_portal, match_id, 0, ~0, LNET_UNLINK, rqbd->rqbd_svcpt->scp_cpt >= 0 ? - LNET_INS_LOCAL : LNET_INS_AFTER, &me_h); - if (rc != 0) { - CERROR("LNetMEAttach failed: %d\n", rc); - return (-ENOMEM); - } + LNET_INS_LOCAL : LNET_INS_AFTER); + if (IS_ERR(me)) { + CERROR("LNetMEAttach failed: %ld\n", PTR_ERR(me)); + return -ENOMEM; + } LASSERT(rqbd->rqbd_refcount == 0); rqbd->rqbd_refcount = 1; @@ -979,15 +978,15 @@ int ptlrpc_register_rqbd(struct ptlrpc_request_buffer_desc *rqbd) md.user_ptr = &rqbd->rqbd_cbid; md.eq_handle = ptlrpc_eq_h; - rc = LNetMDAttach(me_h, md, LNET_UNLINK, &rqbd->rqbd_md_h); - if (rc == 0) - return (0); + rc = LNetMDAttach(me, md, LNET_UNLINK, &rqbd->rqbd_md_h); + if (rc == 0) + return 0; - CERROR("LNetMDAttach failed: %d; \n", rc); - LASSERT (rc == -ENOMEM); - rc = LNetMEUnlink (me_h); - LASSERT (rc == 0); - rqbd->rqbd_refcount = 0; + CERROR("LNetMDAttach failed: %d;\n", rc); + LASSERT(rc == -ENOMEM); + LNetMEUnlink(me); + LASSERT(rc == 0); + rqbd->rqbd_refcount = 0; - return (-ENOMEM); + return -ENOMEM; }