From e533c7c6d1eb2bbdc4818f688f76fab1d03bc2c8 Mon Sep 17 00:00:00 2001 From: Yang Sheng Date: Sat, 19 Dec 2015 02:22:07 +0800 Subject: [PATCH] LU-7084 mem: code cleanup for memory allocation This patch just want fix some memory allocation issues. Signed-off-by: Yang Sheng Change-Id: I53d622d5c4e71ce42488a6cfce17a10063cf4f4d Reviewed-on: http://review.whamcloud.com/17673 Tested-by: Jenkins Reviewed-by: Andreas Dilger Reviewed-by: Fan Yong Tested-by: Maloo Reviewed-by: Mike Pershin Reviewed-by: Oleg Drokin --- libcfs/libcfs/hash.c | 26 ++++++++++----------- lustre/llite/super25.c | 2 +- lustre/lod/lod_object.c | 11 ++++----- lustre/obdecho/echo_client.c | 24 ++++++++++---------- lustre/ofd/ofd_dev.c | 54 +++++++++++++++++++------------------------- 5 files changed, 54 insertions(+), 63 deletions(-) diff --git a/libcfs/libcfs/hash.c b/libcfs/libcfs/hash.c index 5133628..559fc06 100644 --- a/libcfs/libcfs/hash.c +++ b/libcfs/libcfs/hash.c @@ -1231,23 +1231,23 @@ cfs_hash_find_or_add(struct cfs_hash *hs, const void *key, struct cfs_hash_bd bds[2]; int bits = 0; - LASSERT(hlist_unhashed(hnode)); + LASSERTF(hlist_unhashed(hnode), "hnode = %p\n", hnode); - cfs_hash_lock(hs, 0); - cfs_hash_dual_bd_get_and_lock(hs, key, bds, 1); + cfs_hash_lock(hs, 0); + cfs_hash_dual_bd_get_and_lock(hs, key, bds, 1); - cfs_hash_key_validate(hs, key, hnode); - ehnode = cfs_hash_dual_bd_findadd_locked(hs, bds, key, - hnode, noref); - cfs_hash_dual_bd_unlock(hs, bds, 1); + cfs_hash_key_validate(hs, key, hnode); + ehnode = cfs_hash_dual_bd_findadd_locked(hs, bds, key, + hnode, noref); + cfs_hash_dual_bd_unlock(hs, bds, 1); - if (ehnode == hnode) /* new item added */ - bits = cfs_hash_rehash_bits(hs); - cfs_hash_unlock(hs, 0); - if (bits > 0) - cfs_hash_rehash(hs, cfs_hash_rehash_inline(hs)); + if (ehnode == hnode) /* new item added */ + bits = cfs_hash_rehash_bits(hs); + cfs_hash_unlock(hs, 0); + if (bits > 0) + cfs_hash_rehash(hs, cfs_hash_rehash_inline(hs)); - return ehnode; + return ehnode; } /** diff --git a/lustre/llite/super25.c b/lustre/llite/super25.c index 71ac616..bfc42cc 100644 --- a/lustre/llite/super25.c +++ b/lustre/llite/super25.c @@ -137,7 +137,7 @@ static int __init lustre_init(void) ll_rmtperm_hash_cachep = kmem_cache_create("ll_rmtperm_hash_cache", REMOTE_PERM_HASHSIZE * - sizeof(struct list_head), + sizeof(struct hlist_head), 0, 0, NULL); if (ll_rmtperm_hash_cachep == NULL) GOTO(out_cache, rc = -ENOMEM); diff --git a/lustre/lod/lod_object.c b/lustre/lod/lod_object.c index 04cbd1a..62e8967 100644 --- a/lustre/lod/lod_object.c +++ b/lustre/lod/lod_object.c @@ -1713,12 +1713,12 @@ static int lod_prep_md_striped_create(const struct lu_env *env, stripe_count = le32_to_cpu(lum->lum_stripe_count); - OBD_ALLOC(stripe, sizeof(stripe[0]) * stripe_count); - if (stripe == NULL) - RETURN(-ENOMEM); - OBD_ALLOC(idx_array, sizeof(idx_array[0]) * stripe_count); if (idx_array == NULL) + RETURN(-ENOMEM); + + OBD_ALLOC(stripe, sizeof(stripe[0]) * stripe_count); + if (stripe == NULL) GOTO(out_free, rc = -ENOMEM); /* Start index will be the master MDT */ @@ -1842,8 +1842,7 @@ out_put: } out_free: - if (idx_array != NULL) - OBD_FREE(idx_array, sizeof(idx_array[0]) * stripe_count); + OBD_FREE(idx_array, sizeof(idx_array[0]) * stripe_count); RETURN(rc); } diff --git a/lustre/obdecho/echo_client.c b/lustre/obdecho/echo_client.c index e435f5d..68e4318 100644 --- a/lustre/obdecho/echo_client.c +++ b/lustre/obdecho/echo_client.c @@ -2414,20 +2414,20 @@ static int echo_client_prep_commit(const struct lu_env *env, struct niobuf_local *lnb; struct niobuf_remote rnb; u64 off; - u64 npages, tot_pages; + u64 npages, tot_pages, apc; int i, ret = 0, brw_flags = 0; - ENTRY; + ENTRY; if (count <= 0 || (count & ~PAGE_CACHE_MASK) != 0) RETURN(-EINVAL); - npages = batch >> PAGE_CACHE_SHIFT; + apc = npages = batch >> PAGE_CACHE_SHIFT; tot_pages = count >> PAGE_CACHE_SHIFT; - OBD_ALLOC(lnb, npages * sizeof(struct niobuf_local)); + OBD_ALLOC(lnb, apc * sizeof(struct niobuf_local)); if (lnb == NULL) - GOTO(out, ret = -ENOMEM); + RETURN(-ENOMEM); if (rw == OBD_BRW_WRITE && async) brw_flags |= OBD_BRW_ASYNC; @@ -2448,10 +2448,10 @@ static int echo_client_prep_commit(const struct lu_env *env, ioo.ioo_bufcnt = 1; off += npages * PAGE_CACHE_SIZE; - lpages = npages; + lpages = npages; ret = obd_preprw(env, rw, exp, oa, 1, &ioo, &rnb, &lpages, lnb); - if (ret != 0) - GOTO(out, ret); + if (ret != 0) + GOTO(out, ret); for (i = 0; i < lpages; i++) { struct page *page = lnb[i].lnb_page; @@ -2482,8 +2482,8 @@ static int echo_client_prep_commit(const struct lu_env *env, ret = obd_commitrw(env, rw, exp, oa, 1, &ioo, &rnb, npages, lnb, ret); - if (ret != 0) - GOTO(out, ret); + if (ret != 0) + break; /* Reuse env context. */ lu_context_exit((struct lu_context *)&env->le_ctx); @@ -2491,8 +2491,8 @@ static int echo_client_prep_commit(const struct lu_env *env, } out: - if (lnb) - OBD_FREE(lnb, npages * sizeof(struct niobuf_local)); + OBD_FREE(lnb, apc * sizeof(struct niobuf_local)); + RETURN(ret); } diff --git a/lustre/ofd/ofd_dev.c b/lustre/ofd/ofd_dev.c index 78b2a72..d48472b 100644 --- a/lustre/ofd/ofd_dev.c +++ b/lustre/ofd/ofd_dev.c @@ -834,60 +834,52 @@ int ofd_fid_init(const struct lu_env *env, struct ofd_device *ofd) ss->ss_lu = lu->ld_site; ss->ss_node_id = ofd->ofd_lut.lut_lsd.lsd_osd_index; + OBD_ALLOC(name, sizeof(obd_name) * 2 + 10); + if (name == NULL) + return -ENOMEM; + OBD_ALLOC_PTR(ss->ss_server_seq); if (ss->ss_server_seq == NULL) - GOTO(out_free, rc = -ENOMEM); - - OBD_ALLOC(name, strlen(obd_name) + 10); - if (!name) { - OBD_FREE_PTR(ss->ss_server_seq); - ss->ss_server_seq = NULL; - GOTO(out_free, rc = -ENOMEM); - } + GOTO(out_name, rc = -ENOMEM); rc = seq_server_init(env, ss->ss_server_seq, ofd->ofd_osd, obd_name, LUSTRE_SEQ_SERVER, ss); if (rc) { CERROR("%s : seq server init error %d\n", obd_name, rc); - GOTO(out_free, rc); + GOTO(out_server, rc); } ss->ss_server_seq->lss_space.lsr_index = ss->ss_node_id; OBD_ALLOC_PTR(ss->ss_client_seq); if (ss->ss_client_seq == NULL) - GOTO(out_free, rc = -ENOMEM); + GOTO(out_server, rc = -ENOMEM); - snprintf(name, strlen(obd_name) + 6, "%p-super", obd_name); + /* + * It always printed as "%p", so that the name is unique in the kernel, + * even if the filesystem is mounted twice. So sizeof(.) * 2 is enough. + */ + snprintf(name, sizeof(obd_name) * 2 + 7, "%p-super", obd_name); rc = seq_client_init(ss->ss_client_seq, NULL, LUSTRE_SEQ_DATA, name, NULL); if (rc) { CERROR("%s : seq client init error %d\n", obd_name, rc); - GOTO(out_free, rc); + GOTO(out_client, rc); } - OBD_FREE(name, strlen(obd_name) + 10); - name = NULL; rc = seq_server_set_cli(env, ss->ss_server_seq, ss->ss_client_seq); -out_free: if (rc) { - if (ss->ss_server_seq) { - seq_server_fini(ss->ss_server_seq, env); - OBD_FREE_PTR(ss->ss_server_seq); - ss->ss_server_seq = NULL; - } - - if (ss->ss_client_seq) { - seq_client_fini(ss->ss_client_seq); - OBD_FREE_PTR(ss->ss_client_seq); - ss->ss_client_seq = NULL; - } - - if (name) { - OBD_FREE(name, strlen(obd_name) + 10); - name = NULL; - } +out_client: + seq_client_fini(ss->ss_client_seq); + OBD_FREE_PTR(ss->ss_client_seq); + ss->ss_client_seq = NULL; +out_server: + seq_server_fini(ss->ss_server_seq, env); + OBD_FREE_PTR(ss->ss_server_seq); + ss->ss_server_seq = NULL; } +out_name: + OBD_FREE(name, sizeof(obd_name) * 2 + 10); return rc; } -- 1.8.3.1