All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	anton@samba.org, Mike Travis <travis@sgi.com>
Subject: Re: [PATCH 5/5] cpumask: reduce cpumask_size
Date: Mon, 28 Jun 2010 19:57:33 +0930	[thread overview]
Message-ID: <201006281957.34403.rusty@rustcorp.com.au> (raw)
In-Reply-To: <20100628114425.3881.A69D9226@jp.fujitsu.com>

On Mon, 28 Jun 2010 12:42:23 pm KOSAKI Motohiro wrote:
> > Now we're sure noone is using old cpumask operators, nor *cpumask, we can
> > allocate less bits safely.  This reduces the memory usage of off-stack
> > cpumasks when CONFIG_CPUMASK_OFFSTACK=y but we don't have NR_CPUS actual
> > cpus.
> 
> I have to say I'm sorry. Probably I broke your assumption.
> If this patch applied, we reintroduce exposing nr_cpu_ids issue and
> break libnuma again. I think following change is necessary too.
> 
> Or, Am I missing something?

I cc'd you because I remembered you being involved in that libnuma issue
and couldn't remember the details.

Unfortunately, this solution doesn't work:

> diff --git a/kernel/sched.c b/kernel/sched.c
> index 18faf4d..c14acad 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -4823,7 +4823,9 @@ SYSCALL_DEFINE3(sched_getaffinity, pid_t, pid, unsigned int, len,
> 
>         ret = sched_getaffinity(pid, mask);
>         if (ret == 0) {
> -               size_t retlen = min_t(size_t, len, cpumask_size());
> +               size_t retlen = min_t(size_t, len,
> +                                     BITS_TO_LONGS(NR_CPUS) * sizeof(long));
> 

Since mask is a cpumask_var_t, only cpumask_size() is allocated.  We can't
copy NR_CPUS bits.

But I think it's OK, anyway.  libnuma is broken because it gets upset if the
number of cpus it reads from /sys/.../cpumap is more than the cpumask size
returned from sys_sched_getaffinity.

Currently, getaffinity returns cpumask_size() (ie. based on NR_CPUS), and
the printing routines use nr_cpumask_bits (ie. based on NR_CPUS for 
!CPUMASK_OFFSTACK, nr_cpu_ids for CPUMASK_OFFSTACK).

(libnuma is OK on CONFIG_CPUMASK_OFFSTACK=y because the sysfs output is
*shorter* than expected.  I checked the code).

With this patch, cpumask_size() becomes based on nr_cpumask_bits, so both
getaffinity and sysfs are using the same basis.

Do you agree?
Rusty.

  reply	other threads:[~2010-06-28 10:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-25 13:05 [PATCH 5/5] cpumask: reduce cpumask_size Rusty Russell
2010-06-28  3:12 ` KOSAKI Motohiro
2010-06-28 10:27   ` Rusty Russell [this message]
2010-06-28 10:31     ` KOSAKI Motohiro

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=201006281957.34403.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=anton@samba.org \
    --cc=arnd@arndb.de \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=travis@sgi.com \
    /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.