From: Gregoire Pichon Date: Tue, 12 Feb 2013 13:52:54 +0000 (+0100) Subject: LU-744 obdclass: revise cl_page refcount X-Git-Tag: 2.1.5~19 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=17f83b93481932e3476b076651ab60e1fbd15136;p=fs%2Flustre-release.git LU-744 obdclass: revise cl_page refcount By holding a refcount for radix tree, we can remove a couple of spin lock and atomic operation for cl_page. Signed-off-by: Jinshan Xiong Signed-off-by: Gregoire Pichon Change-Id: I4d8dc04b9b4ef066b54a90be3cfbd0c09d96c320 Reviewed-on: http://review.whamcloud.com/5429 Tested-by: Hudson Tested-by: Maloo Reviewed-by: Bobi Jam Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h index 2b99492..b75209a 100644 --- a/lustre/include/cl_object.h +++ b/lustre/include/cl_object.h @@ -716,8 +716,6 @@ struct cl_page { * modified only internally within cl_page.c. Protected by a VM lock. */ const enum cl_page_state cp_state; - /** Protect to get and put page, see cl_page_put and cl_vmpage_page */ - cfs_spinlock_t cp_lock; /** * Linkage of pages within some group. Protected by * cl_page::cp_mutex. */ @@ -1088,6 +1086,16 @@ do { \ } \ } while (0) +static inline int __page_in_use(const struct cl_page *page, int refc) +{ + if (page->cp_type == CPT_CACHEABLE) + ++refc; + LASSERT(cfs_atomic_read(&page->cp_ref) > 0); + return (cfs_atomic_read(&page->cp_ref) > refc); +} +#define cl_page_in_use(pg) __page_in_use(pg, 1) +#define cl_page_in_use_noref(pg) __page_in_use(pg, 0) + /** @} cl_page */ /** \addtogroup cl_lock cl_lock diff --git a/lustre/llite/rw26.c b/lustre/llite/rw26.c index bf5ca8a..29129cf 100644 --- a/lustre/llite/rw26.c +++ b/lustre/llite/rw26.c @@ -178,7 +178,7 @@ static int ll_releasepage(struct page *vmpage, RELEASEPAGE_ARG_TYPE gfp_mask) page = cl_vmpage_page(vmpage, obj); result = page == NULL; if (page != NULL) { - if (cfs_atomic_read(&page->cp_ref) == 1) { + if (!cl_page_in_use(page)) { result = 1; cl_page_delete(env, page); } diff --git a/lustre/obdclass/cl_page.c b/lustre/obdclass/cl_page.c index 7818746..213e2bc 100644 --- a/lustre/obdclass/cl_page.c +++ b/lustre/obdclass/cl_page.c @@ -97,8 +97,9 @@ static struct lu_kmem_descr cl_page_caches[] = { #endif /* !INVARIANT_CHECK */ /** - * Internal version of cl_page_top, it should be called with page referenced, - * or cp_lock held. + * Internal version of cl_page_top, it should be called if the page is + * known to be not freed, says with page referenced, or radix tree lock held, + * or page owned. */ static struct cl_page *cl_page_top_trusted(struct cl_page *page) { @@ -120,11 +121,8 @@ static struct cl_page *cl_page_top_trusted(struct cl_page *page) */ 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); + LASSERT(cfs_atomic_read(&page->cp_ref) > 0); + cfs_atomic_inc(&page->cp_ref); } /** @@ -138,11 +136,6 @@ cl_page_at_trusted(const struct cl_page *page, const struct lu_device_type *dtype) { const struct cl_page_slice *slice; - -#ifdef INVARIANT_CHECK - if (!cfs_atomic_read(&page->cp_ref)) - LASSERT_SPIN_LOCKED(&page->cp_lock); -#endif ENTRY; page = cl_page_top_trusted((struct cl_page *)page); @@ -169,9 +162,8 @@ struct cl_page *cl_page_lookup(struct cl_object_header *hdr, pgoff_t index) LASSERT_SPIN_LOCKED(&hdr->coh_page_guard); page = radix_tree_lookup(&hdr->coh_tree, index); - if (page != NULL) { + if (page != NULL) cl_page_get_trust(page); - } return page; } EXPORT_SYMBOL(cl_page_lookup); @@ -342,13 +334,14 @@ static int cl_page_alloc(const struct lu_env *env, struct cl_object *o, OBD_SLAB_ALLOC_PTR_GFP(page, cl_page_kmem, CFS_ALLOC_IO); if (page != NULL) { cfs_atomic_set(&page->cp_ref, 1); + if (type == CPT_CACHEABLE) /* for radix tree */ + cfs_atomic_inc(&page->cp_ref); page->cp_obj = o; cl_object_get(o); page->cp_obj_ref = lu_object_ref_add(&o->co_lu, "cl_page", page); page->cp_index = ind; cl_page_state_set_trust(page, CPS_CACHED); - cfs_spin_lock_init(&page->cp_lock); page->cp_type = type; CFS_INIT_LIST_HEAD(&page->cp_layers); CFS_INIT_LIST_HEAD(&page->cp_batch); @@ -370,7 +363,6 @@ 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 @@ -420,10 +412,8 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, idx, PFID(&hdr->coh_lu.loh_fid), vmpage, vmpage->private, type); /* fast path. */ if (type == CPT_CACHEABLE) { - /* cl_page::cp_lock is used to protect the page state and - * refcount, but need an external lock to protect the - * child/parent relationship, so vmpage lock must be held for - * this purpose. */ + /* vmpage lock is used to protect the child/parent + * relationship */ KLASSERT(PageLocked(vmpage)); /* * cl_vmpage_page() can be called here without any locks as @@ -496,7 +486,6 @@ static struct cl_page *cl_page_find0(const struct lu_env *env, cfs_spin_unlock(&hdr->coh_page_guard); if (unlikely(ghost != NULL)) { - cfs_atomic_dec(&site->cs_pages.cs_busy); cl_page_delete0(env, ghost, 0); cl_page_free(env, ghost); } @@ -537,7 +526,7 @@ static inline int cl_page_invariant(const struct cl_page *pg) child = pg->cp_child; owner = pg->cp_owner; - return cfs_atomic_read(&pg->cp_ref) > 0 && + return cl_page_in_use(pg) && ergo(parent != NULL, parent->cp_child == pg) && ergo(child != NULL, child->cp_parent == pg) && ergo(child != NULL, pg->cp_obj != child->cp_obj) && @@ -659,40 +648,23 @@ 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; CL_PAGE_HEADER(D_TRACE, env, page, "%d\n", 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); - /* 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. - */ - if (page->cp_state == CPS_FREEING) { - /* We drop the page reference and check the page state - * inside the cp_lock. So that if it gets here, - * it is the REALLY last reference to this page. - */ - cfs_spin_unlock(&page->cp_lock); - - LASSERT(cfs_atomic_read(&page->cp_ref) == 0); - PASSERT(env, page, page->cp_owner == NULL); - PASSERT(env, page, cfs_list_empty(&page->cp_batch)); - /* - * Page is no longer reachable by other threads. Tear - * it down. - */ - cl_page_free(env, page); - - EXIT; - return; - } - cfs_spin_unlock(&page->cp_lock); + if (cfs_atomic_dec_and_test(&page->cp_ref)) { + LASSERT(page->cp_state == CPS_FREEING); + + LASSERT(cfs_atomic_read(&page->cp_ref) == 0); + PASSERT(env, page, page->cp_owner == NULL); + PASSERT(env, page, cfs_list_empty(&page->cp_batch)); + /* + * Page is no longer reachable by other threads. Tear + * it down. + */ + cl_page_free(env, page); } EXIT; @@ -747,14 +719,12 @@ struct cl_page *cl_vmpage_page(cfs_page_t *vmpage, struct cl_object *obj) if (top == NULL) RETURN(NULL); - cfs_spin_lock(&top->cp_lock); for (page = top; page != NULL; page = page->cp_child) { if (cl_object_same(page->cp_obj, obj)) { cl_page_get_trust(page); break; } } - cfs_spin_unlock(&top->cp_lock); LASSERT(ergo(page, page->cp_type == CPT_CACHEABLE)); RETURN(page); } @@ -1156,6 +1126,9 @@ static void cl_page_delete0(const struct lu_env *env, struct cl_page *pg, cl_page_export(env, pg, 0); cl_page_state_set0(env, pg, CPS_FREEING); + CL_PAGE_INVOID(env, pg, CL_PAGE_OP(cpo_delete), + (const struct lu_env *, const struct cl_page_slice *)); + if (tmp->cp_type == CPT_CACHEABLE) { if (!radix) /* !radix means that @pg is not yet in the radix tree, @@ -1174,11 +1147,10 @@ static void cl_page_delete0(const struct lu_env *env, struct cl_page *pg, PASSERT(env, tmp, hdr->coh_pages > 0); hdr->coh_pages--; cfs_spin_unlock(&hdr->coh_page_guard); + cl_page_put(env, tmp); } } - CL_PAGE_INVOID(env, pg, CL_PAGE_OP(cpo_delete), - (const struct lu_env *, const struct cl_page_slice *)); EXIT; }