From 5cae09a2409dcd396a1ee7be1eca7d3bbf77365e Mon Sep 17 00:00:00 2001 From: Jinshan Xiong Date: Fri, 7 Dec 2012 10:01:53 -0800 Subject: [PATCH 1/1] LU-744 obdclass: revise stats for cl_object cache It turns out that atomic doesn't scale well for highly contented cl_page stats. Enable it only when CONFIG_DEBUG_PAGESTATE_TRACKING is turned on. Signed-off-by: Jinshan Xiong Change-Id: I0163f65557135759f100fddde70c7e3ee2e45aed Reviewed-on: http://review.whamcloud.com/4519 Tested-by: Hudson Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Bobi Jam --- lustre/autoconf/lustre-core.m4 | 2 +- lustre/include/cl_object.h | 29 +++++++++------ lustre/obdclass/cl_lock.c | 49 ++++++++++++++----------- lustre/obdclass/cl_object.c | 83 ++++++++++++++++++++++-------------------- lustre/obdclass/cl_page.c | 59 +++++++++++++++--------------- 5 files changed, 120 insertions(+), 102 deletions(-) diff --git a/lustre/autoconf/lustre-core.m4 b/lustre/autoconf/lustre-core.m4 index 44c72d9..ba2571d 100644 --- a/lustre/autoconf/lustre-core.m4 +++ b/lustre/autoconf/lustre-core.m4 @@ -2644,7 +2644,7 @@ AC_ARG_ENABLE([pgstate-track], AC_MSG_CHECKING([whether to enable page state tracking]) AC_MSG_RESULT([$enable_pgstat_track]) if test x$enable_pgstat_track = xyes ; then - AC_DEFINE([LUSTRE_PAGESTATE_TRACKING], 1, + AC_DEFINE([CONFIG_DEBUG_PAGESTATE_TRACKING], 1, [enable page state tracking code]) fi diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h index 9ba73c5..c8b4cb9 100644 --- a/lustre/include/cl_object.h +++ b/lustre/include/cl_object.h @@ -2548,22 +2548,29 @@ struct cl_req_slice { /* @} cl_req */ +enum cache_stats_item { + /** how many cache lookups were performed */ + CS_lookup = 0, + /** how many times cache lookup resulted in a hit */ + CS_hit, + /** how many entities are in the cache right now */ + CS_total, + /** how many entities in the cache are actively used (and cannot be + * evicted) right now */ + CS_busy, + /** how many entities were created at all */ + CS_create, + CS_NR +}; + +#define CS_NAMES { "lookup", "hit", "total", "busy", "create" } + /** * Stats for a generic cache (similar to inode, lu_object, etc. caches). */ struct cache_stats { const char *cs_name; - /** how many entities were created at all */ - cfs_atomic_t cs_created; - /** how many cache lookups were performed */ - cfs_atomic_t cs_lookup; - /** how many times cache lookup resulted in a hit */ - cfs_atomic_t cs_hit; - /** how many entities are in the cache right now */ - cfs_atomic_t cs_total; - /** how many entities in the cache are actively used (and cannot be - * evicted) right now */ - cfs_atomic_t cs_busy; + cfs_atomic_t cs_stats[CS_NR]; }; /** These are not exported so far */ diff --git a/lustre/obdclass/cl_lock.c b/lustre/obdclass/cl_lock.c index 5a9ee70..a78f4b1 100644 --- a/lustre/obdclass/cl_lock.c +++ b/lustre/obdclass/cl_lock.c @@ -65,6 +65,22 @@ static struct lu_kmem_descr cl_lock_caches[] = { } }; +#ifdef CONFIG_DEBUG_PAGESTATE_TRACKING +#define CS_LOCK_INC(o, item) \ + cfs_atomic_inc(&cl_object_site(o)->cs_locks.cs_stats[CS_##item]) +#define CS_LOCK_DEC(o, item) \ + cfs_atomic_dec(&cl_object_site(o)->cs_locks.cs_stats[CS_##item]) +#define CS_LOCKSTATE_INC(o, state) \ + cfs_atomic_inc(&cl_object_site(o)->cs_locks_state[state]) +#define CS_LOCKSTATE_DEC(o, state) \ + cfs_atomic_dec(&cl_object_site(o)->cs_locks_state[state]) +#else +#define CS_LOCK_INC(o, item) +#define CS_LOCK_DEC(o, item) +#define CS_LOCKSTATE_INC(o, state) +#define CS_LOCKSTATE_DEC(o, state) +#endif + /** * Basic lock invariant that is maintained at all times. Caller either has a * reference to \a lock, or somehow assures that \a lock cannot be freed. @@ -269,8 +285,8 @@ static void cl_lock_free(const struct lu_env *env, struct cl_lock *lock) cfs_list_del_init(lock->cll_layers.next); slice->cls_ops->clo_fini(env, slice); } - cfs_atomic_dec(&cl_object_site(obj)->cs_locks.cs_total); - cfs_atomic_dec(&cl_object_site(obj)->cs_locks_state[lock->cll_state]); + CS_LOCK_DEC(obj, total); + CS_LOCKSTATE_DEC(obj, lock->cll_state); lu_object_ref_del_at(&obj->co_lu, lock->cll_obj_ref, "cl_lock", lock); cl_object_put(env, obj); lu_ref_fini(&lock->cll_reference); @@ -308,7 +324,7 @@ void cl_lock_put(const struct lu_env *env, struct cl_lock *lock) LASSERT(cfs_list_empty(&lock->cll_linkage)); cl_lock_free(env, lock); } - cfs_atomic_dec(&site->cs_locks.cs_busy); + CS_LOCK_DEC(obj, busy); } EXIT; } @@ -342,12 +358,10 @@ EXPORT_SYMBOL(cl_lock_get); */ void cl_lock_get_trust(struct cl_lock *lock) { - struct cl_site *site = cl_object_site(lock->cll_descr.cld_obj); - CDEBUG(D_TRACE, "acquiring trusted reference: %d %p %lu\n", cfs_atomic_read(&lock->cll_ref), lock, RETIP); if (cfs_atomic_inc_return(&lock->cll_ref) == 1) - cfs_atomic_inc(&site->cs_locks.cs_busy); + CS_LOCK_INC(lock->cll_descr.cld_obj, busy); } EXPORT_SYMBOL(cl_lock_get_trust); @@ -373,7 +387,6 @@ static struct cl_lock *cl_lock_alloc(const struct lu_env *env, { struct cl_lock *lock; struct lu_object_header *head; - struct cl_site *site = cl_object_site(obj); ENTRY; OBD_SLAB_ALLOC_PTR_GFP(lock, cl_lock_kmem, CFS_ALLOC_IO); @@ -393,9 +406,9 @@ static struct cl_lock *cl_lock_alloc(const struct lu_env *env, lockdep_set_class(&lock->cll_guard, &cl_lock_guard_class); cfs_waitq_init(&lock->cll_wq); head = obj->co_lu.lo_header; - cfs_atomic_inc(&site->cs_locks_state[CLS_NEW]); - cfs_atomic_inc(&site->cs_locks.cs_total); - cfs_atomic_inc(&site->cs_locks.cs_created); + CS_LOCKSTATE_INC(obj, CLS_NEW); + CS_LOCK_INC(obj, total); + CS_LOCK_INC(obj, create); cl_lock_lockdep_init(lock); cfs_list_for_each_entry(obj, &head->loh_layers, co_lu.lo_linkage) { @@ -493,14 +506,12 @@ static struct cl_lock *cl_lock_lookup(const struct lu_env *env, { struct cl_lock *lock; struct cl_object_header *head; - struct cl_site *site; ENTRY; head = cl_object_header(obj); - site = cl_object_site(obj); LINVRNT_SPIN_LOCKED(&head->coh_lock_guard); - cfs_atomic_inc(&site->cs_locks.cs_lookup); + CS_LOCK_INC(obj, lookup); cfs_list_for_each_entry(lock, &head->coh_locks, cll_linkage) { int matched; @@ -514,7 +525,7 @@ static struct cl_lock *cl_lock_lookup(const struct lu_env *env, matched); if (matched) { cl_lock_get_trust(lock); - cfs_atomic_inc(&cl_object_site(obj)->cs_locks.cs_hit); + CS_LOCK_INC(obj, hit); RETURN(lock); } } @@ -538,13 +549,11 @@ static struct cl_lock *cl_lock_find(const struct lu_env *env, struct cl_object_header *head; struct cl_object *obj; struct cl_lock *lock; - struct cl_site *site; ENTRY; obj = need->cld_obj; head = cl_object_header(obj); - site = cl_object_site(obj); spin_lock(&head->coh_lock_guard); lock = cl_lock_lookup(env, obj, io, need); @@ -561,7 +570,7 @@ static struct cl_lock *cl_lock_find(const struct lu_env *env, cfs_list_add_tail(&lock->cll_linkage, &head->coh_locks); spin_unlock(&head->coh_lock_guard); - cfs_atomic_inc(&site->cs_locks.cs_busy); + CS_LOCK_INC(obj, busy); } else { spin_unlock(&head->coh_lock_guard); /* @@ -1036,8 +1045,6 @@ EXPORT_SYMBOL(cl_lock_signal); void cl_lock_state_set(const struct lu_env *env, struct cl_lock *lock, enum cl_lock_state state) { - struct cl_site *site = cl_object_site(lock->cll_descr.cld_obj); - ENTRY; LASSERT(lock->cll_state <= state || (lock->cll_state == CLS_CACHED && @@ -1048,8 +1055,8 @@ void cl_lock_state_set(const struct lu_env *env, struct cl_lock *lock, lock->cll_state == CLS_INTRANSIT); if (lock->cll_state != state) { - cfs_atomic_dec(&site->cs_locks_state[lock->cll_state]); - cfs_atomic_inc(&site->cs_locks_state[state]); + CS_LOCKSTATE_DEC(lock->cll_descr.cld_obj, lock->cll_state); + CS_LOCKSTATE_INC(lock->cll_descr.cld_obj, state); cl_lock_state_signal(env, lock, state); lock->cll_state = state; diff --git a/lustre/obdclass/cl_object.c b/lustre/obdclass/cl_object.c index 248ccce..60dd33c 100644 --- a/lustre/obdclass/cl_object.c +++ b/lustre/obdclass/cl_object.c @@ -389,34 +389,37 @@ EXPORT_SYMBOL(cl_object_has_locks); void cache_stats_init(struct cache_stats *cs, const char *name) { + int i; + cs->cs_name = name; - cfs_atomic_set(&cs->cs_lookup, 0); - cfs_atomic_set(&cs->cs_hit, 0); - cfs_atomic_set(&cs->cs_total, 0); - cfs_atomic_set(&cs->cs_busy, 0); + for (i = 0; i < CS_NR; i++) + cfs_atomic_set(&cs->cs_stats[i], 0); } int cache_stats_print(const struct cache_stats *cs, char *page, int count, int h) { - int nob = 0; -/* - lookup hit total cached create - env: ...... ...... ...... ...... ...... -*/ - if (h) - nob += snprintf(page, count, - " lookup hit total busy create\n"); - - nob += snprintf(page + nob, count - nob, - "%5.5s: %6u %6u %6u %6u %6u", - cs->cs_name, - cfs_atomic_read(&cs->cs_lookup), - cfs_atomic_read(&cs->cs_hit), - cfs_atomic_read(&cs->cs_total), - cfs_atomic_read(&cs->cs_busy), - cfs_atomic_read(&cs->cs_created)); - return nob; + int nob = 0; + int i; + /* + * lookup hit total cached create + * env: ...... ...... ...... ...... ...... + */ + if (h) { + const char *names[CS_NR] = CS_NAMES; + + nob += snprintf(page + nob, count - nob, "%6s", " "); + for (i = 0; i < CS_NR; i++) + nob += snprintf(page + nob, count - nob, + "%8s", names[i]); + nob += snprintf(page + nob, count - nob, "\n"); + } + + nob += snprintf(page + nob, count - nob, "%5.5s:", cs->cs_name); + for (i = 0; i < CS_NR; i++) + nob += snprintf(page + nob, count - nob, "%8u", + cfs_atomic_read(&cs->cs_stats[i])); + return nob; } /** @@ -454,11 +457,7 @@ EXPORT_SYMBOL(cl_site_fini); static struct cache_stats cl_env_stats = { .cs_name = "envs", - .cs_created = CFS_ATOMIC_INIT(0), - .cs_lookup = CFS_ATOMIC_INIT(0), - .cs_hit = CFS_ATOMIC_INIT(0), - .cs_total = CFS_ATOMIC_INIT(0), - .cs_busy = CFS_ATOMIC_INIT(0) + .cs_stats = { CFS_ATOMIC_INIT(0), } }; /** @@ -585,13 +584,17 @@ struct cl_env { void *ce_debug; }; -#define CL_ENV_INC(counter) cfs_atomic_inc(&cl_env_stats.counter) +#ifdef CONFIG_DEBUG_PAGESTATE_TRACKING +#define CL_ENV_INC(counter) cfs_atomic_inc(&cl_env_stats.cs_stats[CS_##counter]) -#define CL_ENV_DEC(counter) \ - do { \ - LASSERT(cfs_atomic_read(&cl_env_stats.counter) > 0); \ - cfs_atomic_dec(&cl_env_stats.counter); \ - } while (0) +#define CL_ENV_DEC(counter) do { \ + LASSERT(cfs_atomic_read(&cl_env_stats.cs_stats[CS_##counter]) > 0); \ + cfs_atomic_dec(&cl_env_stats.cs_stats[CS_##counter]); \ +} while (0) +#else +#define CL_ENV_INC(counter) +#define CL_ENV_DEC(counter) +#endif static void cl_env_init0(struct cl_env *cle, void *debug) { @@ -601,7 +604,7 @@ static void cl_env_init0(struct cl_env *cle, void *debug) cle->ce_ref = 1; cle->ce_debug = debug; - CL_ENV_INC(cs_busy); + CL_ENV_INC(busy); } @@ -776,8 +779,8 @@ static struct lu_env *cl_env_new(__u32 ctx_tags, __u32 ses_tags, void *debug) OBD_SLAB_FREE_PTR(cle, cl_env_kmem); env = ERR_PTR(rc); } else { - CL_ENV_INC(cs_created); - CL_ENV_INC(cs_total); + CL_ENV_INC(create); + CL_ENV_INC(total); } } else env = ERR_PTR(-ENOMEM); @@ -786,7 +789,7 @@ static struct lu_env *cl_env_new(__u32 ctx_tags, __u32 ses_tags, void *debug) static void cl_env_fini(struct cl_env *cle) { - CL_ENV_DEC(cs_total); + CL_ENV_DEC(total); lu_context_fini(&cle->ce_lu.le_ctx); lu_context_fini(&cle->ce_ses); OBD_SLAB_FREE_PTR(cle, cl_env_kmem); @@ -836,7 +839,7 @@ struct lu_env *cl_env_peek(int *refcheck) struct lu_env *env; struct cl_env *cle; - CL_ENV_INC(cs_lookup); + CL_ENV_INC(lookup); /* check that we don't go far from untrusted pointer */ CLASSERT(offsetof(struct cl_env, ce_magic) == 0); @@ -844,7 +847,7 @@ struct lu_env *cl_env_peek(int *refcheck) env = NULL; cle = cl_env_fetch(); if (cle != NULL) { - CL_ENV_INC(cs_hit); + CL_ENV_INC(hit); env = &cle->ce_lu; *refcheck = ++cle->ce_ref; } @@ -960,7 +963,7 @@ void cl_env_put(struct lu_env *env, int *refcheck) CDEBUG(D_OTHER, "%d@%p\n", cle->ce_ref, cle); if (--cle->ce_ref == 0) { - CL_ENV_DEC(cs_busy); + CL_ENV_DEC(busy); cl_env_detach(cle); cle->ce_debug = NULL; cl_env_exit(cle); diff --git a/lustre/obdclass/cl_page.c b/lustre/obdclass/cl_page.c index 9dbdf24..67ca346 100644 --- a/lustre/obdclass/cl_page.c +++ b/lustre/obdclass/cl_page.c @@ -90,6 +90,23 @@ static struct lu_kmem_descr cl_page_caches[] = { ((void)sizeof(env), (void)sizeof(page), (void)sizeof !!(exp)) #endif /* !INVARIANT_CHECK */ +/* Disable page statistic by default due to huge performance penalty. */ +#ifdef CONFIG_DEBUG_PAGESTATE_TRACKING +#define CS_PAGE_INC(o, item) \ + cfs_atomic_inc(&cl_object_site(o)->cs_pages.cs_stats[CS_##item]) +#define CS_PAGE_DEC(o, item) \ + cfs_atomic_dec(&cl_object_site(o)->cs_pages.cs_stats[CS_##item]) +#define CS_PAGESTATE_INC(o, state) \ + cfs_atomic_inc(&cl_object_site(o)->cs_pages_state[state]) +#define CS_PAGESTATE_DEC(o, state) \ + cfs_atomic_dec(&cl_object_site(o)->cs_pages_state[state]) +#else +#define CS_PAGE_INC(o, item) +#define CS_PAGE_DEC(o, item) +#define CS_PAGESTATE_INC(o, state) +#define CS_PAGESTATE_DEC(o, state) +#endif + /** * Internal version of cl_page_top, it should be called with page referenced, * or cp_lock held. @@ -118,7 +135,7 @@ static void cl_page_get_trust(struct cl_page *page) * Checkless version for trusted users. */ if (cfs_atomic_inc_return(&page->cp_ref) == 1) - cfs_atomic_inc(&cl_object_site(page->cp_obj)->cs_pages.cs_busy); + CS_PAGE_INC(page->cp_obj, busy); } /** @@ -280,7 +297,6 @@ EXPORT_SYMBOL(cl_page_gang_lookup); static void cl_page_free(const struct lu_env *env, struct cl_page *page) { struct cl_object *obj = page->cp_obj; - struct cl_site *site = cl_object_site(obj); PASSERT(env, page, cfs_list_empty(&page->cp_batch)); PASSERT(env, page, page->cp_owner == NULL); @@ -298,11 +314,8 @@ static void cl_page_free(const struct lu_env *env, struct cl_page *page) cfs_list_del_init(page->cp_layers.next); slice->cpl_ops->cpo_fini(env, slice); } - cfs_atomic_dec(&site->cs_pages.cs_total); - -#ifdef LUSTRE_PAGESTATE_TRACKING - cfs_atomic_dec(&site->cs_pages_state[page->cp_state]); -#endif + CS_PAGE_DEC(obj, total); + CS_PAGESTATE_DEC(obj, page->cp_state); lu_object_ref_del_at(&obj->co_lu, page->cp_obj_ref, "cl_page", page); cl_object_put(env, obj); lu_ref_fini(&page->cp_reference); @@ -328,7 +341,6 @@ static int cl_page_alloc(const struct lu_env *env, struct cl_object *o, struct cl_page *page; struct cl_page *err = NULL; struct lu_object_header *head; - struct cl_site *site = cl_object_site(o); int result; ENTRY; @@ -364,13 +376,10 @@ static int cl_page_alloc(const struct lu_env *env, struct cl_object *o, } } if (err == NULL) { - cfs_atomic_inc(&site->cs_pages.cs_busy); - cfs_atomic_inc(&site->cs_pages.cs_total); - -#ifdef LUSTRE_PAGESTATE_TRACKING - cfs_atomic_inc(&site->cs_pages_state[CPS_CACHED]); -#endif - cfs_atomic_inc(&site->cs_pages.cs_created); + CS_PAGE_INC(o, busy); + CS_PAGE_INC(o, total); + CS_PAGE_INC(o, create); + CS_PAGESTATE_DEC(o, CPS_CACHED); result = 0; } } else @@ -399,7 +408,6 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, struct cl_page *page = NULL; struct cl_page *ghost = NULL; struct cl_object_header *hdr; - struct cl_site *site = cl_object_site(o); int err; LASSERT(type == CPT_CACHEABLE || type == CPT_TRANSIENT); @@ -408,7 +416,7 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, ENTRY; hdr = cl_object_header(o); - cfs_atomic_inc(&site->cs_pages.cs_lookup); + CS_PAGE_INC(o, lookup); CDEBUG(D_PAGE, "%lu@"DFID" %p %lx %d\n", idx, PFID(&hdr->coh_lu.loh_fid), vmpage, vmpage->private, type); @@ -437,7 +445,7 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, } if (page != NULL) { - cfs_atomic_inc(&site->cs_pages.cs_hit); + CS_PAGE_INC(o, hit); RETURN(page); } @@ -490,7 +498,7 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, spin_unlock(&hdr->coh_page_guard); if (unlikely(ghost != NULL)) { - cfs_atomic_dec(&site->cs_pages.cs_busy); + CS_PAGE_DEC(o, busy); cl_page_delete0(env, ghost, 0); cl_page_free(env, ghost); } @@ -554,9 +562,6 @@ static void cl_page_state_set0(const struct lu_env *env, struct cl_page *page, enum cl_page_state state) { enum cl_page_state old; -#ifdef LUSTRE_PAGESTATE_TRACKING - struct cl_site *site = cl_object_site(page->cp_obj); -#endif /* * Matrix of allowed state transitions [old][new], for sanity @@ -609,10 +614,8 @@ static void cl_page_state_set0(const struct lu_env *env, PASSERT(env, page, equi(state == CPS_OWNED, page->cp_owner != NULL)); -#ifdef LUSTRE_PAGESTATE_TRACKING - cfs_atomic_dec(&site->cs_pages_state[page->cp_state]); - cfs_atomic_inc(&site->cs_pages_state[state]); -#endif + CS_PAGESTATE_DEC(page->cp_obj, page->cp_state); + CS_PAGESTATE_INC(page->cp_obj, state); cl_page_state_set_trust(page, state); } EXIT; @@ -651,8 +654,6 @@ EXPORT_SYMBOL(cl_page_get); */ void cl_page_put(const struct lu_env *env, struct cl_page *page) { - struct cl_site *site = cl_object_site(page->cp_obj); - PASSERT(env, page, cfs_atomic_read(&page->cp_ref) > !!page->cp_parent); ENTRY; @@ -660,7 +661,7 @@ void cl_page_put(const struct lu_env *env, struct cl_page *page) cfs_atomic_read(&page->cp_ref)); if (cfs_atomic_dec_and_lock(&page->cp_ref, &page->cp_lock)) { - cfs_atomic_dec(&site->cs_pages.cs_busy); + CS_PAGE_DEC(page->cp_obj, busy); /* We're going to access the page w/o a reference, but it's * ok because we have grabbed the lock cp_lock, which * means nobody is able to free this page behind us. -- 1.8.3.1