public inbox for linux-kernel@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox