From: NeilBrown Date: Wed, 11 Sep 2019 15:34:54 +0000 (-0400) Subject: LU-12542 handle: move refcount into the lustre_handle. X-Git-Tag: 2.13.51~158 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=refs%2Fchanges%2F94%2F35794%2F15;p=fs%2Flustre-release.git LU-12542 handle: move refcount into the lustre_handle. Most objects with a lustre_handle have a refcount. The exception is mdt_mfd which uses locking (med_open_lock) to manage its lifetime. The lustre_handles code currently needs a call-out to increment its refcount. To simplify things, move the refcount into the lustre_hanle (which will be largely ignored by mdt_mfd) and discard the call-out. To avoid warnings when refcount debugging is enabled the refcount of mdt_mfd is initialized to 1, and decremeneted after any class_handle2object() call which would have incremented it. In order to preserve the same debug messages, we store an object type name in the portals_handle_ops, and use that in a CDEBUG() when incrementing the ref count. Change-Id: I1920330b2aeffd4b865cb9b249997aa28b209c33 Signed-off-by: NeilBrown Reviewed-on: https://review.whamcloud.com/35794 Reviewed-by: Neil Brown Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Shaun Tancheff Reviewed-by: Oleg Drokin --- diff --git a/libcfs/autoconf/lustre-libcfs.m4 b/libcfs/autoconf/lustre-libcfs.m4 index f34af72..279be80 100644 --- a/libcfs/autoconf/lustre-libcfs.m4 +++ b/libcfs/autoconf/lustre-libcfs.m4 @@ -946,6 +946,16 @@ cpu_hotplug_state_machine, [ ]) # LIBCFS_HOTPLUG_STATE_MACHINE # +# Kernel version 4.10-rc3 commit f405df5de3170c00e5c54f8b7cf4766044a032ba +# introduced refcount_t which is atomic_t plus over flow guards. +# +AC_DEFUN([LIBCFS_REFCOUNT_T], [ +LB_CHECK_LINUX_HEADER([linux/refcount.h], [ + AC_DEFINE(HAVE_REFCOUNT_T, 1, + [refcount_t is supported])]) +]) # LIBCFS_REFCOUNT_T + +# # LIBCFS_SCHED_HEADERS # # 4.11 has broken up sched.h into more headers. @@ -1345,6 +1355,7 @@ LIBCFS_GET_USER_PAGES_GUP_FLAGS LIBCFS_RHASHTABLE_WALK_ENTER # 4.10 LIBCFS_HOTPLUG_STATE_MACHINE +LIBCFS_REFCOUNT_T # 4.11 LIBCFS_RHASHTABLE_LOOKUP_GET_INSERT_FAST LIBCFS_SCHED_HEADERS diff --git a/libcfs/include/libcfs/linux/Makefile.am b/libcfs/include/libcfs/linux/Makefile.am index 6942d9f..a3c480d 100644 --- a/libcfs/include/libcfs/linux/Makefile.am +++ b/libcfs/include/libcfs/linux/Makefile.am @@ -1,3 +1,3 @@ EXTRA_DIST = linux-misc.h linux-fs.h linux-mem.h linux-time.h linux-cpu.h \ linux-list.h linux-hash.h linux-uuid.h linux-crypto.h linux-wait.h \ - processor.h + linux-refcount.h processor.h diff --git a/libcfs/include/libcfs/linux/linux-refcount.h b/libcfs/include/libcfs/linux/linux-refcount.h new file mode 100644 index 0000000..61484c2 --- /dev/null +++ b/libcfs/include/libcfs/linux/linux-refcount.h @@ -0,0 +1,39 @@ +/* + * GPL HEADER START + * + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 only, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 for more details (a copy is included + * in the LICENSE file that accompanied this code). + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; If not, see + * http://www.gnu.org/licenses/gpl-2.0.html + * + * GPL HEADER END + */ +#ifndef __LIBCFS_LINUX_REFCOUNT_H__ +#define __LIBCFS_LINUX_REFCOUNT_H__ + +#include + +#ifndef HAVE_REFCOUNT_T + +#define refcount_t atomic_t + +#define refcount_set atomic_set +#define refcount_inc atomic_inc +#define refcount_dec atomic_dec +#define refcount_dec_and_test atomic_dec_and_test +#define refcount_read atomic_read + +#endif + +#endif /* __LIBCFS_LINUX_REFCOUNT_H__ */ diff --git a/lustre/include/lustre_dlm.h b/lustre/include/lustre_dlm.h index e0f1f9a..08d9093 100644 --- a/lustre/include/lustre_dlm.h +++ b/lustre/include/lustre_dlm.h @@ -733,12 +733,6 @@ struct ldlm_lock { */ struct portals_handle l_handle; /** - * Lock reference count. - * This is how many users have pointers to actual structure, so that - * we do not accidentally free lock structure that is in use. - */ - atomic_t l_refc; - /** * Internal spinlock protects l_resource. We should hold this lock * first before taking res_lock. */ diff --git a/lustre/include/lustre_export.h b/lustre/include/lustre_export.h index 5862cb8..05a068d 100644 --- a/lustre/include/lustre_export.h +++ b/lustre/include/lustre_export.h @@ -188,12 +188,11 @@ struct obd_export { * what export they are talking to. */ struct portals_handle exp_handle; - atomic_t exp_refcount; /** * Set of counters below is to track where export references are * kept. The exp_rpc_count is used for reconnect handling also, * the cb_count and locks_count are for debug purposes only for now. - * The sum of them should be less than exp_refcount by 3 + * The sum of them should be less than exp_handle.href by 3 */ atomic_t exp_rpc_count; /* RPC references */ atomic_t exp_cb_count; /* Commit callback references */ diff --git a/lustre/include/lustre_handles.h b/lustre/include/lustre_handles.h index bbbefad..c7f316d 100644 --- a/lustre/include/lustre_handles.h +++ b/lustre/include/lustre_handles.h @@ -39,12 +39,18 @@ */ #include +#ifdef HAVE_REFCOUNT_T +#include +#else +#include +#endif #include #include struct portals_handle_ops { - void (*hop_addref)(void *object); void (*hop_free)(void *object, int size); + /* hop_type is used for some debugging messages */ + char *hop_type; }; /* These handles are most easily used by having them appear at the very top of @@ -62,6 +68,7 @@ struct portals_handle { struct list_head h_link; __u64 h_cookie; const struct portals_handle_ops *h_ops; + refcount_t h_ref; /* newly added fields to handle the RCU issue. -jxiong */ struct rcu_head h_rcu; diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c index be812e7..2688d07 100644 --- a/lustre/ldlm/ldlm_lib.c +++ b/lustre/ldlm/ldlm_lib.c @@ -1261,7 +1261,7 @@ no_export: LCONSOLE_WARN("%s: Client %s (at %s) refused connection, still busy with %d references\n", target->obd_name, cluuid.uuid, libcfs_nid2str(req->rq_peer.nid), - atomic_read(&export->exp_refcount)); + refcount_read(&export->exp_handle.h_ref)); GOTO(out, rc = -EBUSY); } else if (lustre_msg_get_conn_cnt(req->rq_reqmsg) == 1 && rc != EALREADY) { diff --git a/lustre/ldlm/ldlm_lock.c b/lustre/ldlm/ldlm_lock.c index 4276c1e..66304f1 100644 --- a/lustre/ldlm/ldlm_lock.c +++ b/lustre/ldlm/ldlm_lock.c @@ -185,7 +185,7 @@ EXPORT_SYMBOL(ldlm_register_intent); */ struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock) { - atomic_inc(&lock->l_refc); + refcount_inc(&lock->l_handle.h_ref); return lock; } EXPORT_SYMBOL(ldlm_lock_get); @@ -200,8 +200,8 @@ void ldlm_lock_put(struct ldlm_lock *lock) ENTRY; LASSERT(lock->l_resource != LP_POISON); - LASSERT(atomic_read(&lock->l_refc) > 0); - if (atomic_dec_and_test(&lock->l_refc)) { + LASSERT(refcount_read(&lock->l_handle.h_ref) > 0); + if (refcount_dec_and_test(&lock->l_handle.h_ref)) { struct ldlm_resource *res; LDLM_DEBUG(lock, @@ -438,12 +438,6 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock) EXIT; } -/* this is called by portals_handle2object with the handle lock taken */ -static void lock_handle_addref(void *lock) -{ - LDLM_LOCK_GET((struct ldlm_lock *)lock); -} - static void lock_handle_free(void *lock, int size) { LASSERT(size == sizeof(struct ldlm_lock)); @@ -451,8 +445,8 @@ static void lock_handle_free(void *lock, int size) } static struct portals_handle_ops lock_handle_ops = { - .hop_addref = lock_handle_addref, .hop_free = lock_handle_free, + .hop_type = "ldlm", }; /** @@ -479,7 +473,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource) lock->l_resource = resource; lu_ref_add(&resource->lr_reference, "lock", lock); - atomic_set(&lock->l_refc, 2); + refcount_set(&lock->l_handle.h_ref, 2); INIT_LIST_HEAD(&lock->l_res_link); INIT_LIST_HEAD(&lock->l_lru); INIT_LIST_HEAD(&lock->l_pending_chain); @@ -2760,13 +2754,13 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, &vaf, lock, lock->l_handle.h_cookie, - atomic_read(&lock->l_refc), + refcount_read(&lock->l_handle.h_ref), lock->l_readers, lock->l_writers, ldlm_lockname[lock->l_granted_mode], ldlm_lockname[lock->l_req_mode], lock->l_flags, nid, lock->l_remote_handle.cookie, - exp ? atomic_read(&exp->exp_refcount) : -99, + exp ? refcount_read(&exp->exp_handle.h_ref) : -99, lock->l_pid, lock->l_callback_timeout, lock->l_lvb_type); va_end(args); @@ -2780,7 +2774,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, &vaf, ldlm_lock_to_ns_name(lock), lock, lock->l_handle.h_cookie, - atomic_read(&lock->l_refc), + refcount_read(&lock->l_handle.h_ref), lock->l_readers, lock->l_writers, ldlm_lockname[lock->l_granted_mode], ldlm_lockname[lock->l_req_mode], @@ -2792,7 +2786,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, lock->l_req_extent.start, lock->l_req_extent.end, lock->l_flags, nid, lock->l_remote_handle.cookie, - exp ? atomic_read(&exp->exp_refcount) : -99, + exp ? refcount_read(&exp->exp_handle.h_ref) : -99, lock->l_pid, lock->l_callback_timeout, lock->l_lvb_type); break; @@ -2803,7 +2797,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, &vaf, ldlm_lock_to_ns_name(lock), lock, lock->l_handle.h_cookie, - atomic_read(&lock->l_refc), + refcount_read(&lock->l_handle.h_ref), lock->l_readers, lock->l_writers, ldlm_lockname[lock->l_granted_mode], ldlm_lockname[lock->l_req_mode], @@ -2815,7 +2809,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, lock->l_policy_data.l_flock.end, lock->l_flags, nid, lock->l_remote_handle.cookie, - exp ? atomic_read(&exp->exp_refcount) : -99, + exp ? refcount_read(&exp->exp_handle.h_ref) : -99, lock->l_pid, lock->l_callback_timeout); break; @@ -2825,7 +2819,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, &vaf, ldlm_lock_to_ns_name(lock), lock, lock->l_handle.h_cookie, - atomic_read(&lock->l_refc), + refcount_read(&lock->l_handle.h_ref), lock->l_readers, lock->l_writers, ldlm_lockname[lock->l_granted_mode], ldlm_lockname[lock->l_req_mode], @@ -2836,7 +2830,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, ldlm_typename[resource->lr_type], lock->l_flags, nid, lock->l_remote_handle.cookie, - exp ? atomic_read(&exp->exp_refcount) : -99, + exp ? refcount_read(&exp->exp_handle.h_ref) : -99, lock->l_pid, lock->l_callback_timeout, lock->l_lvb_type); break; @@ -2847,7 +2841,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, &vaf, ldlm_lock_to_ns_name(lock), lock, lock->l_handle.h_cookie, - atomic_read(&lock->l_refc), + refcount_read(&lock->l_handle.h_ref), lock->l_readers, lock->l_writers, ldlm_lockname[lock->l_granted_mode], ldlm_lockname[lock->l_req_mode], @@ -2856,7 +2850,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock, ldlm_typename[resource->lr_type], lock->l_flags, nid, lock->l_remote_handle.cookie, - exp ? atomic_read(&exp->exp_refcount) : -99, + exp ? refcount_read(&exp->exp_handle.h_ref) : -99, lock->l_pid, lock->l_callback_timeout, lock->l_lvb_type); break; diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index b89abc9..c13fe07 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -44,14 +44,9 @@ #include "mdt_internal.h" #include -/* we do nothing because we do not have refcount now */ -static void mdt_mfd_get(void *mfdp) -{ -} - static const struct portals_handle_ops mfd_open_handle_ops = { - .hop_addref = mdt_mfd_get, .hop_free = NULL, + .hop_type = "mdt", }; /* Create a new mdt_file_data struct, initialize it, @@ -63,6 +58,7 @@ struct mdt_file_data *mdt_mfd_new(const struct mdt_export_data *med) OBD_ALLOC_PTR(mfd); if (mfd != NULL) { + refcount_set(&mfd->mfd_open_handle.h_ref, 1); INIT_LIST_HEAD_RCU(&mfd->mfd_open_handle.h_link); mfd->mfd_owner = med; INIT_LIST_HEAD(&mfd->mfd_list); @@ -87,6 +83,9 @@ struct mdt_file_data *mdt_open_handle2mfd(struct mdt_export_data *med, LASSERT(open_handle != NULL); mfd = class_handle2object(open_handle->cookie, &mfd_open_handle_ops); + if (mfd) + refcount_dec(&mfd->mfd_open_handle.h_ref); + /* during dw/setattr replay the mfd can be found by old handle */ if ((!mfd || mfd->mfd_owner != med) && is_replay_or_resent) { list_for_each_entry(mfd, &med->med_open_head, mfd_list) { @@ -103,6 +102,7 @@ struct mdt_file_data *mdt_open_handle2mfd(struct mdt_export_data *med, /* free mfd */ void mdt_mfd_free(struct mdt_file_data *mfd) { + LASSERT(refcount_read(&mfd->mfd_open_handle.h_ref) == 1); LASSERT(list_empty(&mfd->mfd_list)); OBD_FREE_RCU(mfd, sizeof *mfd, &mfd->mfd_open_handle); } diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index bb8bc83..bba84a9 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -958,7 +958,7 @@ static void class_export_destroy(struct obd_export *exp) struct obd_device *obd = exp->exp_obd; ENTRY; - LASSERT_ATOMIC_ZERO(&exp->exp_refcount); + LASSERT(refcount_read(&exp->exp_handle.h_ref) == 0); LASSERT(obd != NULL); CDEBUG(D_IOCTL, "destroying export %p/%s for %s\n", exp, @@ -982,21 +982,16 @@ static void class_export_destroy(struct obd_export *exp) EXIT; } -static void export_handle_addref(void *export) -{ - class_export_get(export); -} - static struct portals_handle_ops export_handle_ops = { - .hop_addref = export_handle_addref, .hop_free = NULL, + .hop_type = "export", }; struct obd_export *class_export_get(struct obd_export *exp) { - atomic_inc(&exp->exp_refcount); - CDEBUG(D_INFO, "GETting export %p : new refcount %d\n", exp, - atomic_read(&exp->exp_refcount)); + refcount_inc(&exp->exp_handle.h_ref); + CDEBUG(D_INFO, "GET export %p refcount=%d\n", exp, + refcount_read(&exp->exp_handle.h_ref)); return exp; } EXPORT_SYMBOL(class_export_get); @@ -1004,11 +999,12 @@ EXPORT_SYMBOL(class_export_get); void class_export_put(struct obd_export *exp) { LASSERT(exp != NULL); - LASSERT_ATOMIC_GT_LT(&exp->exp_refcount, 0, LI_POISON); + LASSERT(refcount_read(&exp->exp_handle.h_ref) > 0); + LASSERT(refcount_read(&exp->exp_handle.h_ref) < LI_POISON); CDEBUG(D_INFO, "PUTting export %p : new refcount %d\n", exp, - atomic_read(&exp->exp_refcount) - 1); + refcount_read(&exp->exp_handle.h_ref) - 1); - if (atomic_dec_and_test(&exp->exp_refcount)) { + if (refcount_dec_and_test(&exp->exp_handle.h_ref)) { struct obd_device *obd = exp->exp_obd; CDEBUG(D_IOCTL, "final put %p/%s\n", @@ -1063,7 +1059,7 @@ struct obd_export *__class_new_export(struct obd_device *obd, export->exp_lock_hash = NULL; export->exp_flock_hash = NULL; /* 2 = class_handle_hash + last */ - atomic_set(&export->exp_refcount, 2); + refcount_set(&export->exp_handle.h_ref, 2); atomic_set(&export->exp_rpc_count, 0); atomic_set(&export->exp_cb_count, 0); atomic_set(&export->exp_locks_count, 0); @@ -1786,7 +1782,8 @@ static void print_export_data(struct obd_export *exp, const char *status, CDEBUG(debug_level, "%s: %s %p %s %s %d (%d %d %d) %d %d %d %d: " "%p %s %llu stale:%d\n", exp->exp_obd->obd_name, status, exp, exp->exp_client_uuid.uuid, - obd_export_nid2str(exp), atomic_read(&exp->exp_refcount), + obd_export_nid2str(exp), + refcount_read(&exp->exp_handle.h_ref), atomic_read(&exp->exp_rpc_count), atomic_read(&exp->exp_cb_count), atomic_read(&exp->exp_locks_count), diff --git a/lustre/obdclass/lustre_handles.c b/lustre/obdclass/lustre_handles.c index f790f7d..75987f1 100644 --- a/lustre/obdclass/lustre_handles.c +++ b/lustre/obdclass/lustre_handles.c @@ -158,7 +158,10 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops) spin_lock(&h->h_lock); if (likely(h->h_in != 0)) { - h->h_ops->hop_addref(h); + refcount_inc(&h->h_ref); + CDEBUG(D_INFO, "GET %s %p refcount=%d\n", + h->h_ops->hop_type, h, + refcount_read(&h->h_ref)); retval = h; } spin_unlock(&h->h_lock); diff --git a/lustre/obdecho/echo_client.c b/lustre/obdecho/echo_client.c index 07ed6b7..4b430c5 100644 --- a/lustre/obdecho/echo_client.c +++ b/lustre/obdecho/echo_client.c @@ -3053,7 +3053,7 @@ static int echo_client_cleanup(struct obd_device *obddev) RETURN(-EBUSY); } - LASSERT(atomic_read(&ec->ec_exp->exp_refcount) > 0); + LASSERT(refcount_read(&ec->ec_exp->exp_handle.h_ref) > 0); rc = obd_disconnect(ec->ec_exp); if (rc != 0) CERROR("fail to disconnect device: %d\n", rc); diff --git a/lustre/ptlrpc/service.c b/lustre/ptlrpc/service.c index 5dd5e0c..bb2a274 100644 --- a/lustre/ptlrpc/service.c +++ b/lustre/ptlrpc/service.c @@ -2294,7 +2294,7 @@ static int ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, (request->rq_export ? (char *)request->rq_export->exp_client_uuid.uuid : "0"), (request->rq_export ? - atomic_read(&request->rq_export->exp_refcount) : -99), + refcount_read(&request->rq_export->exp_handle.h_ref) : -99), lustre_msg_get_status(request->rq_reqmsg), request->rq_xid, libcfs_id2str(request->rq_peer), lustre_msg_get_opc(request->rq_reqmsg), @@ -2334,7 +2334,7 @@ put_conn: (request->rq_export ? (char *)request->rq_export->exp_client_uuid.uuid : "0"), (request->rq_export ? - atomic_read(&request->rq_export->exp_refcount) : -99), + refcount_read(&request->rq_export->exp_handle.h_ref) : -99), lustre_msg_get_status(request->rq_reqmsg), request->rq_xid, libcfs_id2str(request->rq_peer),