All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jeff Garzik <jeff@garzik.org>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mike Travis <travis@sgi.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	viro@zeniv.linux.org.uk,
	Andrew Morton <akpm@linux-foundation.org>,
	roland@redhat.com
Subject: Re: [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct
Date: Fri, 8 May 2009 21:09:44 +0200	[thread overview]
Message-ID: <20090508190944.GB12130@elte.hu> (raw)
In-Reply-To: <20090508185015.GA11320@havoc.gtf.org>


* Jeff Garzik <jeff@garzik.org> wrote:

> The semantics for variable-length arrays __in the middle of structs__
> are quite muddy, and a case in sched.c presents an interesting case,
> as the preceding code comment indicates:
> 
> 	/*
> 	 * The cpus mask in sched_group and sched_domain hangs off
> 	 the end.  * FIXME: use cpumask_var_t or dynamic percpu alloc
> 	 to avoid * wasting space for nr_cpu_ids < CONFIG_NR_CPUS.  */
> 	struct static_sched_group {
> 		struct sched_group sg; DECLARE_BITMAP(cpus,
> 		CONFIG_NR_CPUS);
> 	};
> 
> 	struct static_sched_domain {
> 		struct sched_domain sd; DECLARE_BITMAP(span,
> 		CONFIG_NR_CPUS);
> 	};
> 
> Both sched_group and sched_domain have the following trailing struct
> member:
> 
> 	unsigned long cpumask[];
> 
> So this change is intended largely to spawn a discussion, because
> I'm not sure this VLA-in-middle-of-struct behavior is guaranteed to
> always behave as expected?
> 
> Maybe a C expert can say whether cpumask[0] is better than cpumask[],
> or have other comments?

This bit of the code is a bit temporary, with a half-done cpumask 
conversion ...

I'm not convinced at all that this code wants to be converted to 
cpuvar_t. These are all either statically percpu or dynamically 
allocated structures, so we wont allocate NR_CPUS of them. So i'd 
lean towards embedding a full struct cpumask and that's it.

That cpumask[] should probably be cpumask[0], to document the 
aliasing to ->span and ->cpus properly.

Rusty, what do you think?

	Ingo

  reply	other threads:[~2009-05-08 19:11 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-08 18:48 [PATCH 1/2] kernel/{sched,smp}.c: fix static decl prior to struct declaration Jeff Garzik
2009-05-08 18:50 ` [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct Jeff Garzik
2009-05-08 19:09   ` Ingo Molnar [this message]
2009-05-10  8:49     ` Rusty Russell
2009-05-10 15:09       ` Jeff Garzik
2009-05-12 13:34         ` Rusty Russell
2009-05-12 14:03           ` Al Viro
2009-05-13  2:12             ` Rusty Russell
2009-05-13  2:31               ` Jeff Garzik
2009-05-13  5:36               ` Al Viro
2009-05-13  6:49                 ` [PATCH] sched: avoid flexible array member inside struct (gcc extension) Rusty Russell
2009-05-13 13:51                   ` [tip:sched/urgent] " tip-bot for Rusty Russell
2009-05-11 10:58       ` [RFC PATCH 2/2] kernel/sched.c: VLA in middle of struct Ingo Molnar
2009-05-11 20:43         ` Jeff Garzik
2009-05-11 20:49           ` Ingo Molnar
2009-05-12  2:24             ` Jeff Garzik
2009-05-12  8:54               ` Ingo Molnar
2009-05-08 19:04 ` [PATCH 1/2] kernel/{sched,smp}.c: fix static decl prior to struct declaration Ingo Molnar
2009-05-08 19:08   ` Jeff Garzik
2009-05-08 19:13     ` Ingo Molnar
2009-05-08 19:38 ` [PATCH 1/2 v2] " Jeff Garzik
2009-05-11 11:26   ` Ingo Molnar
2009-05-11 20:06     ` Jeff Garzik
2009-05-11 20:51       ` Ingo Molnar
2009-05-11 11:30   ` [tip:sched/core] kernel/{sched, smp}.c: " tip-bot for Jeff Garzik
2009-05-11 20:02   ` [PATCH 1/2 v3] kernel/{sched,smp}.c: " Jeff Garzik
2009-05-11 21:51     ` [tip:sched/urgent] kernel/{sched, smp}.c: " tip-bot for Jeff Garzik
2009-05-11 21:56       ` Ingo Molnar

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=20090508190944.GB12130@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=travis@sgi.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.