All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@novell.com>
To: "Andrew Morton" <akpm@osdl.org>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tvec_bases too large for per-cpu data
Date: Tue, 24 Jan 2006 09:33:00 +0100	[thread overview]
Message-ID: <43D5F44C.76F0.0078.0@novell.com> (raw)
In-Reply-To: <20060123025702.1f116e70.akpm@osdl.org>

>> >Did you consider using alloc_percpu()?
>> 
>> I did, but I saw drawbacks with that (most notably the fact that all instances are allocated at
>> once, possibly wasting a lot of memory).
>
>It's 4k for each cpu which is in the possible_map but which will never be
>brought online.  I don't think that'll be a lot of memory - are there
>machines which have a lot of possible-but-not-really-there CPUs?

I would suppose so. Why wouldn't a machine supporting CPU hotplug not reasonably be able to double,
triple, etc the number of CPUs originally present?

>There _must_ be ordering issues.  Otherwise we'd just dynamically allocate
>all the structs up-front and be done with it.
>
>Presumably the ordering issue is that init_timers() is called before
>kmem_cache_init().  That's non-obvious and should be commented.

That I can easily do, sure.

>- The `#ifdef CONFIG_NUMA' in init_timers_cpu() seems to be unnecessary -
>  kmalloc_node() will use kmalloc() if !NUMA.

That is correct, but I wanted the fallback if kmalloc_node() fails (from briefly looking at that code it didn't
seem like it would do such fallback itself). And calling kmalloc() twice if !NUMA seemed pointless.

>- The likely()s in init_timers_cpu() seems fairly pointless - it's not a
>  fastpath.

OK, will change that.

>- We prefer to do this:
>
>	if (expr) {
>		...
>	} else {
>		...
>	}
>
>  and not
>
>	if (expr) {
>		...
>	}
>	else {
>		...
>	}

I can change that, too, but I don't see why this gets pointed out again and again when there really
is no consistency across the entire kernel...

Jan

  reply	other threads:[~2006-01-24  8:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-18 13:11 [PATCH] tvec_bases too large for per-cpu data Jan Beulich
2006-01-21  7:25 ` Andrew Morton
2006-01-23 10:31   ` Jan Beulich
2006-01-23 10:57     ` Andrew Morton
2006-01-24  8:33       ` Jan Beulich [this message]
2006-01-24  8:58         ` Andrew Morton
2006-01-24 14:46           ` [PATCH] [SMP] reduce size of percpudata, and make sure per_cpu(object, not_possible_cpu) cause an invalid memory reference Eric Dumazet
2006-01-24 14:53             ` Andi Kleen
2006-02-01  9:21             ` [PATCH] [SMP] __GENERIC_PER_CPU changes Eric Dumazet
2006-01-30  8:43       ` [PATCH] tvec_bases too large for per-cpu data Jan Beulich
2006-01-31 22:27         ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43D5F44C.76F0.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.