Re: [PATCH 05/31] cpumask: Provide new cpumask API

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@xxxxxxx> wrote:
> 
>>     /* replaces cpumask_t dst = (cpumask_t)src */
>>     void cpus_copy(cpumask_t dst, const cpumask_t src);
> 
> minor namespace nit i noticed while looking at actual usage of 
> cpus_copy(): could you please rename it cpumask_set(dst, src)?
> 
> That streamlines it to have the same naming concept as atomic_set(), 
> node_set(), zero_fd_set(), etc.

Cpus_copy came from it's underlying function: bits_copy().  Cpumask_set
would deviate from the current naming convention of cpu_XXX for single
cpu ops and cpus_XXX for all cpus ops.  Do we want that?

> 
> the patch-set looks quite nice otherwise already, the changes are 
> straightforward and the end result already looks a lot more maintainable 
> and not fragile at all.

I was hoping for a stronger compiler error to indicate incorrect usage,
it currently just says "may be used before it's initialized" if you mistakenly
have cpumask_t as the local variable declaration.  I'm testing it now.

> 
> In what way will Rusty's changes differ? Since you incorporate some of 
> Rusty's changes already, could you please iterate towards a single 
> patchset which we could then start testing?

Our timezones are not very conducive to a lot of email exchanges (and he's moving.)
>From what I've seen I believe he's leaning towards using struct cpumask * and
less trickery than I have.

The other alternative that is very easy to implement with the new code is
using a simple unsigned long list for cpumask_t (as Linus first suggested):

	typedef unsigned long cpumask_var_t[1];	/* small NR_CPUS */
	typedef unsigned long *cpumask_var_t;	/* large NR_CPUS */

This simplifies things quite a bit and I can get rid of some trickery (and
it's a pointer already without having to invent a pointer to a struct.)  The
downside is arrays of cpumask_t's are less clean, but doable.  The best thing
about the changes in this patchset is the context of the cpumask is more well
known, and changes to the underlying type for cpumask are confined to only a
few files (cpumask.h, lib/cpumask.c, etc.)

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/