lkml.org 
[lkml]   [2008]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 01/31] cpumask: Documentation
Date
On Tuesday 30 September 2008 04:02:51 Mike Travis wrote:
> +The Changes
> +
> +Provide new cpumask interface API. The relevant change is basically
> +cpumask_t becomes an opaque object. This should result in the minimum
> +amount of modifications while still allowing the inline cpumask functions,
> +and the ability to declare static cpumask objects.
> +
> +
> + /* raw declaration */
> + struct __cpumask_data_s { DECLARE_BITMAP(bits, NR_CPUS); };
> +
> + /* cpumask_map_t used for declaring static cpumask maps */
> + typedef struct __cpumask_data_s cpumask_map_t[1];
> +
> + /* cpumask_t used for function args and return pointers */
> + typedef struct __cpumask_data_s *cpumask_t;
> + typedef const struct __cpumask_data_s *const_cpumask_t;
> +
> + /* cpumask_var_t used for local variable, definition follows */
> + typedef struct __cpumask_data_s cpumask_var_t[1]; /* SMALL NR_CPUS */
> + typedef struct __cpumask_data_s *cpumask_var_t; /* LARGE NR_CPUS */
> +
> + /* replaces cpumask_t dst = (cpumask_t)src */
> + void cpus_copy(cpumask_t dst, const cpumask_t src);

Hi Mike,

I have several problems with this patch series. First, it's a flag day
change, which means it isn't bisectable and can't go through linux-next.
Secondly, we still can't hide the definition of the cpumask struct as long as
they're passed as cpumask_t, so it's going to be hard to find assignments
(illegal once we allocate nr_cpu_ids bits rather than NR_CPUS), and on-stack
users.

Finally, we end up with code which is slightly more opaque than the
current code, with two new typedefs. And that's an ongoing problem.

I took a slightly divergent line with my patch series, and introduced a
parallel cpumask system which always passes and returns masks by pointer:

cpumask_t -> struct cpumask
on-stack cpumask_t -> cpumask_var_t (same as your patch)
cpus_and(dst, src1, src2) etc -> cpumask_and(&dst, &src1, &src2)
cpumask_t cpumask_of_cpu(cpu) -> const struct cpumask *cpumask_of(cpu)
cpumask_t cpu_online_map etc -> const struct cpumask *cpu_online_mask etc.

The old ops are expressed in terms of the new ops, and can be phased out over
time.

In addition, I added some new twists:

static cpumasks and cpumasks in structures
-> DECLARE_BITMAP(foo, NR_CPUS) and to_cpumask()

This means we can eventually obscure the actual definition of struct cpumask,
to catch abuse.

cpus_and(tmp, mask, online_mask); for_each_cpu(i, tmp)
-> for_each_cpu_both(i, mask, online_mask)

This helper saves numerous on-stack temporaries.

NR_CPUS -> CONFIG_NR_CPUS

The config option is now valid for UP as well. This cleanup allows us to
audit users of NR_CPUS (which might be used incorrectly now cpumask_
iterators only go to nr_cpu_ids).

The patches are fairly uninteresting, but here is the summary:

x86: remove noop cpus_and() with CPU_MASK_ALL.
x86: clean up speedctep-centrino and reduce cpumask_t usage
cpumask: remove min from first_cpu/next_cpu
cpumask: introduce struct cpumask.
cpumask: change cpumask_scnprintf, cpumask_parse_user, cpulist_parse, and
cpulist_scnprintf to take pointers.
cpumask: add cpumask_copy()
cpumask: introduce cpumask_var_t for local cpumask vars
cpumask: make CONFIG_NR_CPUS always valid.
cpumask: use setup_nr_cpu_ids() instead of direct assignment.
cpumask: make nr_cpu_ids valid in all configurations.
cpumask: prepare for iterators to only go to nr_cpu_ids.
cpumask: make nr_cpu_ids the actual limit on bitmap size
cpumask: replace for_each_cpu_mask_nr with for_each_cpu_mask everywhere
cpumask: use cpumask_bits() everywhere.
cpumask: Use &cpu_mask_all instead of CPU_MASK_ALL_PTR.
cpumask: cpumask_of(): cpumask_of_cpu() which returns a pointer.
cpumask: for_each_cpu(): for_each_cpu_mask which takes a pointer
cpumask: cpumask_first/cpumask_next
cpumask: for_each_cpu_both() / cpumask_first_both() / cpumask_next_both()
cpumask: deprecate any_online_cpu() in favour of cpumask_any/cpumask_any_both
cpumask: Replace CPUMASK_ALLOC etc with cpumask_var_t.
cpumask: get rid of boutique sched.c allocations, use cpumask_var_t.
cpumask: reorder header to minimize separate #ifdefs
cpumask: accessors to manipulate possible/present/online/active maps
cpumask: Use accessors code.
cpumask: switch over to cpu_online/possible/active/present_mask
cpumask: to_cpumask()
cpumask: cpu_all_mask: pointer version of cpu_mask_all.
cpumask: remove any_online_cpu() users.
cpumask: smp_call_function_many()
cpumask: Use smp_call_function_many()
cpumask: make irq_set_affinity() take a const struct cpumask *
x86: make TARGET_CPUS/target_cpus take a const struct cpumask *

I'll commit these to my quilt series today.

Thanks,
Rusty.


\
 
 \ /
  Last update: 2008-10-01 00:53    [from the cache]
©2003-2011 Jasper Spaans