[PATCH 14/14] staging: lustre: remove LIBCFS_ALLOC, LIBCFS_FREE and related macros.
LIBCFS_ALLOC LIBCFS_ALLOC_ATOMIC LIBCFS_ALLOC_POST LIBCFS_CPT_ALLOC LIBCFS_FREE are no longer used, and so are removed. Signed-off-by: NeilBrown --- .../lustre/include/linux/libcfs/libcfs_private.h | 51 1 file changed, 51 deletions(-) diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h index 50a600564fb2..491d5971d199 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h @@ -75,57 +75,6 @@ do { \ lbug_with_loc(&msgdata);\ } while (0) -#define LIBCFS_ALLOC_POST(ptr, size) \ -do { \ - if (unlikely(!(ptr))) { \ - CERROR("LNET: out of memory at %s:%d (tried to alloc '" \ - #ptr "' = %d)\n", __FILE__, __LINE__, (int)(size)); \ - } else {\ - memset((ptr), 0, (size)); \ - } \ -} while (0) - -/** - * default allocator - */ -#define LIBCFS_ALLOC(ptr, size) \ -do { \ - LASSERT(!in_interrupt()); \ - (ptr) = kvmalloc((size), GFP_NOFS); \ - LIBCFS_ALLOC_POST((ptr), (size)); \ -} while (0) - -/** - * non-sleeping allocator - */ -#define LIBCFS_ALLOC_ATOMIC(ptr, size) \ -do { \ - (ptr) = kmalloc((size), GFP_ATOMIC);\ - LIBCFS_ALLOC_POST(ptr, size); \ -} while (0) - -/** - * allocate memory for specified CPU partition - * \a cptab != NULL, \a cpt is CPU partition id of \a cptab - * \a cptab == NULL, \a cpt is HW NUMA node id - */ -#define LIBCFS_CPT_ALLOC(ptr, cptab, cpt, size) \ -do { \ - LASSERT(!in_interrupt()); \ - (ptr) = kvmalloc_node((size), GFP_NOFS, cfs_cpt_spread_node(cptab, cpt)); \ - LIBCFS_ALLOC_POST((ptr), (size)); \ -} while (0) - -#define LIBCFS_FREE(ptr, size) \ -do { \ - if (unlikely(!(ptr))) { \ - CERROR("LIBCFS: free NULL '" #ptr "' (%d bytes) at "\ - "%s:%d\n", (int)(size), __FILE__, __LINE__); \ - break;\ - } \ - kvfree(ptr); \ -} while (0) - /* * Use #define rather than inline, as lnet_cpt_table() might * not be defined yet
[PATCH 07/14] staging: lustre: more LIBCFS_ALLOC conversions to GFP_KERNEL allocations.
None of these need GFP_NOFS so allocate directly. Change matching LIBCFS_FREE() to kfree() or kvfree(). Signed-off-by: NeilBrown --- .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 19 +++ drivers/staging/lustre/lnet/lnet/lib-eq.c |9 - drivers/staging/lustre/lnet/lnet/lib-socket.c | 14 +++--- drivers/staging/lustre/lnet/selftest/conctl.c | 11 +-- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c index 51196fda2a32..c07165e0ad95 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c @@ -93,11 +93,7 @@ cfs_cpt_table_free(struct cfs_cpt_table *cptab) { int i; - if (cptab->ctb_cpu2cpt) { - LIBCFS_FREE(cptab->ctb_cpu2cpt, - num_possible_cpus() * - sizeof(cptab->ctb_cpu2cpt[0])); - } + kvfree(cptab->ctb_cpu2cpt); for (i = 0; cptab->ctb_parts && i < cptab->ctb_nparts; i++) { struct cfs_cpu_partition *part = &cptab->ctb_parts[i]; @@ -106,10 +102,7 @@ cfs_cpt_table_free(struct cfs_cpt_table *cptab) free_cpumask_var(part->cpt_cpumask); } - if (cptab->ctb_parts) { - LIBCFS_FREE(cptab->ctb_parts, - cptab->ctb_nparts * sizeof(cptab->ctb_parts[0])); - } + kvfree(cptab->ctb_parts); kfree(cptab->ctb_nodemask); free_cpumask_var(cptab->ctb_cpumask); @@ -136,15 +129,17 @@ cfs_cpt_table_alloc(unsigned int ncpt) !cptab->ctb_nodemask) goto failed; - LIBCFS_ALLOC(cptab->ctb_cpu2cpt, -num_possible_cpus() * sizeof(cptab->ctb_cpu2cpt[0])); + cptab->ctb_cpu2cpt = kvmalloc_array(num_possible_cpus(), + sizeof(cptab->ctb_cpu2cpt[0]), + GFP_KERNEL); if (!cptab->ctb_cpu2cpt) goto failed; memset(cptab->ctb_cpu2cpt, -1, num_possible_cpus() * sizeof(cptab->ctb_cpu2cpt[0])); - LIBCFS_ALLOC(cptab->ctb_parts, ncpt * sizeof(cptab->ctb_parts[0])); + cptab->ctb_parts = kvmalloc_array(ncpt, sizeof(cptab->ctb_parts[0]), + GFP_KERNEL); if (!cptab->ctb_parts) goto failed; diff --git a/drivers/staging/lustre/lnet/lnet/lib-eq.c b/drivers/staging/lustre/lnet/lnet/lib-eq.c index 7a4d1f7a693e..a173b69e2f92 100644 --- a/drivers/staging/lustre/lnet/lnet/lib-eq.c +++ b/drivers/staging/lustre/lnet/lnet/lib-eq.c @@ -95,7 +95,8 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, return -ENOMEM; if (count) { - LIBCFS_ALLOC(eq->eq_events, count * sizeof(struct lnet_event)); + eq->eq_events = kvmalloc_array(count, sizeof(struct lnet_event), + GFP_KERNEL | __GFP_ZERO); if (!eq->eq_events) goto failed; /* @@ -132,8 +133,7 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, return 0; failed: - if (eq->eq_events) - LIBCFS_FREE(eq->eq_events, count * sizeof(struct lnet_event)); + kvfree(eq->eq_events); if (eq->eq_refs) cfs_percpt_free(eq->eq_refs); @@ -202,8 +202,7 @@ LNetEQFree(struct lnet_handle_eq eqh) lnet_eq_wait_unlock(); lnet_res_unlock(LNET_LOCK_EX); - if (events) - LIBCFS_FREE(events, size * sizeof(struct lnet_event)); + kvfree(events); if (refs) cfs_percpt_free(refs); diff --git a/drivers/staging/lustre/lnet/lnet/lib-socket.c b/drivers/staging/lustre/lnet/lnet/lib-socket.c index 7d49d4865298..fcd9d1b7c619 100644 --- a/drivers/staging/lustre/lnet/lnet/lib-socket.c +++ b/drivers/staging/lustre/lnet/lnet/lib-socket.c @@ -170,7 +170,7 @@ lnet_ipif_enumerate(char ***namesp) nalloc); } - LIBCFS_ALLOC(ifr, nalloc * sizeof(*ifr)); + ifr = kzalloc(nalloc * sizeof(*ifr), GFP_KERNEL); if (!ifr) { CERROR("ENOMEM enumerating up to %d interfaces\n", nalloc); @@ -195,14 +195,14 @@ lnet_ipif_enumerate(char ***namesp) if (nfound < nalloc || toobig) break; - LIBCFS_FREE(ifr, nalloc * sizeof(*ifr)); + kfree(ifr); nalloc *= 2; } if (!nfound) goto out1; - LIBCFS_ALLOC(names, nfound * sizeof(*names)); +
[PATCH 06/14] staging: lustre: Convert more LIBCFS_ALLOC allocation to direct GFP_KERNEL
None of these need to be GFP_NOFS, so use GFP_KERNEL explicitly with kmalloc(), kvmalloc(), or kvmalloc_array(). Change matching LIBCFS_FREE() to kfree() or kvfree() Signed-off-by: NeilBrown --- .../lustre/lnet/libcfs/linux/linux-module.c|4 +- drivers/staging/lustre/lnet/libcfs/module.c|9 ++--- drivers/staging/lustre/lnet/lnet/api-ni.c | 17 -- drivers/staging/lustre/lnet/lnet/config.c | 34 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c index b5746230ab31..ddf625669bff 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c @@ -146,7 +146,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return -EINVAL; } - LIBCFS_ALLOC(*hdr_pp, hdr.ioc_len); + *hdr_pp = kvmalloc(hdr.ioc_len, GFP_KERNEL); if (!*hdr_pp) return -ENOMEM; @@ -164,7 +164,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return 0; free: - LIBCFS_FREE(*hdr_pp, hdr.ioc_len); + kvfree(*hdr_pp); return err; } diff --git a/drivers/staging/lustre/lnet/libcfs/module.c b/drivers/staging/lustre/lnet/libcfs/module.c index eb1a1dea723d..555f47651730 100644 --- a/drivers/staging/lustre/lnet/libcfs/module.c +++ b/drivers/staging/lustre/lnet/libcfs/module.c @@ -156,7 +156,7 @@ int libcfs_ioctl(unsigned long cmd, void __user *uparam) break; } } out: - LIBCFS_FREE(hdr, hdr->ioc_len); + kvfree(hdr); return err; } @@ -302,7 +302,7 @@ static int __proc_cpt_table(void *data, int write, LASSERT(cfs_cpt_table); while (1) { - LIBCFS_ALLOC(buf, len); + buf = kzalloc(len, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -311,7 +311,7 @@ static int __proc_cpt_table(void *data, int write, break; if (rc == -EFBIG) { - LIBCFS_FREE(buf, len); + kfree(buf); len <<= 1; continue; } @@ -325,8 +325,7 @@ static int __proc_cpt_table(void *data, int write, rc = cfs_trace_copyout_string(buffer, nob, buf + pos, NULL); out: - if (buf) - LIBCFS_FREE(buf, len); + kfree(buf); return rc; } diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index e8f623190133..6a1fb0397604 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -108,7 +108,8 @@ lnet_create_remote_nets_table(void) LASSERT(!the_lnet.ln_remote_nets_hash); LASSERT(the_lnet.ln_remote_nets_hbits > 0); - LIBCFS_ALLOC(hash, LNET_REMOTE_NETS_HASH_SIZE * sizeof(*hash)); + hash = kvmalloc_array(LNET_REMOTE_NETS_HASH_SIZE, sizeof(*hash), + GFP_KERNEL); if (!hash) { CERROR("Failed to create remote nets hash table\n"); return -ENOMEM; @@ -131,9 +132,7 @@ lnet_destroy_remote_nets_table(void) for (i = 0; i < LNET_REMOTE_NETS_HASH_SIZE; i++) LASSERT(list_empty(&the_lnet.ln_remote_nets_hash[i])); - LIBCFS_FREE(the_lnet.ln_remote_nets_hash, - LNET_REMOTE_NETS_HASH_SIZE * - sizeof(the_lnet.ln_remote_nets_hash[0])); + kvfree(the_lnet.ln_remote_nets_hash); the_lnet.ln_remote_nets_hash = NULL; } @@ -831,7 +830,7 @@ lnet_ping_info_create(int num_ni) unsigned int infosz; infosz = offsetof(struct lnet_ping_info, pi_ni[num_ni]); - LIBCFS_ALLOC(ping_info, infosz); + ping_info = kvzalloc(infosz, GFP_KERNEL); if (!ping_info) { CERROR("Can't allocate ping info[%d]\n", num_ni); return NULL; @@ -864,9 +863,7 @@ lnet_get_ni_count(void) static inline void lnet_ping_info_free(struct lnet_ping_info *pinfo) { - LIBCFS_FREE(pinfo, - offsetof(struct lnet_ping_info, -pi_ni[pinfo->pi_nnis])); + kvfree(pinfo); } static void @@ -2160,7 +2157,7 @@ static int lnet_ping(struct lnet_process_id id, int timeout_ms, if (id.pid == LNET_PID_ANY) id.pid = LNET_PID_LUSTRE; - LIBCFS_ALLOC(info, infosz); + info = kzalloc(infosz, GFP_KERNEL); if (!info) return -ENOMEM; @@ -2310,6 +2307,6 @@ static int lnet_ping(struct lnet_process_id id, int timeout_ms, LASSERT(!rc2); out_0: - LIBCFS_FREE(info, infosz); + kfree(info); return rc; } diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/
[PATCH 01/14] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
All usages of the form LIBCFS_ALLOC(variable, sizeof(variable)) or LIBCFS_ALLOC(variable, sizeof(variable's-type)) are changed to variable = kzalloc(sizeof(...), GFP_NOFS); Similarly, all LIBCFS_FREE(variable, sizeof(variable)) become kfree(variable); None of these need the vmalloc option, or any of the other minor benefits of LIBCFS_ALLOC(). Signed-off-by: NeilBrown --- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 39 ++-- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |4 +- .../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c |4 +- .../staging/lustre/lnet/klnds/socklnd/socklnd.c| 22 ++- .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |2 + .../lustre/lnet/klnds/socklnd/socklnd_proto.c |8 ++-- drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c|4 +- drivers/staging/lustre/lnet/libcfs/libcfs_lock.c |6 ++- drivers/staging/lustre/lnet/libcfs/libcfs_string.c | 10 +++-- .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 19 -- drivers/staging/lustre/lnet/libcfs/workitem.c |8 ++-- drivers/staging/lustre/lnet/lnet/api-ni.c |4 +- drivers/staging/lustre/lnet/lnet/config.c |7 ++-- drivers/staging/lustre/lnet/lnet/lib-move.c|6 ++- drivers/staging/lustre/lnet/lnet/nidstrings.c |8 ++-- drivers/staging/lustre/lnet/lnet/peer.c|2 + drivers/staging/lustre/lnet/lnet/router.c | 28 ++ drivers/staging/lustre/lnet/lnet/router_proc.c |6 ++- drivers/staging/lustre/lnet/selftest/conrpc.c | 10 +++-- drivers/staging/lustre/lnet/selftest/console.c | 26 +++-- drivers/staging/lustre/lnet/selftest/framework.c | 22 ++- drivers/staging/lustre/lnet/selftest/rpc.c | 12 +++--- 22 files changed, 124 insertions(+), 133 deletions(-) diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 8024843521ab..3aa81168c84f 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -367,7 +367,7 @@ void kiblnd_destroy_peer(struct kib_peer *peer) LASSERT(kiblnd_peer_idle(peer)); LASSERT(list_empty(&peer->ibp_tx_queue)); - LIBCFS_FREE(peer, sizeof(*peer)); + kfree(peer); /* * NB a peer's connections keep a reference on their peer until @@ -776,7 +776,7 @@ struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, struct rdma_cm_id *cm goto failed_2; } - LIBCFS_FREE(init_qp_attr, sizeof(*init_qp_attr)); + kfree(init_qp_attr); /* 1 ref for caller and each rxmsg */ atomic_set(&conn->ibc_refcount, 1 + IBLND_RX_MSGS(conn)); @@ -828,7 +828,7 @@ struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, struct rdma_cm_id *cm failed_2: kiblnd_destroy_conn(conn, true); failed_1: - LIBCFS_FREE(init_qp_attr, sizeof(*init_qp_attr)); + kfree(init_qp_attr); failed_0: return NULL; } @@ -882,8 +882,7 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) IBLND_RX_MSGS(conn) * sizeof(struct kib_rx)); } - if (conn->ibc_connvars) - LIBCFS_FREE(conn->ibc_connvars, sizeof(*conn->ibc_connvars)); + kfree(conn->ibc_connvars); if (conn->ibc_hdev) kiblnd_hdev_decref(conn->ibc_hdev); @@ -897,7 +896,7 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) atomic_dec(&net->ibn_nconns); } - LIBCFS_FREE(conn, sizeof(*conn)); + kfree(conn); } int kiblnd_close_peer_conns_locked(struct kib_peer *peer, int why) @@ -1299,7 +1298,7 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo) frd_list) { list_del(&frd->frd_list); ib_dereg_mr(frd->frd_mr); - LIBCFS_FREE(frd, sizeof(*frd)); + kfree(frd); i++; } if (i < fpo->fast_reg.fpo_pool_size) @@ -1310,7 +1309,7 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo) if (fpo->fpo_hdev) kiblnd_hdev_decref(fpo->fpo_hdev); - LIBCFS_FREE(fpo, sizeof(*fpo)); + kfree(fpo); } static void kiblnd_destroy_fmr_pool_list(struct list_head *head) @@ -1405,14 +1404,14 @@ static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_po out_middle: if (frd->frd_mr) ib_dereg_mr(frd->frd_mr); - LIBCFS_FREE(frd, sizeof(*frd)); + kfree(frd); out: list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list, frd_list)
[PATCH 05/14] staging: lustre: change some LIBCFS_ALLOC calls to k?alloc(GFP_KERNEL)
When an allocation happens from process context rather than filesystem context, it is best to use GFP_KERNEL rather than LIBCFS_ALLOC() which always uses GFP_NOFS. This include initialization during, or prior to, mount, and code run from separate worker threads. So for some of these cases, switch to kmalloc, kvmalloc, or kvmalloc_array() as appropriate. In some cases we preserve __GFP_ZERO (via kzalloc/kvzalloc), but in others it is clear that allocated memory is immediately initialized. In each case, the matching LIBCFS_FREE() is converted to kfree() or kvfree() This is just a subset of locations that need changing. As there are quite a lot, I've broken them up into several ad-hoc sets to avoid review-fatigue. Signed-off-by: NeilBrown --- .../lustre/include/linux/libcfs/libcfs_string.h|4 ++-- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 11 -- .../staging/lustre/lnet/klnds/socklnd/socklnd.c| 22 drivers/staging/lustre/lnet/libcfs/hash.c | 18 +++- drivers/staging/lustre/lnet/libcfs/libcfs_mem.c|9 drivers/staging/lustre/lnet/libcfs/libcfs_string.c |2 +- drivers/staging/lustre/lnet/lnet/config.c |2 +- 7 files changed, 29 insertions(+), 39 deletions(-) diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h index bb95eaf9f3d5..66463477074a 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h @@ -85,11 +85,11 @@ static inline void cfs_expr_list_values_free(u32 *values, int num) { /* -* This array is allocated by LIBCFS_ALLOC(), so it shouldn't be freed +* This array is allocated by kvalloc(), so it shouldn't be freed * by OBD_FREE() if it's called by module other than libcfs & LNet, * otherwise we will see fake memory leak */ - LIBCFS_FREE(values, num * sizeof(values[0])); + kvfree(values); } void cfs_expr_list_free(struct cfs_expr_list *expr_list); diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 9fbf8a044962..bb7b19473e3a 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -2576,11 +2576,7 @@ static void kiblnd_base_shutdown(void) break; } - if (kiblnd_data.kib_peers) { - LIBCFS_FREE(kiblnd_data.kib_peers, - sizeof(struct list_head) * - kiblnd_data.kib_peer_hash_size); - } + kvfree(kiblnd_data.kib_peers); if (kiblnd_data.kib_scheds) cfs_percpt_free(kiblnd_data.kib_scheds); @@ -2672,8 +2668,9 @@ static int kiblnd_base_startup(void) INIT_LIST_HEAD(&kiblnd_data.kib_failed_devs); kiblnd_data.kib_peer_hash_size = IBLND_PEER_HASH_SIZE; - LIBCFS_ALLOC(kiblnd_data.kib_peers, -sizeof(struct list_head) * kiblnd_data.kib_peer_hash_size); + kiblnd_data.kib_peers = kvmalloc_array(kiblnd_data.kib_peer_hash_size, + sizeof(struct list_head), + GFP_KERNEL); if (!kiblnd_data.kib_peers) goto failed; for (i = 0; i < kiblnd_data.kib_peer_hash_size; i++) diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c index 51157ae14a9e..dc63ed2ceb97 100644 --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c @@ -1070,8 +1070,9 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, conn->ksnc_tx_carrier = NULL; atomic_set(&conn->ksnc_tx_nob, 0); - LIBCFS_ALLOC(hello, offsetof(struct ksock_hello_msg, -kshm_ips[LNET_MAX_INTERFACES])); + hello = kvzalloc(offsetof(struct ksock_hello_msg, + kshm_ips[LNET_MAX_INTERFACES]), +GFP_KERNEL); if (!hello) { rc = -ENOMEM; goto failed_1; @@ -1334,8 +1335,7 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, rc = ksocknal_send_hello(ni, conn, peerid.nid, hello); } - LIBCFS_FREE(hello, offsetof(struct ksock_hello_msg, - kshm_ips[LNET_MAX_INTERFACES])); + kvfree(hello); /* * setup the socket AFTER I've received hello (it disables @@ -1415,9 +1415,7 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, ksocknal_peer_decref(peer); failed_1: - if (hello) - LIBC
Re: [PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
On Mon, Jan 08 2018, Greg Kroah-Hartman wrote: > On Mon, Dec 18, 2017 at 11:46:30AM +1100, NeilBrown wrote: >> All usages of the form >> LIBCFS_ALLOC(variable, sizeof(variable)) >> or >> LIBCFS_ALLOC(variable, sizeof(variable's-type)) >> >> are changed to >> variable = kzalloc(sizeof(...), GFP_NOFS); >> >> Similarly, all >>LIBCFS_FREE(variable, sizeof(variable)) >> become >>kfree(variable); >> >> None of these need the vmalloc option, or any of the other minor >> benefits of LIBCFS_ALLOC(). >> >> Signed-off-by: NeilBrown > > As this broke the kbuild system, I'll stop here in this patch series. > Can you please fix it up, rebase and resend the remaining ones in this > series? Hi Greg, thanks for applying the patches. That patch had a typo in code that was only included if CONFIG_INFINIBAND is enabled, which I didn't have in my testing at the time. I hadn't resent when I fixed it as I was waiting to see if there would be feedback. If the protocol is "you apply sometime after two weeks if there is no feedback (and no breakage)", then I'll aim to work with that. I'll resend with the rest of the kmalloc patches. Thanks, NeilBrown signature.asc Description: PGP signature
Re: [PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
On Mon, Dec 18, 2017 at 11:46:30AM +1100, NeilBrown wrote: > All usages of the form > LIBCFS_ALLOC(variable, sizeof(variable)) > or > LIBCFS_ALLOC(variable, sizeof(variable's-type)) > > are changed to > variable = kzalloc(sizeof(...), GFP_NOFS); > > Similarly, all >LIBCFS_FREE(variable, sizeof(variable)) > become >kfree(variable); > > None of these need the vmalloc option, or any of the other minor > benefits of LIBCFS_ALLOC(). > > Signed-off-by: NeilBrown As this broke the kbuild system, I'll stop here in this patch series. Can you please fix it up, rebase and resend the remaining ones in this series? thanks, greg k-h
Re: [PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
Hi NeilBrown, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on next-20171221] [cannot apply to v4.15-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/NeilBrown/staging-lustre-convert-most-LIBCFS-ALLOC-to-k-malloc/20171220-113029 coccinelle warnings: (new ones prefixed by >>) >> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:886:2-7: WARNING: NULL >> check before freeing functions like kfree, debugfs_remove, >> debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider >> reorganizing relevant code to avoid passing NULL values. Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
Hi NeilBrown, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on next-20171220] [cannot apply to v4.15-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/NeilBrown/staging-lustre-convert-most-LIBCFS-ALLOC-to-k-malloc/20171220-113029 config: x86_64-randconfig-r0-12200451 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c: In function 'kiblnd_dev_failover': >> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:2395:2: error: 'kdev' >> undeclared (first use in this function) kdev = kzalloc(sizeof(*hdev), GFP_NOFS); ^~~~ drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:2395:2: note: each undeclared identifier is reported only once for each function it appears in vim +/kdev +2395 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 2329 2330 int kiblnd_dev_failover(struct kib_dev *dev) 2331 { 2332 LIST_HEAD(zombie_tpo); 2333 LIST_HEAD(zombie_ppo); 2334 LIST_HEAD(zombie_fpo); 2335 struct rdma_cm_id *cmid = NULL; 2336 struct kib_hca_dev *hdev = NULL; 2337 struct ib_pd *pd; 2338 struct kib_net *net; 2339 struct sockaddr_in addr; 2340 unsigned long flags; 2341 int rc = 0; 2342 int i; 2343 2344 LASSERT(*kiblnd_tunables.kib_dev_failover > 1 || 2345 dev->ibd_can_failover || !dev->ibd_hdev); 2346 2347 rc = kiblnd_dev_need_failover(dev); 2348 if (rc <= 0) 2349 goto out; 2350 2351 if (dev->ibd_hdev && 2352 dev->ibd_hdev->ibh_cmid) { 2353 /* 2354 * XXX it's not good to close old listener at here, 2355 * because we can fail to create new listener. 2356 * But we have to close it now, otherwise rdma_bind_addr 2357 * will return EADDRINUSE... How crap! 2358 */ 2359 write_lock_irqsave(&kiblnd_data.kib_global_lock, flags); 2360 2361 cmid = dev->ibd_hdev->ibh_cmid; 2362 /* 2363 * make next schedule of kiblnd_dev_need_failover() 2364 * return 1 for me 2365 */ 2366 dev->ibd_hdev->ibh_cmid = NULL; 2367 write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags); 2368 2369 rdma_destroy_id(cmid); 2370 } 2371 2372 cmid = kiblnd_rdma_create_id(kiblnd_cm_callback, dev, RDMA_PS_TCP, 2373 IB_QPT_RC); 2374 if (IS_ERR(cmid)) { 2375 rc = PTR_ERR(cmid); 2376 CERROR("Failed to create cmid for failover: %d\n", rc); 2377 goto out; 2378 } 2379 2380 memset(&addr, 0, sizeof(addr)); 2381 addr.sin_family = AF_INET; 2382 addr.sin_addr.s_addr = htonl(dev->ibd_ifip); 2383 addr.sin_port = htons(*kiblnd_tunables.kib_service); 2384 2385 /* Bind to failover device or port */ 2386 rc = rdma_bind_addr(cmid, (struct sockaddr *)&addr); 2387 if (rc || !cmid->device) { 2388 CERROR("Failed to bind %s:%pI4h to device(%p): %d\n", 2389 dev->ibd_ifname, &dev->ibd_ifip, 2390 cmid->device, rc); 2391 rdma_destroy_id(cmid); 2392 goto out; 2393 } 2394 > 2395 kdev = kzalloc(sizeof(*hdev), GFP_NOFS); 2396 if (!hdev) { 2397 CERROR("Failed to allocate kib_hca_dev\n"); 2398 rdma_destroy_id(cmid); 2399 rc = -ENOMEM; 2400 goto out; 2401 } 2402 2403 atomic_set(&hdev->ibh_ref, 1); 2404 hdev->ibh_dev = dev; 2405 hdev->ibh_cmid = cmid; 2406 hdev->ibh_ibdev = cmid->device; 2407 2408 pd = ib_alloc_pd(cmid->device, 0); 2409 if (IS_ERR(pd)) { 2410 rc = PTR_ERR(pd); 2411 CERROR("Can't allocate PD: %d\n", rc); 2412 goto out; 2413 } 2414 2415 hdev->ibh_pd = pd; 2416 2417 rc = rdma_listen(cmid, 0); 2418 if (rc) { 2419 CERROR("Can't start new listener: %d\n", rc); 2420 goto out; 2421 } 2422 2423 rc = kiblnd_hdev_get_attr(hdev); 2424 if (
Re: [PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
Hi NeilBrown, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on next-20171219] [cannot apply to v4.15-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/NeilBrown/staging-lustre-convert-most-LIBCFS-ALLOC-to-k-malloc/20171220-113029 config: i386-randconfig-x009-201751 (attached as .config) compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c: In function 'kiblnd_dev_failover': >> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:2395:2: error: 'kdev' >> undeclared (first use in this function); did you mean 'hdev'? kdev = kzalloc(sizeof(*hdev), GFP_NOFS); ^~~~ hdev drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c:2395:2: note: each undeclared identifier is reported only once for each function it appears in vim +2395 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 2329 2330 int kiblnd_dev_failover(struct kib_dev *dev) 2331 { 2332 LIST_HEAD(zombie_tpo); 2333 LIST_HEAD(zombie_ppo); 2334 LIST_HEAD(zombie_fpo); 2335 struct rdma_cm_id *cmid = NULL; 2336 struct kib_hca_dev *hdev = NULL; 2337 struct ib_pd *pd; 2338 struct kib_net *net; 2339 struct sockaddr_in addr; 2340 unsigned long flags; 2341 int rc = 0; 2342 int i; 2343 2344 LASSERT(*kiblnd_tunables.kib_dev_failover > 1 || 2345 dev->ibd_can_failover || !dev->ibd_hdev); 2346 2347 rc = kiblnd_dev_need_failover(dev); 2348 if (rc <= 0) 2349 goto out; 2350 2351 if (dev->ibd_hdev && 2352 dev->ibd_hdev->ibh_cmid) { 2353 /* 2354 * XXX it's not good to close old listener at here, 2355 * because we can fail to create new listener. 2356 * But we have to close it now, otherwise rdma_bind_addr 2357 * will return EADDRINUSE... How crap! 2358 */ 2359 write_lock_irqsave(&kiblnd_data.kib_global_lock, flags); 2360 2361 cmid = dev->ibd_hdev->ibh_cmid; 2362 /* 2363 * make next schedule of kiblnd_dev_need_failover() 2364 * return 1 for me 2365 */ 2366 dev->ibd_hdev->ibh_cmid = NULL; 2367 write_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags); 2368 2369 rdma_destroy_id(cmid); 2370 } 2371 2372 cmid = kiblnd_rdma_create_id(kiblnd_cm_callback, dev, RDMA_PS_TCP, 2373 IB_QPT_RC); 2374 if (IS_ERR(cmid)) { 2375 rc = PTR_ERR(cmid); 2376 CERROR("Failed to create cmid for failover: %d\n", rc); 2377 goto out; 2378 } 2379 2380 memset(&addr, 0, sizeof(addr)); 2381 addr.sin_family = AF_INET; 2382 addr.sin_addr.s_addr = htonl(dev->ibd_ifip); 2383 addr.sin_port = htons(*kiblnd_tunables.kib_service); 2384 2385 /* Bind to failover device or port */ 2386 rc = rdma_bind_addr(cmid, (struct sockaddr *)&addr); 2387 if (rc || !cmid->device) { 2388 CERROR("Failed to bind %s:%pI4h to device(%p): %d\n", 2389 dev->ibd_ifname, &dev->ibd_ifip, 2390 cmid->device, rc); 2391 rdma_destroy_id(cmid); 2392 goto out; 2393 } 2394 > 2395 kdev = kzalloc(sizeof(*hdev), GFP_NOFS); 2396 if (!hdev) { 2397 CERROR("Failed to allocate kib_hca_dev\n"); 2398 rdma_destroy_id(cmid); 2399 rc = -ENOMEM; 2400 goto out; 2401 } 2402 2403 atomic_set(&hdev->ibh_ref, 1); 2404 hdev->ibh_dev = dev; 2405 hdev->ibh_cmid = cmid; 2406 hdev->ibh_ibdev = cmid->device; 2407 2408 pd = ib_alloc_pd(cmid->device, 0); 2409 if (IS_ERR(pd)) { 2410 rc = PTR_ERR(pd); 2411 CERROR("Can't allocate PD: %d\n", rc); 2412 goto out; 2413 } 2414 2415 hdev->ibh_pd = pd; 2416 2417 rc = rdma_listen(cmid, 0); 2418 if (rc) { 2419 CERROR("Can't start new listener: %d\n", rc); 2420 goto out; 2421 } 2422 2423 rc = kiblnd_hdev_get_attr(hdev)
Re: [lustre-devel] [PATCH 08/15] staging: lustre: Convert more LIBCFS_ALLOC allocation to direct GFP_KERNEL
Sorry, it seems I mustn't have tested the file version of this patch. That "kvmalloc_node()" was clearly wrong. This version passes testing. Thanks, NeilBrown "git am" will ignore everything before this line: -8<- None of these need to be GFP_NOFS, so use GFP_KERNEL explicitly with kmalloc(), kvmalloc(), or kvmalloc_array(). Changing matching LIBCFS_FREE() to kfree() or kvfree() Signed-off-by: NeilBrown --- .../lustre/lnet/libcfs/linux/linux-module.c| 4 +-- drivers/staging/lustre/lnet/libcfs/module.c| 9 +++--- drivers/staging/lustre/lnet/lnet/api-ni.c | 17 +-- drivers/staging/lustre/lnet/lnet/config.c | 34 +- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c index b5746230ab31..ddf625669bff 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c @@ -146,7 +146,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return -EINVAL; } - LIBCFS_ALLOC(*hdr_pp, hdr.ioc_len); + *hdr_pp = kvmalloc(hdr.ioc_len, GFP_KERNEL); if (!*hdr_pp) return -ENOMEM; @@ -164,7 +164,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return 0; free: - LIBCFS_FREE(*hdr_pp, hdr.ioc_len); + kvfree(*hdr_pp); return err; } diff --git a/drivers/staging/lustre/lnet/libcfs/module.c b/drivers/staging/lustre/lnet/libcfs/module.c index 4ead55920e79..1bd33497a7c0 100644 --- a/drivers/staging/lustre/lnet/libcfs/module.c +++ b/drivers/staging/lustre/lnet/libcfs/module.c @@ -156,7 +156,7 @@ int libcfs_ioctl(unsigned long cmd, void __user *uparam) break; } } out: - LIBCFS_FREE(hdr, hdr->ioc_len); + kvfree(hdr); return err; } @@ -302,7 +302,7 @@ static int __proc_cpt_table(void *data, int write, LASSERT(cfs_cpt_table); while (1) { - LIBCFS_ALLOC(buf, len); + buf = kzalloc(len, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -311,7 +311,7 @@ static int __proc_cpt_table(void *data, int write, break; if (rc == -EFBIG) { - LIBCFS_FREE(buf, len); + kfree(buf); len <<= 1; continue; } @@ -325,8 +325,7 @@ static int __proc_cpt_table(void *data, int write, rc = cfs_trace_copyout_string(buffer, nob, buf + pos, NULL); out: - if (buf) - LIBCFS_FREE(buf, len); + kfree(buf); return rc; } diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index e8f623190133..6a1fb0397604 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -108,7 +108,8 @@ lnet_create_remote_nets_table(void) LASSERT(!the_lnet.ln_remote_nets_hash); LASSERT(the_lnet.ln_remote_nets_hbits > 0); - LIBCFS_ALLOC(hash, LNET_REMOTE_NETS_HASH_SIZE * sizeof(*hash)); + hash = kvmalloc_array(LNET_REMOTE_NETS_HASH_SIZE, sizeof(*hash), + GFP_KERNEL); if (!hash) { CERROR("Failed to create remote nets hash table\n"); return -ENOMEM; @@ -131,9 +132,7 @@ lnet_destroy_remote_nets_table(void) for (i = 0; i < LNET_REMOTE_NETS_HASH_SIZE; i++) LASSERT(list_empty(&the_lnet.ln_remote_nets_hash[i])); - LIBCFS_FREE(the_lnet.ln_remote_nets_hash, - LNET_REMOTE_NETS_HASH_SIZE * - sizeof(the_lnet.ln_remote_nets_hash[0])); + kvfree(the_lnet.ln_remote_nets_hash); the_lnet.ln_remote_nets_hash = NULL; } @@ -831,7 +830,7 @@ lnet_ping_info_create(int num_ni) unsigned int infosz; infosz = offsetof(struct lnet_ping_info, pi_ni[num_ni]); - LIBCFS_ALLOC(ping_info, infosz); + ping_info = kvzalloc(infosz, GFP_KERNEL); if (!ping_info) { CERROR("Can't allocate ping info[%d]\n", num_ni); return NULL; @@ -864,9 +863,7 @@ lnet_get_ni_count(void) static inline void lnet_ping_info_free(struct lnet_ping_info *pinfo) { - LIBCFS_FREE(pinfo, - offsetof(struct lnet_ping_info, -pi_ni[pinfo->pi_nnis])); + kvfree(pinfo); } static void @@ -2160,7 +2157,7 @@ static int lnet_ping(struct lnet_process_id id, int timeout_ms, if (id.pid == LNET_PID_ANY) id.pid = LNET_PID_LUSTRE; - LIBCFS_ALLOC(info, infosz); + info = kzalloc(infosz, GFP_KERNEL); if (!info) return -
[PATCH 15/15] staging: lustre: remove LIBCFS_ALLOC and LIBCFS_ALLOC_ATOMIC
These are no longer used. Signed-off-by: NeilBrown --- .../lustre/include/linux/libcfs/libcfs_private.h | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h index d230c7f7cced..35ed07ff7c71 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h @@ -85,25 +85,6 @@ do { \ } \ } while (0) -/** - * default allocator - */ -#define LIBCFS_ALLOC(ptr, size) \ -do { \ - LASSERT(!in_interrupt()); \ - (ptr) = kvmalloc((size), GFP_NOFS); \ - LIBCFS_ALLOC_POST((ptr), (size)); \ -} while (0) - -/** - * non-sleeping allocator - */ -#define LIBCFS_ALLOC_ATOMIC(ptr, size) \ -do { \ - (ptr) = kmalloc((size), GFP_ATOMIC);\ - LIBCFS_ALLOC_POST(ptr, size); \ -} while (0) - /** * allocate memory for specified CPU partition * \a cptab != NULL, \a cpt is CPU partition id of \a cptab
[PATCH 09/15] staging: lustre: more LIBCFS_ALLOC conversions to GFP_KERNEL allocations.
None of these need GFP_NOFS so allocate directly. Change matching LIBCFS_FREE() to kfree() or kvfree(). Signed-off-by: NeilBrown --- .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 19 +++ drivers/staging/lustre/lnet/lnet/lib-eq.c |9 - drivers/staging/lustre/lnet/lnet/lib-socket.c | 14 +++--- drivers/staging/lustre/lnet/selftest/conctl.c | 11 +-- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c index 44e44a999648..e9156bf05ed4 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c @@ -93,11 +93,7 @@ cfs_cpt_table_free(struct cfs_cpt_table *cptab) { int i; - if (cptab->ctb_cpu2cpt) { - LIBCFS_FREE(cptab->ctb_cpu2cpt, - num_possible_cpus() * - sizeof(cptab->ctb_cpu2cpt[0])); - } + kvfree(cptab->ctb_cpu2cpt); for (i = 0; cptab->ctb_parts && i < cptab->ctb_nparts; i++) { struct cfs_cpu_partition *part = &cptab->ctb_parts[i]; @@ -109,10 +105,7 @@ cfs_cpt_table_free(struct cfs_cpt_table *cptab) free_cpumask_var(part->cpt_cpumask); } - if (cptab->ctb_parts) { - LIBCFS_FREE(cptab->ctb_parts, - cptab->ctb_nparts * sizeof(cptab->ctb_parts[0])); - } + kvfree(cptab->ctb_parts); if (cptab->ctb_nodemask) kfree(cptab->ctb_nodemask); @@ -140,15 +133,17 @@ cfs_cpt_table_alloc(unsigned int ncpt) !cptab->ctb_nodemask) goto failed; - LIBCFS_ALLOC(cptab->ctb_cpu2cpt, -num_possible_cpus() * sizeof(cptab->ctb_cpu2cpt[0])); + cptab->ctb_cpu2cpt = kvmalloc_array(num_possible_cpus(), + sizeof(cptab->ctb_cpu2cpt[0]), + GFP_KERNEL); if (!cptab->ctb_cpu2cpt) goto failed; memset(cptab->ctb_cpu2cpt, -1, num_possible_cpus() * sizeof(cptab->ctb_cpu2cpt[0])); - LIBCFS_ALLOC(cptab->ctb_parts, ncpt * sizeof(cptab->ctb_parts[0])); + cptab->ctb_parts = kvmalloc_array(ncpt, sizeof(cptab->ctb_parts[0]), + GFP_KERNEL); if (!cptab->ctb_parts) goto failed; diff --git a/drivers/staging/lustre/lnet/lnet/lib-eq.c b/drivers/staging/lustre/lnet/lnet/lib-eq.c index 7a4d1f7a693e..a173b69e2f92 100644 --- a/drivers/staging/lustre/lnet/lnet/lib-eq.c +++ b/drivers/staging/lustre/lnet/lnet/lib-eq.c @@ -95,7 +95,8 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, return -ENOMEM; if (count) { - LIBCFS_ALLOC(eq->eq_events, count * sizeof(struct lnet_event)); + eq->eq_events = kvmalloc_array(count, sizeof(struct lnet_event), + GFP_KERNEL | __GFP_ZERO); if (!eq->eq_events) goto failed; /* @@ -132,8 +133,7 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, return 0; failed: - if (eq->eq_events) - LIBCFS_FREE(eq->eq_events, count * sizeof(struct lnet_event)); + kvfree(eq->eq_events); if (eq->eq_refs) cfs_percpt_free(eq->eq_refs); @@ -202,8 +202,7 @@ LNetEQFree(struct lnet_handle_eq eqh) lnet_eq_wait_unlock(); lnet_res_unlock(LNET_LOCK_EX); - if (events) - LIBCFS_FREE(events, size * sizeof(struct lnet_event)); + kvfree(events); if (refs) cfs_percpt_free(refs); diff --git a/drivers/staging/lustre/lnet/lnet/lib-socket.c b/drivers/staging/lustre/lnet/lnet/lib-socket.c index 539a26444f31..d3c35887598c 100644 --- a/drivers/staging/lustre/lnet/lnet/lib-socket.c +++ b/drivers/staging/lustre/lnet/lnet/lib-socket.c @@ -174,7 +174,7 @@ lnet_ipif_enumerate(char ***namesp) nalloc); } - LIBCFS_ALLOC(ifr, nalloc * sizeof(*ifr)); + ifr = kzalloc(nalloc * sizeof(*ifr), GFP_KERNEL); if (!ifr) { CERROR("ENOMEM enumerating up to %d interfaces\n", nalloc); @@ -199,14 +199,14 @@ lnet_ipif_enumerate(char ***namesp) if (nfound < nalloc || toobig) break; - LIBCFS_FREE(ifr, nalloc * sizeof(*ifr)); + kfree(ifr); nalloc *= 2; } if (!nfound) goto out1; - LIBCFS_ALLOC(names, nfound * sizeof(*names)); + names
[PATCH 08/15] staging: lustre: Convert more LIBCFS_ALLOC allocation to direct GFP_KERNEL
None of these need to be GFP_NOFS, so use GFP_KERNEL explicitly with kmalloc(), kvmalloc(), or kvmalloc_array(). Changing matching LIBCFS_FREE() to kfree() or kvfree() Signed-off-by: NeilBrown --- .../lustre/lnet/libcfs/linux/linux-module.c|4 +- drivers/staging/lustre/lnet/libcfs/module.c|9 ++--- drivers/staging/lustre/lnet/lnet/api-ni.c | 17 -- drivers/staging/lustre/lnet/lnet/config.c | 34 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c index b5746230ab31..ddf625669bff 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-module.c @@ -146,7 +146,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return -EINVAL; } - LIBCFS_ALLOC(*hdr_pp, hdr.ioc_len); + *hdr_pp = kvmalloc(hdr.ioc_len, GFP_KERNEL); if (!*hdr_pp) return -ENOMEM; @@ -164,7 +164,7 @@ int libcfs_ioctl_getdata(struct libcfs_ioctl_hdr **hdr_pp, return 0; free: - LIBCFS_FREE(*hdr_pp, hdr.ioc_len); + kvfree(*hdr_pp); return err; } diff --git a/drivers/staging/lustre/lnet/libcfs/module.c b/drivers/staging/lustre/lnet/libcfs/module.c index 4ead55920e79..ff4b0cec1bbe 100644 --- a/drivers/staging/lustre/lnet/libcfs/module.c +++ b/drivers/staging/lustre/lnet/libcfs/module.c @@ -156,7 +156,7 @@ int libcfs_ioctl(unsigned long cmd, void __user *uparam) break; } } out: - LIBCFS_FREE(hdr, hdr->ioc_len); + kvfree(hdr); return err; } @@ -302,7 +302,7 @@ static int __proc_cpt_table(void *data, int write, LASSERT(cfs_cpt_table); while (1) { - LIBCFS_ALLOC(buf, len); + buf = kmalloc(len, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -311,7 +311,7 @@ static int __proc_cpt_table(void *data, int write, break; if (rc == -EFBIG) { - LIBCFS_FREE(buf, len); + kfree(buf); len <<= 1; continue; } @@ -325,8 +325,7 @@ static int __proc_cpt_table(void *data, int write, rc = cfs_trace_copyout_string(buffer, nob, buf + pos, NULL); out: - if (buf) - LIBCFS_FREE(buf, len); + kfree(buf); return rc; } diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c index e8f623190133..f201d2d1eb02 100644 --- a/drivers/staging/lustre/lnet/lnet/api-ni.c +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c @@ -108,7 +108,8 @@ lnet_create_remote_nets_table(void) LASSERT(!the_lnet.ln_remote_nets_hash); LASSERT(the_lnet.ln_remote_nets_hbits > 0); - LIBCFS_ALLOC(hash, LNET_REMOTE_NETS_HASH_SIZE * sizeof(*hash)); + hash = kvmalloc_node(LNET_REMOTE_NETS_HASH_SIZE, sizeof(*hash), +GFP_KERNEL); if (!hash) { CERROR("Failed to create remote nets hash table\n"); return -ENOMEM; @@ -131,9 +132,7 @@ lnet_destroy_remote_nets_table(void) for (i = 0; i < LNET_REMOTE_NETS_HASH_SIZE; i++) LASSERT(list_empty(&the_lnet.ln_remote_nets_hash[i])); - LIBCFS_FREE(the_lnet.ln_remote_nets_hash, - LNET_REMOTE_NETS_HASH_SIZE * - sizeof(the_lnet.ln_remote_nets_hash[0])); + kvfree(the_lnet.ln_remote_nets_hash); the_lnet.ln_remote_nets_hash = NULL; } @@ -831,7 +830,7 @@ lnet_ping_info_create(int num_ni) unsigned int infosz; infosz = offsetof(struct lnet_ping_info, pi_ni[num_ni]); - LIBCFS_ALLOC(ping_info, infosz); + ping_info = kvzalloc(infosz, GFP_KERNEL); if (!ping_info) { CERROR("Can't allocate ping info[%d]\n", num_ni); return NULL; @@ -864,9 +863,7 @@ lnet_get_ni_count(void) static inline void lnet_ping_info_free(struct lnet_ping_info *pinfo) { - LIBCFS_FREE(pinfo, - offsetof(struct lnet_ping_info, -pi_ni[pinfo->pi_nnis])); + kvfree(pinfo); } static void @@ -2160,7 +2157,7 @@ static int lnet_ping(struct lnet_process_id id, int timeout_ms, if (id.pid == LNET_PID_ANY) id.pid = LNET_PID_LUSTRE; - LIBCFS_ALLOC(info, infosz); + info = kzalloc(infosz, GFP_KERNEL); if (!info) return -ENOMEM; @@ -2310,6 +2307,6 @@ static int lnet_ping(struct lnet_process_id id, int timeout_ms, LASSERT(!rc2); out_0: - LIBCFS_FREE(info, infosz); + kfree(info); return rc; } diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/
[PATCH 07/15] staging: lustre: change some LIBCFS_ALLOC calls to k?alloc(GFP_KERNEL)
When an allocation happens from process context rather than filesystem context, it is best to use GFP_KERNEL rather than LIBCFS_ALLOC() which always uses GFP_NOFS. This include initialization during, or prior to, mount, and code run from separate worker threads. So for some of these cases, switch to kmalloc, kvmalloc, or kvmalloc_array() as appropriate. In some cases we preserve __GFP_ZERO (via kzalloc/kvzalloc), but in others it is clear that allocated memory is immediately initialized. In each case, the matching LIBCFS_FREE() is converted to kfree() or kvfree() This is just a subset of location that need changing. As there are quite a lot, I've broken them up into several ad-hoc sets to avoid review-fatigue. Signed-off-by: NeilBrown --- .../lustre/include/linux/libcfs/libcfs_string.h|4 ++-- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 11 -- .../staging/lustre/lnet/klnds/socklnd/socklnd.c| 22 drivers/staging/lustre/lnet/libcfs/hash.c | 18 +++- drivers/staging/lustre/lnet/libcfs/libcfs_mem.c|9 drivers/staging/lustre/lnet/libcfs/libcfs_string.c |2 +- drivers/staging/lustre/lnet/lnet/config.c |2 +- 7 files changed, 29 insertions(+), 39 deletions(-) diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h index 1191764c431a..c1375733ff31 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_string.h @@ -86,11 +86,11 @@ static inline void cfs_expr_list_values_free(u32 *values, int num) { /* -* This array is allocated by LIBCFS_ALLOC(), so it shouldn't be freed +* This array is allocated by kvalloc(), so it shouldn't be freed * by OBD_FREE() if it's called by module other than libcfs & LNet, * otherwise we will see fake memory leak */ - LIBCFS_FREE(values, num * sizeof(values[0])); + kvfree(values); } void cfs_expr_list_free(struct cfs_expr_list *expr_list); diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index f9bdf1877794..f59c20a870a8 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -2577,11 +2577,7 @@ static void kiblnd_base_shutdown(void) break; } - if (kiblnd_data.kib_peers) { - LIBCFS_FREE(kiblnd_data.kib_peers, - sizeof(struct list_head) * - kiblnd_data.kib_peer_hash_size); - } + kvfree(kiblnd_data.kib_peers); if (kiblnd_data.kib_scheds) cfs_percpt_free(kiblnd_data.kib_scheds); @@ -2673,8 +2669,9 @@ static int kiblnd_base_startup(void) INIT_LIST_HEAD(&kiblnd_data.kib_failed_devs); kiblnd_data.kib_peer_hash_size = IBLND_PEER_HASH_SIZE; - LIBCFS_ALLOC(kiblnd_data.kib_peers, -sizeof(struct list_head) * kiblnd_data.kib_peer_hash_size); + kiblnd_data.kib_peers = kvmalloc_array(kiblnd_data.kib_peer_hash_size, + sizeof(struct list_head), + GFP_KERNEL); if (!kiblnd_data.kib_peers) goto failed; for (i = 0; i < kiblnd_data.kib_peer_hash_size; i++) diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c index 51157ae14a9e..dc63ed2ceb97 100644 --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c @@ -1070,8 +1070,9 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, conn->ksnc_tx_carrier = NULL; atomic_set(&conn->ksnc_tx_nob, 0); - LIBCFS_ALLOC(hello, offsetof(struct ksock_hello_msg, -kshm_ips[LNET_MAX_INTERFACES])); + hello = kvzalloc(offsetof(struct ksock_hello_msg, + kshm_ips[LNET_MAX_INTERFACES]), +GFP_KERNEL); if (!hello) { rc = -ENOMEM; goto failed_1; @@ -1334,8 +1335,7 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, rc = ksocknal_send_hello(ni, conn, peerid.nid, hello); } - LIBCFS_FREE(hello, offsetof(struct ksock_hello_msg, - kshm_ips[LNET_MAX_INTERFACES])); + kvfree(hello); /* * setup the socket AFTER I've received hello (it disables @@ -1415,9 +1415,7 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route, ksocknal_peer_decref(peer); failed_1: - if (hello) - LIBC
[PATCH 03/15] staging: lustre: replace simple cases of LIBCFS_ALLOC with kzalloc.
All usages of the form LIBCFS_ALLOC(variable, sizeof(variable)) or LIBCFS_ALLOC(variable, sizeof(variable's-type)) are changed to variable = kzalloc(sizeof(...), GFP_NOFS); Similarly, all LIBCFS_FREE(variable, sizeof(variable)) become kfree(variable); None of these need the vmalloc option, or any of the other minor benefits of LIBCFS_ALLOC(). Signed-off-by: NeilBrown --- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 38 ++-- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |4 +- .../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c |4 +- .../staging/lustre/lnet/klnds/socklnd/socklnd.c| 22 ++-- .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |2 + .../lustre/lnet/klnds/socklnd/socklnd_proto.c |8 ++-- drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c|4 +- drivers/staging/lustre/lnet/libcfs/libcfs_lock.c |6 ++- drivers/staging/lustre/lnet/libcfs/libcfs_string.c | 10 +++-- .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 15 drivers/staging/lustre/lnet/libcfs/workitem.c |8 ++-- drivers/staging/lustre/lnet/lnet/api-ni.c |4 +- drivers/staging/lustre/lnet/lnet/config.c |6 ++- drivers/staging/lustre/lnet/lnet/lib-move.c|6 ++- drivers/staging/lustre/lnet/lnet/nidstrings.c |8 ++-- drivers/staging/lustre/lnet/lnet/peer.c|2 + drivers/staging/lustre/lnet/lnet/router.c | 26 ++ drivers/staging/lustre/lnet/lnet/router_proc.c |6 ++- drivers/staging/lustre/lnet/selftest/conrpc.c | 10 +++-- drivers/staging/lustre/lnet/selftest/console.c | 26 +++--- drivers/staging/lustre/lnet/selftest/framework.c | 22 ++-- drivers/staging/lustre/lnet/selftest/rpc.c | 12 +++--- 22 files changed, 124 insertions(+), 125 deletions(-) diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 8024843521ab..039f53d787e4 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -367,7 +367,7 @@ void kiblnd_destroy_peer(struct kib_peer *peer) LASSERT(kiblnd_peer_idle(peer)); LASSERT(list_empty(&peer->ibp_tx_queue)); - LIBCFS_FREE(peer, sizeof(*peer)); + kfree(peer); /* * NB a peer's connections keep a reference on their peer until @@ -776,7 +776,7 @@ struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, struct rdma_cm_id *cm goto failed_2; } - LIBCFS_FREE(init_qp_attr, sizeof(*init_qp_attr)); + kfree(init_qp_attr); /* 1 ref for caller and each rxmsg */ atomic_set(&conn->ibc_refcount, 1 + IBLND_RX_MSGS(conn)); @@ -828,7 +828,7 @@ struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, struct rdma_cm_id *cm failed_2: kiblnd_destroy_conn(conn, true); failed_1: - LIBCFS_FREE(init_qp_attr, sizeof(*init_qp_attr)); + kfree(init_qp_attr); failed_0: return NULL; } @@ -883,7 +883,7 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) } if (conn->ibc_connvars) - LIBCFS_FREE(conn->ibc_connvars, sizeof(*conn->ibc_connvars)); + kfree(conn->ibc_connvars); if (conn->ibc_hdev) kiblnd_hdev_decref(conn->ibc_hdev); @@ -897,7 +897,7 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) atomic_dec(&net->ibn_nconns); } - LIBCFS_FREE(conn, sizeof(*conn)); + kfree(conn); } int kiblnd_close_peer_conns_locked(struct kib_peer *peer, int why) @@ -1299,7 +1299,7 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo) frd_list) { list_del(&frd->frd_list); ib_dereg_mr(frd->frd_mr); - LIBCFS_FREE(frd, sizeof(*frd)); + kfree(frd); i++; } if (i < fpo->fast_reg.fpo_pool_size) @@ -1310,7 +1310,7 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo) if (fpo->fpo_hdev) kiblnd_hdev_decref(fpo->fpo_hdev); - LIBCFS_FREE(fpo, sizeof(*fpo)); + kfree(fpo); } static void kiblnd_destroy_fmr_pool_list(struct list_head *head) @@ -1405,14 +1405,14 @@ static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_po out_middle: if (frd->frd_mr) ib_dereg_mr(frd->frd_mr); - LIBCFS_FREE(frd, sizeof(*frd)); + kfree(frd); out: list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list, frd_list) { list_del(&frd->frd_list); ib_dereg
[PATCH 07/10] staging: lustre: do not memset after LIBCFS_ALLOC
From: Frank Zago LIBCFS_ALLOC already zero out the memory allocated, so there is no need to zero out the memory again. Signed-off-by: Frank Zago Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5304 Reviewed-on: http://review.whamcloud.com/11012 Reviewed-by: Patrick Farrell Reviewed-by: James Simmons Reviewed-by: Andreas Dilger Reviewed-by: Dmitry Eremin Reviewed-by: Oleg Drokin --- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 2e7b5ca..98570b3 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -339,8 +339,6 @@ int kiblnd_create_peer(lnet_ni_t *ni, kib_peer_t **peerp, lnet_nid_t nid) return -ENOMEM; } - memset(peer, 0, sizeof(*peer)); /* zero flags etc */ - peer->ibp_ni = ni; peer->ibp_nid = nid; peer->ibp_error = 0; -- 1.7.1
[PATCH 08/40] staging: lustre: do not memset after LIBCFS_ALLOC
From: Frank Zago LIBCFS_ALLOC already zero out the memory allocated, so there is no need to zero out the memory again. Signed-off-by: Frank Zago Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5304 Reviewed-on: http://review.whamcloud.com/11012 Reviewed-by: Patrick Farrell Reviewed-by: James Simmons Reviewed-by: Andreas Dilger Reviewed-by: Dmitry Eremin Reviewed-by: Oleg Drokin --- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 0f4154c..f3cbc3b 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -335,8 +335,6 @@ int kiblnd_create_peer(lnet_ni_t *ni, kib_peer_t **peerp, lnet_nid_t nid) return -ENOMEM; } - memset(peer, 0, sizeof(*peer)); /* zero flags etc */ - peer->ibp_ni = ni; peer->ibp_nid = nid; peer->ibp_error = 0; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lustre-devel] LIBCFS_ALLOC
On 2015/07/02, 4:25 PM, "Simmons, James A." wrote: > >>> >Yeah. You're right. Doing a vmalloc() when kmalloc() doesn't have >>>even >>> >a tiny sliver of RAM isn't going to work. It's easier to use >>> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing. >>> >>> The original reason we have the vmalloc water mark wasn't so much the >>> issue of memory exhaustion but to handle the case of memory >>>fragmentation. >>> Some sites had after a extended period of time started to see failures >>>of >>> allocating even 32K using kmalloc. In our latest development branch >>>we moved >>> away from using a water mark to always try kmalloc first and if it >>>fails then we >>> try vmalloc. At ORNL we ran into severe performance issues when we >>>entered >>> vmalloc territory. It has been discussed before on what might replace >>>vmalloc >>> handling in the case of kmalloc fails but no solution has been worked >>>out. >> >>OK, but if a structure contains only 4 words, would it be better to just >>use kzalloc? Or does it not matter? It would only save trying vmalloc >>in >>a case that it is guaranteed to fail, but if a structure with 4 words >>can't be allocated, the system has other problems. Another argument is >>that kzalloc is a well known function that people and bug-finding tools >>understand, so it is better to use it whenever possible. >> >>Some of the other structures contain a lot more fields, as well as small >>arrays. They are probably acceptable for kzalloc too, but I wouldn't >>know >>the exact dividing line. > >The reason I bring this up is to discuss sorting this out. Once long ago >we had just LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got > spawned off of that. Currently LIBCFS_ALLOC is used just by the >libcfs/LNet > layer. That is because there was (is?) interest from Cray and others to use LNet independently from Lustre (Zest and DVS, for example) so LNet should be self-contained and not depend on anything from Lustre. > Now OBD_ALLOC in our development branch has moved to a try kmalloc first >and >if it fails try vmalloc for any size memory allocation. LIBCFS_ALLOC >still > does the original approach. So we have two possible solutions >depending on if libcfs/LNet needs to ever do a vmalloc. > >One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC >and replace it with kzalloc everywhere. We can then move libcfs_kzvalloc >to > the lustre layer and port the change we did in the development branch to > here of the try kmalloc then vmalloc approach. > >The other approach is if libcfs/LNet does in some case need to use vmalloc > we could then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once > this is implemented we can nuke the OBD_ALLOC system. I don't agree. I think there are a few places where vmalloc() makes sense to try (if the allocation may be large), but in most places LIBCFS_ALLOC() should only use kmalloc(). Unfortunately, there wasn't a separate LIBCFS_ALLOC_LARGE() like there was for OBD_ALLOC_LARGE() that made it clear which callsites are (potentially) large and which are small. The macro approach allowed the compile-time optimization of the small callsites, but that needs to be done by hand now. >Either way I like to see it consolidated down to one system. Given the proliferation of foo_kvmalloc() and foo_kvzalloc() helpers (ext4_, kvm_, dm_, apparmor, ceph_, __aa_), maybe it it is time to move these to common kernel code instead of introducing yet another new one? Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel High Performance Data Division -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [lustre-devel] LIBCFS_ALLOC
>> >Yeah. You're right. Doing a vmalloc() when kmalloc() doesn't have even >> >a tiny sliver of RAM isn't going to work. It's easier to use >> >libcfs_kvzalloc() everywhere, but it's probably the wrong thing. >> >> The original reason we have the vmalloc water mark wasn't so much the >> issue of memory exhaustion but to handle the case of memory fragmentation. >> Some sites had after a extended period of time started to see failures of >> allocating even 32K using kmalloc. In our latest development branch we moved >> away from using a water mark to always try kmalloc first and if it fails >> then we >> try vmalloc. At ORNL we ran into severe performance issues when we entered >> vmalloc territory. It has been discussed before on what might replace vmalloc >> handling in the case of kmalloc fails but no solution has been worked out. > >OK, but if a structure contains only 4 words, would it be better to just >use kzalloc? Or does it not matter? It would only save trying vmalloc in >a case that it is guaranteed to fail, but if a structure with 4 words >can't be allocated, the system has other problems. Another argument is >that kzalloc is a well known function that people and bug-finding tools >understand, so it is better to use it whenever possible. > >Some of the other structures contain a lot more fields, as well as small >arrays. They are probably acceptable for kzalloc too, but I wouldn't know >the exact dividing line. The reason I bring this up is to discuss sorting this out. Once long ago we had just LIBCFS_ALLOC. For some reason before my time OBD_ALLOC got spawned off of that. Currently LIBCFS_ALLOC is used just by the libcfs/LNet layer. Now OBD_ALLOC in our development branch has moved to a try kmalloc first and if it fails try vmalloc for any size memory allocation. LIBCFS_ALLOC still does the original approach. So we have two possible solutions depending on if libcfs/LNet needs to ever do a vmalloc. One solution if libcfs/LNet never needs a vmalloc is remove LIBCFS_ALLOC and replace it with kzalloc everywhere. We can then move libcfs_kzvalloc to the lustre layer and port the change we did in the development branch to here of the try kmalloc then vmalloc approach. The other approach is if libcfs/LNet does in some case need to use vmalloc we could then update LIBCFS_ALLOC to first try kmalloc then vmalloc. Once this is implemented we can nuke the OBD_ALLOC system. Either way I like to see it consolidated down to one system. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lustre-devel] LIBCFS_ALLOC
On 2015/06/28, 12:52 AM, "Julia Lawall" wrote: >It is not clear that all of the uses of LIBCFS_ALLOC really risk needing >vmalloc. For example: > >lnet/klnds/socklnd/socklnd.c, function ksocknal_accept: > >ksock_connreq_t *cr; >... >LIBCFS_ALLOC(cr, sizeof(*cr)); > >The definition of ksock_connreq_t is: > >typedef struct ksock_connreq { >struct list_head ksncr_list; /* stash on ksnd_connd_connreqs */ >lnet_ni_t*ksncr_ni; /* chosen NI */ >struct socket*ksncr_sock; /* accepted socket */ >} ksock_connreq_t; > >This looks like a very small structure. > >LIBCFS_ALLOC relies on a test on the size, which should be able to be >compiled away. libcfs_kvzalloc on the other hand relies on the failure >of >kmalloc and so the test for that won't be compiled away. There are probably only a handful of places where trying vmalloc() even makes sense. In most cases, LIBCFS_ALLOC() can be replaced by a straight call to kmalloc() because the allocation size is small enough, and only a few need to use libcfs_kvzalloc(). Anything at PAGE_SIZE or less will either work with kmalloc() or it won't work at all. I do agree with James' comments that vmalloc() was needed for both very large allocations that can't be satisfied by kmalloc() at all, as well as smaller allocations (anything over a couple of pages) due to fragmentation of free pages after running for a long time. I think libcfs_kvzalloc() is at least as good as the size-based threshold used previously, since using kmalloc() is going to be faster than vmalloc() and would work better on 32-bit platforms with limited vmalloc() size. Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel High Performance Data Division -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: LIBCFS_ALLOC
All that you are saying is true and stuff that Julia and I have discussed before. For this call site though we are not allocating 32k, we're allocating 4 pointers so libcfs_kvzalloc() doesn't make sense. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: LIBCFS_ALLOC
On Tue, 30 Jun 2015, Simmons, James A. wrote: > >Yeah. You're right. Doing a vmalloc() when kmalloc() doesn't have even > >a tiny sliver of RAM isn't going to work. It's easier to use > >libcfs_kvzalloc() everywhere, but it's probably the wrong thing. > > The original reason we have the vmalloc water mark wasn't so much the > issue of memory exhaustion but to handle the case of memory fragmentation. > Some sites had after a extended period of time started to see failures of > allocating even 32K using kmalloc. In our latest development branch we moved > away from using a water mark to always try kmalloc first and if it fails then > we > try vmalloc. At ORNL we ran into severe performance issues when we entered > vmalloc territory. It has been discussed before on what might replace vmalloc > handling in the case of kmalloc fails but no solution has been worked out. OK, but if a structure contains only 4 words, would it be better to just use kzalloc? Or does it not matter? It would only save trying vmalloc in a case that it is guaranteed to fail, but if a structure with 4 words can't be allocatted, the system has other problems. Another argument is that kzalloc is a well known function that people and bug-finding tools understand, so it is better to use it whenever possible. Some of the other structures contain a lot more fields, as well as small arrays. They are probably acceptable for kzalloc too, but I wouldn't know the exact dividing line. julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: LIBCFS_ALLOC
>Yeah. You're right. Doing a vmalloc() when kmalloc() doesn't have even >a tiny sliver of RAM isn't going to work. It's easier to use >libcfs_kvzalloc() everywhere, but it's probably the wrong thing. The original reason we have the vmalloc water mark wasn't so much the issue of memory exhaustion but to handle the case of memory fragmentation. Some sites had after a extended period of time started to see failures of allocating even 32K using kmalloc. In our latest development branch we moved away from using a water mark to always try kmalloc first and if it fails then we try vmalloc. At ORNL we ran into severe performance issues when we entered vmalloc territory. It has been discussed before on what might replace vmalloc handling in the case of kmalloc fails but no solution has been worked out. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: LIBCFS_ALLOC
Yeah. You're right. Doing a vmalloc() when kmalloc() doesn't have even a tiny sliver of RAM isn't going to work. It's easier to use libcfs_kvzalloc() everywhere, but it's probably the wrong thing. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
LIBCFS_ALLOC
It is not clear that all of the uses of LIBCFS_ALLOC really risk needing vmalloc. For example: lnet/klnds/socklnd/socklnd.c, function ksocknal_accept: ksock_connreq_t *cr; ... LIBCFS_ALLOC(cr, sizeof(*cr)); The definition of ksock_connreq_t is: typedef struct ksock_connreq { struct list_head ksncr_list; /* stash on ksnd_connd_connreqs */ lnet_ni_t*ksncr_ni; /* chosen NI */ struct socket*ksncr_sock; /* accepted socket */ } ksock_connreq_t; This looks like a very small structure. LIBCFS_ALLOC relies on a test on the size, which should be able to be compiled away. libcfs_kvzalloc on the other hand relies on the failure of kmalloc and so the test for that won't be compiled away. Does it matter? julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: lustre: LIBCFS_ALLOC
On Jun 23, 2015, at 2:23 AM, Julia Lawall wrote: > It seems that libcfs_kvzalloc doesn't use any particular threshold or > switchingbetween kzalloc and vmalloc, so can be replaced by ths function > too? If you mean to replace all instances of LIBCFS_ALLOC with libcfs_kvzalloc (and all frees with kvfree) then yes, that could be done I think. Bye, Oleg-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
lustre: LIBCFS_ALLOC
It seems that libcfs_kvzalloc doesn't use any particular threshold or switchingbetween kzalloc and vmalloc, so can be replaced by ths function too? thanks, julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: remove memset(0) after LIBCFS_ALLOC
On Sat, Jun 14, 2014 at 05:29:51PM +1000, Vitaly Osipov wrote: > Joe Perches mentioned on driverdev-devel that memset after LIBCFS_ALLOC > is not necessary as it is already done during LIBCFS_ALLOC_POST. This > commit removes these unnecessary memsets. Based on the results of running > a cocci patch along the lines of: > > @@ > expression E1, E2; > @@ > > LIBCFS_ALLOC (E1,E2); > ... > - memset(E1,0,E2); > > Signed-off-by: Vitaly Osipov Reviewed-by: Dan Carpenter regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: lustre: remove memset(0) after LIBCFS_ALLOC
Joe Perches mentioned on driverdev-devel that memset after LIBCFS_ALLOC is not necessary as it is already done during LIBCFS_ALLOC_POST. This commit removes these unnecessary memsets. Based on the results of running a cocci patch along the lines of: @@ expression E1, E2; @@ LIBCFS_ALLOC (E1,E2); ... - memset(E1,0,E2); Signed-off-by: Vitaly Osipov --- drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 5 - drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 4 drivers/staging/lustre/lnet/lnet/router.c| 1 - drivers/staging/lustre/lnet/selftest/console.c | 10 -- drivers/staging/lustre/lnet/selftest/framework.c | 2 -- drivers/staging/lustre/lustre/libcfs/linux/linux-tcpip.c | 2 -- 6 files changed, 24 deletions(-) diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index 892c419..1a4c9e6 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -2430,8 +2430,6 @@ kiblnd_hdev_setup_mrs(kib_hca_dev_t *hdev) return -ENOMEM; } - memset(hdev->ibh_mrs, 0, sizeof(*hdev->ibh_mrs) * hdev->ibh_nmrs); - for (i = 0; i < hdev->ibh_nmrs; i++) { struct ib_phys_buf ipb; __u64 iova; @@ -2704,7 +2702,6 @@ kiblnd_create_dev(char *ifname) if (dev == NULL) return NULL; - memset(dev, 0, sizeof(*dev)); netdev = dev_get_by_name(&init_net, ifname); if (netdev == NULL) { dev->ibd_can_failover = 0; @@ -3088,8 +3085,6 @@ kiblnd_startup (lnet_ni_t *ni) if (net == NULL) goto failed; - memset(net, 0, sizeof(*net)); - do_gettimeofday(&tv); net->ibn_incarnation = (((__u64)tv.tv_sec) * 100) + tv.tv_usec; diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c index 775dcd2..f528b65 100644 --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c @@ -113,8 +113,6 @@ ksocknal_create_peer (ksock_peer_t **peerp, lnet_ni_t *ni, lnet_process_id_t id) if (peer == NULL) return -ENOMEM; - memset (peer, 0, sizeof (*peer)); /* NULL pointers/clear flags etc */ - peer->ksnp_ni = ni; peer->ksnp_id = id; atomic_set (&peer->ksnp_refcount, 1); /* 1 ref for caller */ @@ -1040,8 +1038,6 @@ ksocknal_create_conn (lnet_ni_t *ni, ksock_route_t *route, goto failed_0; } - memset (conn, 0, sizeof (*conn)); - conn->ksnc_peer = NULL; conn->ksnc_route = NULL; conn->ksnc_sock = sock; diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c index 926923a..1359b11 100644 --- a/drivers/staging/lustre/lnet/lnet/router.c +++ b/drivers/staging/lustre/lnet/lnet/router.c @@ -866,7 +866,6 @@ lnet_create_rc_data_locked(lnet_peer_t *gateway) if (pi == NULL) goto out; - memset(pi, 0, LNET_PINGINFO_SIZE); for (i = 0; i < LNET_MAX_RTR_NIS; i++) { pi->pi_ni[i].ns_nid = LNET_NID_ANY; pi->pi_ni[i].ns_status = LNET_NI_STATUS_INVALID; diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c index 352fc96..2aeebbf 100644 --- a/drivers/staging/lustre/lnet/selftest/console.c +++ b/drivers/staging/lustre/lnet/selftest/console.c @@ -204,9 +204,6 @@ lstcon_group_alloc(char *name, lstcon_group_t **grpp) if (grp == NULL) return -ENOMEM; - memset(grp, 0, offsetof(lstcon_group_t, - grp_ndl_hash[LST_NODE_HASHSIZE])); - grp->grp_ref = 1; if (name != NULL) strcpy(grp->grp_name, name); @@ -815,8 +812,6 @@ lstcon_group_info(char *name, lstcon_ndlist_ent_t *gents_p, return -ENOMEM; } - memset(gentp, 0, sizeof(lstcon_ndlist_ent_t)); - list_for_each_entry(ndl, &grp->grp_ndl_list, ndl_link) LST_NODE_STATE_COUNTER(ndl->ndl_node, gentp); @@ -971,8 +966,6 @@ lstcon_batch_info(char *name, lstcon_test_batch_ent_t *ent_up, int server, if (entp == NULL) return -ENOMEM; - memset(entp, 0, sizeof(lstcon_test_batch_ent_t)); - if (test == NULL) { entp->u.tbe_batch.bae_ntest = bat->bat_ntest; entp->u.tbe_batch.bae_state = bat->bat_state; @@ -1319,7 +1312,6 @@ lstcon_test_add(char *batch_name, int type, int loop, goto out; } - memset(test, 0, offsetof(lstcon_test_t, tes_param[paramlen])); test->tes_hdr.tsb_id= batch->bat_hdr.tsb_id;