[PATCH] Avoid mask based num_possible_cpus and num_online_cpus

January 17th, 2012 - 09:10 pm ET by Venkatesh Pallipadi | Report spam
Kernel's notion of possible cpus (from include/linux/cpumask.h)
* cpu_possible_mask- has bit 'cpu' set iff cpu is populatable

* The cpu_possible_mask is fixed at boot time, as the set of CPU id's
* that it is possible might ever be plugged in at anytime during the
* life of that system boot.

#define num_possible_cpus() cpumask_weight(cpu_possible_mask)

and on x86 cpumask_weight() calls hweight64 and hweight64 (on older kernels
and systems with !X86_FEATURE_POPCNT) or a popcnt based alternative.

i.e, We needlessly go through this mask based calculation everytime
num_possible_cpus() is called.

The problem is there with cpu_online_mask() as well, which is fixed value at
boot time in !CONFIG_HOTPLUG_CPU case and should not change that often even
in HOTPLUG case.

Though most of the callers of these two routines are init time (with few
exceptions of runtime calls), it is cleaner to use variables
and not go through this repeated mask based calculation.

Signed-off-by: Venkatesh Pallipadi <venki@google.com>

include/linux/cpumask.h | 8 ++++++--
kernel/cpu.c | 9 +++++++++
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 4f7a632..2eb04dd 100644
a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -80,9 +80,13 @@ extern const struct cpumask *const cpu_online_mask;
extern const struct cpumask *const cpu_present_mask;
extern const struct cpumask *const cpu_active_mask;

+extern int nr_online_cpus;
+
#if NR_CPUS > 1
-#define num_online_cpus() cpumask_weight(cpu_online_mask)
-#define num_possible_cpus() cpumask_weight(cpu_possible_mask)
+
+#define num_online_cpus() (nr_online_cpus)
+#define num_possible_cpus() (nr_cpu_ids)
+
#define num_present_cpus() cpumask_weight(cpu_present_mask)
#define num_active_cpus() cpumask_weight(cpu_active_mask)
#define cpu_online(cpu) cpumask_test_cpu((cpu), cpu_online_mask)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..eed2169 100644
a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -622,6 +622,13 @@ static DECLARE_BITMAP(cpu_active_bits, CONFIG_NR_CPUS) __read_mostly;
const struct cpumask *const cpu_active_mask = to_cpumask(cpu_active_bits);
EXPORT_SYMBOL(cpu_active_mask);

+#ifdef CONFIG_HOTPLUG_CPU
+int nr_online_cpus;
+#else
+int nr_online_cpus __read_mostly;
+#endif
+EXPORT_SYMBOL(nr_online_cpus);
+
void set_cpu_possible(unsigned int cpu, bool possible)
{
if (possible)
@@ -644,6 +651,8 @@ void set_cpu_online(unsigned int cpu, bool online)
cpumask_set_cpu(cpu, to_cpumask(cpu_online_bits));
else
cpumask_clear_cpu(cpu, to_cpumask(cpu_online_bits));
+
+ nr_online_cpus = cpumask_weight(cpu_online_mask);
}

void set_cpu_active(unsigned int cpu, bool active)
1.7.7.3

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussionReplies 3 repliesReplies Make a reply

Similar topics

Replies

#1 KOSAKI Motohiro
January 18th, 2012 - 01:00 am ET | Report spam
(1/17/12 9:07 PM), Venkatesh Pallipadi wrote:
Kernel's notion of possible cpus (from include/linux/cpumask.h)
* cpu_possible_mask- has bit 'cpu' set iff cpu is populatable

* The cpu_possible_mask is fixed at boot time, as the set of CPU id's
* that it is possible might ever be plugged in at anytime during the
* life of that system boot.

#define num_possible_cpus() cpumask_weight(cpu_possible_mask)

and on x86 cpumask_weight() calls hweight64 and hweight64 (on older kernels
and systems with !X86_FEATURE_POPCNT) or a popcnt based alternative.

i.e, We needlessly go through this mask based calculation everytime
num_possible_cpus() is called.

The problem is there with cpu_online_mask() as well, which is fixed value at
boot time in !CONFIG_HOTPLUG_CPU case and should not change that often even
in HOTPLUG case.

Though most of the callers of these two routines are init time (with few
exceptions of runtime calls), it is cleaner to use variables
and not go through this repeated mask based calculation.

Signed-off-by: Venkatesh Pallipadi

include/linux/cpumask.h | 8 ++++++--
kernel/cpu.c | 9 +++++++++
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 4f7a632..2eb04dd 100644
a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -80,9 +80,13 @@ extern const struct cpumask *const cpu_online_mask;
extern const struct cpumask *const cpu_present_mask;
extern const struct cpumask *const cpu_active_mask;

+extern int nr_online_cpus;
+
#if NR_CPUS> 1
-#define num_online_cpus() cpumask_weight(cpu_online_mask)
-#define num_possible_cpus() cpumask_weight(cpu_possible_mask)
+
+#define num_online_cpus() (nr_online_cpus)
+#define num_possible_cpus() (nr_cpu_ids)



nr_cpu_ids mean maximum cpu id of cpus. if cpu id are sparse, maximum id
doesn't match number of cpus.



+
#define num_present_cpus() cpumask_weight(cpu_present_mask)
#define num_active_cpus() cpumask_weight(cpu_active_mask)
#define cpu_online(cpu) cpumask_test_cpu((cpu), cpu_online_mask)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..eed2169 100644
a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -622,6 +622,13 @@ static DECLARE_BITMAP(cpu_active_bits, CONFIG_NR_CPUS) __read_mostly;
const struct cpumask *const cpu_active_mask = to_cpumask(cpu_active_bits);
EXPORT_SYMBOL(cpu_active_mask);

+#ifdef CONFIG_HOTPLUG_CPU
+int nr_online_cpus;
+#else
+int nr_online_cpus __read_mostly;
+#endif
+EXPORT_SYMBOL(nr_online_cpus);



You can always mark this to __read_mostly. other cpu hotplug stuff do so.
Because of, I guess, cpu hotplug developers don't think hotplugging is
frequently event.


void set_cpu_possible(unsigned int cpu, bool possible)
{
if (possible)
@@ -644,6 +651,8 @@ void set_cpu_online(unsigned int cpu, bool online)
cpumask_set_cpu(cpu, to_cpumask(cpu_online_bits));
else
cpumask_clear_cpu(cpu, to_cpumask(cpu_online_bits));
+
+ nr_online_cpus = cpumask_weight(cpu_online_mask);
}



I like this change. :)



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#2 Venki Pallipadi
January 18th, 2012 - 02:00 pm ET | Report spam
On Tue, Jan 17, 2012 at 9:55 PM, KOSAKI Motohiro
wrote:
(1/17/12 9:07 PM), Venkatesh Pallipadi wrote:
Kernel's notion of possible cpus (from include/linux/cpumask.h)
  *     cpu_possible_mask- has bit 'cpu' set iff cpu is populatable

  *  The cpu_possible_mask is fixed at boot time, as the set of CPU id's
  *  that it is possible might ever be plugged in at anytime during the
  *  life of that system boot.

  #define num_possible_cpus()     cpumask_weight(cpu_possible_mask)

and on x86 cpumask_weight() calls hweight64 and hweight64 (on older kernels
and systems with !X86_FEATURE_POPCNT) or a popcnt based alternative.

i.e, We needlessly go through this mask based calculation everytime
num_possible_cpus() is called.

The problem is there with cpu_online_mask() as well, which is fixed value at
boot time in !CONFIG_HOTPLUG_CPU case and should not change that often even
in HOTPLUG case.

Though most of the callers of these two routines are init time (with few
exceptions of runtime calls), it is cleaner to use variables
and not go through this repeated mask based calculation.

Signed-off-by: Venkatesh Pallipadi

  include/linux/cpumask.h |    8 ++++++--
  kernel/cpu.c            |    9 +++++++++
  2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 4f7a632..2eb04dd 100644
a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -80,9 +80,13 @@ extern const struct cpumask *const cpu_online_mask;
  extern const struct cpumask *const cpu_present_mask;
  extern const struct cpumask *const cpu_active_mask;

+extern int nr_online_cpus;
+
  #if NR_CPUS>  1
-#define num_online_cpus()    cpumask_weight(cpu_online_mask)
-#define num_possible_cpus()  cpumask_weight(cpu_possible_mask)
+
+#define num_online_cpus()    (nr_online_cpus)
+#define num_possible_cpus()  (nr_cpu_ids)



nr_cpu_ids mean maximum cpu id of cpus. if cpu id are sparse, maximum id
doesn't match number of cpus.




Yes. But will it be sparse in any arch? I saw some of the users of
num_possible_cpus() doing things like allocating a buffer for that
size and then indexing it using get_cpu(). So, I thought it would be
better to use the existing nr_cpu_ids instead of inventing another
variable. If indeed any arch is depending on this being sparse, we can
have a new variable similar to num_possible_cpus and also audit all
users of num_possible_cpus to see whether they should be using
nr_cpu_ids instead.



+
  #define num_present_cpus()  cpumask_weight(cpu_present_mask)
  #define num_active_cpus()   cpumask_weight(cpu_active_mask)
  #define cpu_online(cpu)             cpumask_test_cpu((cpu), cpu_online_mask)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..eed2169 100644
a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -622,6 +622,13 @@ static DECLARE_BITMAP(cpu_active_bits, CONFIG_NR_CPUS) __read_mostly;
  const struct cpumask *const cpu_active_mask = to_cpumask(cpu_active_bits);
  EXPORT_SYMBOL(cpu_active_mask);

+#ifdef CONFIG_HOTPLUG_CPU
+int nr_online_cpus;
+#else
+int nr_online_cpus __read_mostly;
+#endif
+EXPORT_SYMBOL(nr_online_cpus);



You can always mark this to __read_mostly. other cpu hotplug stuff do so.
Because of, I guess, cpu hotplug developers don't think hotplugging is
frequently event.




OK. Agree.


  void set_cpu_possible(unsigned int cpu, bool possible)
  {
      if (possible)
@@ -644,6 +651,8 @@ void set_cpu_online(unsigned int cpu, bool online)
              cpumask_set_cpu(cpu, to_cpumask(cpu_online_bits));
      else
              cpumask_clear_cpu(cpu, to_cpumask(cpu_online_bits));
+
+     nr_online_cpus = cpumask_weight(cpu_online_mask);
  }



I like this change. :)





Thanks,
Venki
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Replies Reply to this message
#3 KOSAKI Motohiro
January 18th, 2012 - 02:30 pm ET | Report spam
(1/18/12 1:52 PM), Venki Pallipadi wrote:
On Tue, Jan 17, 2012 at 9:55 PM, KOSAKI Motohiro
wrote:
(1/17/12 9:07 PM), Venkatesh Pallipadi wrote:
Kernel's notion of possible cpus (from include/linux/cpumask.h)
* cpu_possible_mask- has bit 'cpu' set iff cpu is populatable

* The cpu_possible_mask is fixed at boot time, as the set of CPU id's
* that it is possible might ever be plugged in at anytime during the
* life of that system boot.

#define num_possible_cpus() cpumask_weight(cpu_possible_mask)

and on x86 cpumask_weight() calls hweight64 and hweight64 (on older kernels
and systems with !X86_FEATURE_POPCNT) or a popcnt based alternative.

i.e, We needlessly go through this mask based calculation everytime
num_possible_cpus() is called.

The problem is there with cpu_online_mask() as well, which is fixed value at
boot time in !CONFIG_HOTPLUG_CPU case and should not change that often even
in HOTPLUG case.

Though most of the callers of these two routines are init time (with few
exceptions of runtime calls), it is cleaner to use variables
and not go through this repeated mask based calculation.

Signed-off-by: Venkatesh Pallipadi

include/linux/cpumask.h | 8 ++++++--
kernel/cpu.c | 9 +++++++++
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 4f7a632..2eb04dd 100644
a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -80,9 +80,13 @@ extern const struct cpumask *const cpu_online_mask;
extern const struct cpumask *const cpu_present_mask;
extern const struct cpumask *const cpu_active_mask;

+extern int nr_online_cpus;
+
#if NR_CPUS> 1
-#define num_online_cpus() cpumask_weight(cpu_online_mask)
-#define num_possible_cpus() cpumask_weight(cpu_possible_mask)
+
+#define num_online_cpus() (nr_online_cpus)
+#define num_possible_cpus() (nr_cpu_ids)



nr_cpu_ids mean maximum cpu id of cpus. if cpu id are sparse, maximum id
doesn't match number of cpus.




Yes. But will it be sparse in any arch? I saw some of the users of
num_possible_cpus() doing things like allocating a buffer for that
size and then indexing it using get_cpu(). So, I thought it would be
better to use the existing nr_cpu_ids instead of inventing another
variable. If indeed any arch is depending on this being sparse, we can
have a new variable similar to num_possible_cpus and also audit all
users of num_possible_cpus to see whether they should be using
nr_cpu_ids instead.



If my remember is correct, sparc does.


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussion Replies Reply to this message
Help Create a new topicReplies Make a reply
Search Make your own search