From cad6fafa68de08570b499396d82a62c68140bfd3 Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Mon, 3 Jun 2013 21:40:45 +0800 Subject: [PATCH] staging/lustre/osc: some cleanup to reduce stack overflow chance ptlrpcd_add_req() will wake_up other process, do not hold a spinlock before calling ptlrpcd_queue_work()->ptlrpcd_add_req(). If current process is allocating memory, memory shrinker could get to osc_lru_del(), don't call osc_lru_shrink() further since it could lead a long calling chain. Use static string OES_STRINGS in OSC_EXTENT_DUMP() to reduce stack footprint. Alloc crattr on heap for osc_build_rpc() to reduce stack footprint. Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3281 Lustre-change: http://review.whamcloud.com/6270 Signed-off-by: Bobi Jam Reviewed-by: Jinshan Xiong Reviewed-by: Keith Mannthey Reviewed-by: Andreas Dilger Signed-off-by: Peng Tao Signed-off-by: Andreas Dilger Signed-off-by: Greg Kroah-Hartman --- drivers/staging/lustre/lustre/osc/osc_cache.c | 44 +++++++------ .../lustre/lustre/osc/osc_cl_internal.h | 2 - drivers/staging/lustre/lustre/osc/osc_page.c | 3 +- .../staging/lustre/lustre/osc/osc_request.c | 62 +++++++++++-------- 4 files changed, 59 insertions(+), 52 deletions(-) diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c index 54770f9b51da..c7889ab1acd9 100644 --- a/drivers/staging/lustre/lustre/osc/osc_cache.c +++ b/drivers/staging/lustre/lustre/osc/osc_cache.c @@ -99,10 +99,11 @@ static inline char list_empty_marker(struct list_head *list) #define EXTSTR "[%lu -> %lu/%lu]" #define EXTPARA(ext) (ext)->oe_start, (ext)->oe_end, (ext)->oe_max_end +static const char *oes_strings[] = { + "inv", "active", "cache", "locking", "lockdone", "rpc", "trunc", NULL }; #define OSC_EXTENT_DUMP(lvl, extent, fmt, ...) do { \ struct osc_extent *__ext = (extent); \ - const char *__str[] = OES_STRINGS; \ char __buf[16]; \ \ CDEBUG(lvl, \ @@ -114,7 +115,7 @@ static inline char list_empty_marker(struct list_head *list) atomic_read(&__ext->oe_refc), \ atomic_read(&__ext->oe_users), \ list_empty_marker(&__ext->oe_link), \ - __str[__ext->oe_state], ext_flags(__ext, __buf), \ + oes_strings[__ext->oe_state], ext_flags(__ext, __buf), \ __ext->oe_obj, \ /* ----- part 2 ----- */ \ __ext->oe_grants, __ext->oe_nr_pages, \ @@ -128,10 +129,10 @@ static inline char list_empty_marker(struct list_head *list) #undef EASSERTF #define EASSERTF(expr, ext, fmt, args...) do { \ if (!(expr)) { \ - OSC_EXTENT_DUMP(D_ERROR, (ext), fmt, ##args); \ - osc_extent_tree_dump(D_ERROR, (ext)->oe_obj); \ + OSC_EXTENT_DUMP(D_ERROR, (ext), fmt, ##args); \ + osc_extent_tree_dump(D_ERROR, (ext)->oe_obj); \ LASSERT(expr); \ - } \ + } \ } while (0) #undef EASSERT @@ -2125,27 +2126,24 @@ static void osc_check_rpcs(const struct lu_env *env, struct client_obd *cli, static int osc_io_unplug0(const struct lu_env *env, struct client_obd *cli, struct osc_object *osc, pdl_policy_t pol, int async) { - int has_rpcs = 1; int rc = 0; - client_obd_list_lock(&cli->cl_loi_list_lock); - if (osc != NULL) - has_rpcs = __osc_list_maint(cli, osc); - if (has_rpcs) { - if (!async) { - /* disable osc_lru_shrink() temporarily to avoid - * potential stack overrun problem. LU-2859 */ - atomic_inc(&cli->cl_lru_shrinkers); - osc_check_rpcs(env, cli, pol); - atomic_dec(&cli->cl_lru_shrinkers); - } else { - CDEBUG(D_CACHE, "Queue writeback work for client %p.\n", - cli); - LASSERT(cli->cl_writeback_work != NULL); - rc = ptlrpcd_queue_work(cli->cl_writeback_work); - } + if (osc != NULL && osc_list_maint(cli, osc) == 0) + return 0; + + if (!async) { + /* disable osc_lru_shrink() temporarily to avoid + * potential stack overrun problem. LU-2859 */ + atomic_inc(&cli->cl_lru_shrinkers); + client_obd_list_lock(&cli->cl_loi_list_lock); + osc_check_rpcs(env, cli, pol); + client_obd_list_unlock(&cli->cl_loi_list_lock); + atomic_dec(&cli->cl_lru_shrinkers); + } else { + CDEBUG(D_CACHE, "Queue writeback work for client %p.\n", cli); + LASSERT(cli->cl_writeback_work != NULL); + rc = ptlrpcd_queue_work(cli->cl_writeback_work); } - client_obd_list_unlock(&cli->cl_loi_list_lock); return rc; } diff --git a/drivers/staging/lustre/lustre/osc/osc_cl_internal.h b/drivers/staging/lustre/lustre/osc/osc_cl_internal.h index 001a9c84ab8e..158e8fff838f 100644 --- a/drivers/staging/lustre/lustre/osc/osc_cl_internal.h +++ b/drivers/staging/lustre/lustre/osc/osc_cl_internal.h @@ -584,8 +584,6 @@ enum osc_extent_state { OES_TRUNC = 6, /** being truncated */ OES_STATE_MAX }; -#define OES_STRINGS { "inv", "active", "cache", "locking", "lockdone", "rpc", \ - "trunc", NULL } /** * osc_extent data to manage dirty pages. diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c index 07d3702ac574..baba959a7450 100644 --- a/drivers/staging/lustre/lustre/osc/osc_page.c +++ b/drivers/staging/lustre/lustre/osc/osc_page.c @@ -806,7 +806,8 @@ static void osc_lru_del(struct client_obd *cli, struct osc_page *opg, bool del) * stealing one of them. * cl_lru_shrinkers is to avoid recursive call in case * we're already in the context of osc_lru_shrink(). */ - if (atomic_read(&cli->cl_lru_shrinkers) == 0) + if (atomic_read(&cli->cl_lru_shrinkers) == 0 && + !memory_pressure_get()) osc_lru_shrink(cli, osc_cache_too_much(cli)); wake_up(&osc_lru_waitq); } diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c index d2811d4cc8d7..9b48181083d4 100644 --- a/drivers/staging/lustre/lustre/osc/osc_request.c +++ b/drivers/staging/lustre/lustre/osc/osc_request.c @@ -2041,21 +2041,26 @@ static int brw_interpret(const struct lu_env *env, int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, struct list_head *ext_list, int cmd, pdl_policy_t pol) { - struct ptlrpc_request *req = NULL; - struct osc_extent *ext; + struct ptlrpc_request *req = NULL; + struct osc_extent *ext; + struct brw_page **pga = NULL; + struct osc_brw_async_args *aa = NULL; + struct obdo *oa = NULL; + struct osc_async_page *oap; + struct osc_async_page *tmp; + struct cl_req *clerq = NULL; + enum cl_req_type crt = (cmd & OBD_BRW_WRITE) ? CRT_WRITE : + CRT_READ; + struct ldlm_lock *lock = NULL; + struct cl_req_attr *crattr = NULL; + obd_off starting_offset = OBD_OBJECT_EOF; + obd_off ending_offset = 0; + int mpflag = 0; + int mem_tight = 0; + int page_count = 0; + int i; + int rc; LIST_HEAD(rpc_list); - struct brw_page **pga = NULL; - struct osc_brw_async_args *aa = NULL; - struct obdo *oa = NULL; - struct osc_async_page *oap; - struct osc_async_page *tmp; - struct cl_req *clerq = NULL; - enum cl_req_type crt = (cmd & OBD_BRW_WRITE) ? CRT_WRITE : CRT_READ; - struct ldlm_lock *lock = NULL; - struct cl_req_attr crattr; - obd_off starting_offset = OBD_OBJECT_EOF; - obd_off ending_offset = 0; - int i, rc, mpflag = 0, mem_tight = 0, page_count = 0; ENTRY; LASSERT(!list_empty(ext_list)); @@ -2083,7 +2088,10 @@ int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, if (mem_tight) mpflag = cfs_memory_pressure_get_and_set(); - memset(&crattr, 0, sizeof crattr); + OBD_ALLOC(crattr, sizeof(*crattr)); + if (crattr == NULL) + GOTO(out, rc = -ENOMEM); + OBD_ALLOC(pga, sizeof(*pga) * page_count); if (pga == NULL) GOTO(out, rc = -ENOMEM); @@ -2097,8 +2105,7 @@ int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, struct cl_page *page = oap2cl_page(oap); if (clerq == NULL) { clerq = cl_req_alloc(env, page, crt, - 1 /* only 1-object rpcs for - * now */); + 1 /* only 1-object rpcs for now */); if (IS_ERR(clerq)) GOTO(out, rc = PTR_ERR(clerq)); lock = oap->oap_ldlm_lock; @@ -2108,17 +2115,16 @@ int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, pga[i] = &oap->oap_brw_page; pga[i]->off = oap->oap_obj_off + oap->oap_page_off; CDEBUG(0, "put page %p index %lu oap %p flg %x to pga\n", - pga[i]->pg, page_index(oap->oap_page), oap, pga[i]->flag); + pga[i]->pg, page_index(oap->oap_page), oap, + pga[i]->flag); i++; cl_req_page_add(env, clerq, page); } /* always get the data for the obdo for the rpc */ LASSERT(clerq != NULL); - crattr.cra_oa = oa; - crattr.cra_capa = NULL; - memset(crattr.cra_jobid, 0, JOBSTATS_JOBID_SIZE); - cl_req_attr_set(env, clerq, &crattr, ~0ULL); + crattr->cra_oa = oa; + cl_req_attr_set(env, clerq, crattr, ~0ULL); if (lock) { oa->o_handle = lock->l_remote_handle; oa->o_valid |= OBD_MD_FLHANDLE; @@ -2132,7 +2138,7 @@ int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, sort_brw_pages(pga, page_count); rc = osc_brw_prep_request(cmd, cli, oa, NULL, page_count, - pga, &req, crattr.cra_capa, 1, 0); + pga, &req, crattr->cra_capa, 1, 0); if (rc != 0) { CERROR("prep_req failed: %d\n", rc); GOTO(out, rc); @@ -2148,10 +2154,10 @@ int osc_build_rpc(const struct lu_env *env, struct client_obd *cli, * later setattr before earlier BRW (as determined by the request xid), * the OST will not use BRW timestamps. Sadly, there is no obvious * way to do this in a single call. bug 10150 */ - cl_req_attr_set(env, clerq, &crattr, + cl_req_attr_set(env, clerq, crattr, OBD_MD_FLMTIME|OBD_MD_FLCTIME|OBD_MD_FLATIME); - lustre_msg_set_jobid(req->rq_reqmsg, crattr.cra_jobid); + lustre_msg_set_jobid(req->rq_reqmsg, crattr->cra_jobid); CLASSERT(sizeof(*aa) <= sizeof(req->rq_async_args)); aa = ptlrpc_req_async_args(req); @@ -2218,7 +2224,11 @@ out: if (mem_tight != 0) cfs_memory_pressure_restore(mpflag); - capa_put(crattr.cra_capa); + if (crattr != NULL) { + capa_put(crattr->cra_capa); + OBD_FREE(crattr, sizeof(*crattr)); + } + if (rc != 0) { LASSERT(req == NULL); -- 2.30.2