Linus Torvalds <[email protected]> writes: > On Sat, Jul 30, 2016 at 7:23 AM, Michael Ellerman <[email protected]> wrote: >> #ifdef CONFIG_NUMA >> - pool = kmalloc(num_nodes * sizeof(void *), >> + pool = kmalloc(nr_node_ids * sizeof(void *), >> GFP_KERNEL|__GFP_NOFAIL|__GFP_ZERO); >> for_each_online_node(i) { >> crng = kmalloc_node(sizeof(struct crng_state), > > Ugh. Can we please also just change that kmalloc to kcalloc()? Get rid > of the odd multiplication and the unusual GFP mask bit crud? > > And instead of using "sizeof(void *)", just use the pool entry size, > ie "sizeof(*pool)". Yes, we have other places where we depend on void > pointers having the same size as others, but it's the RightThing(tm) > to do anyway, and it makes more sense when you grep things ("Oh, we're > allocating 'nr_node_id' copes of *pool entries" even without knowing > what type is behind the "pool" pointer). > > IOW, can you confirm that you could just use > > pool = kcalloc(nr_node_ids, sizeof(*pool), GFP_KERNEL); > > instead? I'd much rather apply that patch.
Dropping NOFAIL means we need to handle allocation failures, which makes the patch a bit bigger, and less of a pure fix. Here's a separate patch to do those cleanups, which you can squash with the first if you prefer. I did test the allocation failure case for both the whole pool and individual nodes. cheers >From 2f5ab5fa2b9e4997fe01053fc12f689fb2117f45 Mon Sep 17 00:00:00 2001 From: Michael Ellerman <[email protected]> Date: Sun, 31 Jul 2016 12:54:40 +1000 Subject: [PATCH] random: Clean up NUMA allocations Use kcalloc(), rather than doing the multiply by hand. Use sizeof(*pool) rather than assuming it's == sizeof(void *). kcalloc() zeroes by default so we don't need __GFP_ZERO. Drop the __GFP_NOFAILs, we can easily handle allocation failures, the code is already written to cope with a NULL crng_node_pool, or a NULL entry for a given node in the pool. Signed-off-by: Michael Ellerman <[email protected]> --- drivers/char/random.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index ea03dfe2f21c..22c8ac173666 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1666,11 +1666,15 @@ static int rand_initialize(void) crng_initialize(&primary_crng); #ifdef CONFIG_NUMA - pool = kmalloc(nr_node_ids * sizeof(void *), - GFP_KERNEL|__GFP_NOFAIL|__GFP_ZERO); + pool = kcalloc(nr_node_ids, sizeof(*pool), GFP_KERNEL); + if (!pool) + return 0; + for_each_online_node(i) { - crng = kmalloc_node(sizeof(struct crng_state), - GFP_KERNEL | __GFP_NOFAIL, i); + crng = kmalloc_node(sizeof(struct crng_state), GFP_KERNEL, i); + if (!crng) + continue; + spin_lock_init(&crng->lock); crng_initialize(crng); pool[i] = crng; -- 2.7.4
