LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup
@ 2018-03-20 11:04 Dou Liyang
  2018-03-20 11:04 ` [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus Dou Liyang
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

Recently, we hoped to make the possible CPU count more accurate for
Kernel. I stuck on the issue how do I run acpi_early_init() _before_ 
setup_percpu() is invoked. So send these insignificant patches first.

This patchset does this things:

  - two document-related work(the 1th and 2th patch),
  - two cleapup work (the 3th and 5th patch)
  - a bug fix for CPU hotplug(4th patch)

Dou Liyang (5):
  x86/smpboot: Add the missing description of possible_cpus
  x86/cpu_hotplug: Update the link of cpu_hotplug.rst
  x86/smpboot: Make the check code more clear in prefill_possible_map()
  acpi/processor: Fix the return value of acpi_processor_ids_walk()
  acpi/processor: Make the acpi_duplicate_processor_id() static

 Documentation/00-INDEX                          |  2 -
 Documentation/admin-guide/kernel-parameters.txt |  5 ++
 Documentation/cputopology.txt                   | 10 ++--
 Documentation/x86/x86_64/cpu-hotplug-spec       |  2 +-
 arch/x86/kernel/smpboot.c                       | 31 +++++++-----
 drivers/acpi/acpi_processor.c                   | 66 ++++++++++++-------------
 include/linux/acpi.h                            |  3 --
 7 files changed, 62 insertions(+), 57 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
@ 2018-03-20 11:04 ` Dou Liyang
  2018-03-20 12:37   ` Peter Zijlstra
  2018-03-20 11:04 ` [PATCH 2/5] x86/cpu_hotplug: Update the link of cpu_hotplug.rst Dou Liyang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

Kernel uses the possible_cpus in command line to reset the possible_cpus bits
in cpu_possible_map. It doesn't be recorded in the kernel-parameters.txt

Add its description in this document, also replace the wrong option additional_cpus
with possible_cpus in cpu-gotplug-spec.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 5 +++++
 Documentation/x86/x86_64/cpu-hotplug-spec       | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..34f8a5f04e63 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2832,6 +2832,11 @@
 			variables need be pre-allocated for later physical cpu
 			hot plugging.
 
+	possible_cpus=	[s390,x86_64] Use this to set hotpluggable cpus.
+			This option sets possible_cpus bits in cpu_possible_map.
+			Thus keeping the numbers of bits set constant even if
+			the machine gets rebooted.
+
 	nr_uarts=	[SERIAL] maximum number of UARTs to be registered.
 
 	numa_balancing=	[KNL,X86] Enable or disable automatic NUMA balancing.
diff --git a/Documentation/x86/x86_64/cpu-hotplug-spec b/Documentation/x86/x86_64/cpu-hotplug-spec
index 3c23e0587db3..d218bc0bdaaa 100644
--- a/Documentation/x86/x86_64/cpu-hotplug-spec
+++ b/Documentation/x86/x86_64/cpu-hotplug-spec
@@ -16,6 +16,6 @@ it should have its LAPIC Enabled bit set to 0. Linux will use the number
 of disabled LAPICs to compute the maximum number of future CPUs.
 
 In the worst case the user can overwrite this choice using a command line
-option (additional_cpus=...), but it is recommended to supply the correct
+option (possible_cpus=...), but it is recommended to supply the correct
 number (or a reasonable approximation of it, with erring towards more not less)
 in the MADT to avoid manual configuration.
-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/5] x86/cpu_hotplug: Update the link of cpu_hotplug.rst
  2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
  2018-03-20 11:04 ` [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus Dou Liyang
@ 2018-03-20 11:04 ` Dou Liyang
  2018-03-20 11:04 ` [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map() Dou Liyang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

The original cpu_hotplug.txt documents describing CPU hotplug support in
Linux kernel. it was moved in to core-api/ and renamed cpu_hotplug.rst.

Update it's link in other documents

Fixes: ff58fa7f556c ("Documentation: Update CPU hotplug and move it to core-api")
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 Documentation/00-INDEX        |  2 --
 Documentation/cputopology.txt | 10 +++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 7f3a0728ccf2..3773c67ea9e5 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -104,8 +104,6 @@ core-api/
 	- documentation on kernel core components.
 cpu-freq/
 	- info on CPU frequency and voltage scaling.
-cpu-hotplug.txt
-	- document describing CPU hotplug support in the Linux kernel.
 cpu-load.txt
 	- document describing how CPU load statistics are collected.
 cpuidle/
diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
index c6e7e9196a8b..e05b0879fe91 100644
--- a/Documentation/cputopology.txt
+++ b/Documentation/cputopology.txt
@@ -117,9 +117,9 @@ source for the output is in brackets ("[]").
 		[NR_CPUS-1]
 
     offline:	CPUs that are not online because they have been
-		HOTPLUGGED off (see cpu-hotplug.txt) or exceed the limit
-		of CPUs allowed by the kernel configuration (kernel_max
-		above). [~cpu_online_mask + cpus >= NR_CPUS]
+		HOTPLUGGED off (see core-api/cpu_hotplug.rst) or exceed
+		the limit of CPUs allowed by the kernel configuration
+		(kernel_max above). [~cpu_online_mask + cpus >= NR_CPUS]
 
     online:	CPUs that are online and being scheduled [cpu_online_mask]
 
@@ -155,5 +155,5 @@ online.)::
        possible: 0-127
         present: 0-3
 
-See cpu-hotplug.txt for the possible_cpus=NUM kernel start parameter
-as well as more information on the various cpumasks.
+See core-api/cpu_hotplug.rst for the possible_cpus=NUM kernel start
+parameter as well as more information on the various cpumasks.
-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
  2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
  2018-03-20 11:04 ` [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus Dou Liyang
  2018-03-20 11:04 ` [PATCH 2/5] x86/cpu_hotplug: Update the link of cpu_hotplug.rst Dou Liyang
@ 2018-03-20 11:04 ` Dou Liyang
  2018-03-20 12:39   ` Peter Zijlstra
  2018-03-20 11:04 ` [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk() Dou Liyang
  2018-03-20 11:04 ` [PATCH 5/5] acpi/processor: Make the acpi_duplicate_processor_id() static Dou Liyang
  4 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

In prefill_possible_map(), Kernel need to get the number of possible CPUs
to reset cpu_possible_map. The number is related to the options:

  -nosmp, maxcpus, possible_cpus, nr_cpus

... which need to be checked.

Currentlly, the check code mixed these options together in confusion and
hard to follow.

Isolate them, and check them linearly.

No functionary change, Prepare for cutting the number of possible CPUs

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 
Situations:

setup_possible_cpus == -1 |  setup_max_cpus == 0 |  CONFIG_HOTPLUG_CPU == y | 

        yes               |         yes          |         yes              |
        no                |         no           |         no               |

Test cases:
           Cases          |          the number of possible cpus
                          |       (the same with or w/o this patch)
case 1: no  | no  | no | -->  min (setup_possible_cpus, nr_cpu_ids, setup_max_cpus)
case 2: no  | no  | yes| -->  min (setup_possible_cpus, nr_cpu_ids)
case 3: no  | yes | no | -->  1                                    
case 4: no  | yes | yes| -->  1
case 5: yes | no  | no | -->  min (num_processors, nr_cpu_ids, setup_max_cpus)
case 6: yes | no  | yes| -->  min (num_processors + disabled_cpus, nr_cpu_ids)
case 7: yes | yes | no | -->  1 
case 8: yes | yes | yes| -->  1

 arch/x86/kernel/smpboot.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index ff99e2b6fc54..2fdda8567bf9 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1334,7 +1334,7 @@ early_param("possible_cpus", _setup_possible_cpus);
  * We do this because additional CPUs waste a lot of memory.
  * -AK
  */
-__init void prefill_possible_map(void)
+void __init prefill_possible_map(void)
 {
 	int i, possible;
 
@@ -1356,18 +1356,22 @@ __init void prefill_possible_map(void)
 			num_processors = 1;
 	}
 
-	i = setup_max_cpus ?: 1;
+	/* The SMP kernel could be acted as an UP kernel via nosmp/maxcpus=0 */
+	if (!setup_max_cpus) {
+		possible = 1;
+		total_cpus = num_processors + disabled_cpus;
+		goto set_cpu_possible_mask;
+	}
+
+	/* Possible CPUs could be overwrited via possible_cpus= */
 	if (setup_possible_cpus == -1) {
 		possible = num_processors;
 #ifdef CONFIG_HOTPLUG_CPU
-		if (setup_max_cpus)
-			possible += disabled_cpus;
-#else
-		if (possible > i)
-			possible = i;
+		possible += disabled_cpus;
 #endif
-	} else
+	} else {
 		possible = setup_possible_cpus;
+	}
 
 	total_cpus = max_t(int, possible, num_processors + disabled_cpus);
 
@@ -1378,15 +1382,16 @@ __init void prefill_possible_map(void)
 		possible = nr_cpu_ids;
 	}
 
-#ifdef CONFIG_HOTPLUG_CPU
-	if (!setup_max_cpus)
-#endif
-	if (possible > i) {
+#ifndef CONFIG_HOTPLUG_CPU
+	/* Possible CPUs could be reduced via max_cpus= */
+	if (possible > setup_max_cpus) {
 		pr_warn("%d Processors exceeds max_cpus limit of %u\n",
 			possible, setup_max_cpus);
-		possible = i;
+		possible = setup_max_cpus;
 	}
+#endif
 
+set_cpu_possible_mask:
 	nr_cpu_ids = possible;
 
 	pr_info("Allowing %d CPUs, %d hotplug CPUs\n",
-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
  2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
                   ` (2 preceding siblings ...)
  2018-03-20 11:04 ` [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map() Dou Liyang
@ 2018-03-20 11:04 ` Dou Liyang
  2018-05-19 15:06   ` Thomas Gleixner
  2018-03-20 11:04 ` [PATCH 5/5] acpi/processor: Make the acpi_duplicate_processor_id() static Dou Liyang
  4 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

ACPI driver should make sure all the processor IDs in their ACPI Namespace
are unique for CPU hotplug. the driver performs a depth-first walk of the
namespace tree and calls the acpi_processor_ids_walk().

But, the acpi_processor_ids_walk() will return true if one processor is
checked, that cause the walk break after walking pass the first processor.

Repace the value with AE_OK which is the standard acpi_status value.

Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for processor enumeration")

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 drivers/acpi/acpi_processor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 449d86d39965..db5bdb59639c 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -663,11 +663,11 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
 	}
 
 	processor_validated_ids_update(uid);
-	return true;
+	return AE_OK;
 
 err:
 	acpi_handle_info(handle, "Invalid processor object\n");
-	return false;
+	return AE_OK;
 
 }
 
-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/5] acpi/processor: Make the acpi_duplicate_processor_id() static
  2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
                   ` (3 preceding siblings ...)
  2018-03-20 11:04 ` [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk() Dou Liyang
@ 2018-03-20 11:04 ` Dou Liyang
  4 siblings, 0 replies; 18+ messages in thread
From: Dou Liyang @ 2018-03-20 11:04 UTC (permalink / raw)
  To: linux-kernel, x86, linux-acpi, linux-doc
  Cc: tglx, mingo, corbet, rjw, lenb, hpa, peterz, Dou Liyang

The acpi_duplicate_processor_id() is only called in acpi_processor_get_info(),
So move it in front of acpi_processor_get_info() and make it static.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 drivers/acpi/acpi_processor.c | 62 +++++++++++++++++++++----------------------
 include/linux/acpi.h          |  3 ---
 2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index db5bdb59639c..03ec7690710c 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -229,6 +229,37 @@ static inline int acpi_processor_hotadd_init(struct acpi_processor *pr)
 }
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 
+/* The number of the unique processor IDs */
+static int nr_unique_ids __initdata;
+
+/* The number of the duplicate processor IDs */
+static int nr_duplicate_ids;
+
+/* Used to store the unique processor IDs */
+static int unique_processor_ids[] __initdata = {
+	[0 ... NR_CPUS - 1] = -1,
+};
+
+/* Used to store the duplicate processor IDs */
+static int duplicate_processor_ids[] = {
+	[0 ... NR_CPUS - 1] = -1,
+};
+
+static bool acpi_duplicate_processor_id(int proc_id)
+{
+	int i;
+
+	/*
+	 * Compare the proc_id with duplicate IDs, if the proc_id is already
+	 * in the duplicate IDs, return true, otherwise, return false.
+	 */
+	for (i = 0; i < nr_duplicate_ids; i++) {
+		if (duplicate_processor_ids[i] == proc_id)
+			return true;
+	}
+	return false;
+}
+
 static int acpi_processor_get_info(struct acpi_device *device)
 {
 	union acpi_object object = { 0 };
@@ -579,22 +610,6 @@ static struct acpi_scan_handler processor_container_handler = {
 	.attach = acpi_processor_container_attach,
 };
 
-/* The number of the unique processor IDs */
-static int nr_unique_ids __initdata;
-
-/* The number of the duplicate processor IDs */
-static int nr_duplicate_ids;
-
-/* Used to store the unique processor IDs */
-static int unique_processor_ids[] __initdata = {
-	[0 ... NR_CPUS - 1] = -1,
-};
-
-/* Used to store the duplicate processor IDs */
-static int duplicate_processor_ids[] = {
-	[0 ... NR_CPUS - 1] = -1,
-};
-
 static void __init processor_validated_ids_update(int proc_id)
 {
 	int i;
@@ -682,21 +697,6 @@ static void __init acpi_processor_check_duplicates(void)
 						NULL, NULL);
 }
 
-bool acpi_duplicate_processor_id(int proc_id)
-{
-	int i;
-
-	/*
-	 * compare the proc_id with duplicate IDs, if the proc_id is already
-	 * in the duplicate IDs, return true, otherwise, return false.
-	 */
-	for (i = 0; i < nr_duplicate_ids; i++) {
-		if (duplicate_processor_ids[i] == proc_id)
-			return true;
-	}
-	return false;
-}
-
 void __init acpi_processor_init(void)
 {
 	acpi_processor_check_duplicates();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 968173ec2726..dd4591dc1eb3 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -285,9 +285,6 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id)
 	return phys_id == PHYS_CPUID_INVALID;
 }
 
-/* Validate the processor object's proc_id */
-bool acpi_duplicate_processor_id(int proc_id);
-
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
-- 
2.14.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-20 11:04 ` [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus Dou Liyang
@ 2018-03-20 12:37   ` Peter Zijlstra
  2018-03-21  5:33     ` Dou Liyang
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2018-03-20 12:37 UTC (permalink / raw)
  To: Dou Liyang
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

On Tue, Mar 20, 2018 at 07:04:28PM +0800, Dou Liyang wrote:

> +	possible_cpus=	[s390,x86_64] Use this to set hotpluggable cpus.
> +			This option sets possible_cpus bits in cpu_possible_map.
> +			Thus keeping the numbers of bits set constant even if
> +			the machine gets rebooted.

That description, esp. the last sentence, doesn't make any kind of sense
to me. What?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
  2018-03-20 11:04 ` [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map() Dou Liyang
@ 2018-03-20 12:39   ` Peter Zijlstra
  2018-03-21  5:38     ` Dou Liyang
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2018-03-20 12:39 UTC (permalink / raw)
  To: Dou Liyang
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

On Tue, Mar 20, 2018 at 07:04:30PM +0800, Dou Liyang wrote:
> case 1: no  | no  | no | -->  min (setup_possible_cpus, nr_cpu_ids, setup_max_cpus)
> case 2: no  | no  | yes| -->  min (setup_possible_cpus, nr_cpu_ids)
> case 3: no  | yes | no | -->  1                                    
> case 4: no  | yes | yes| -->  1
> case 5: yes | no  | no | -->  min (num_processors, nr_cpu_ids, setup_max_cpus)
> case 6: yes | no  | yes| -->  min (num_processors + disabled_cpus, nr_cpu_ids)
> case 7: yes | yes | no | -->  1 
> case 8: yes | yes | yes| -->  1

The case number is off by one ;-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-20 12:37   ` Peter Zijlstra
@ 2018-03-21  5:33     ` Dou Liyang
  2018-03-21  9:08       ` Peter Zijlstra
  0 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-03-21  5:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

Hi Peter,

At 03/20/2018 08:37 PM, Peter Zijlstra wrote:
> On Tue, Mar 20, 2018 at 07:04:28PM +0800, Dou Liyang wrote:
> 
>> +	possible_cpus=	[s390,x86_64] Use this to set hotpluggable cpus.
>> +			This option sets possible_cpus bits in cpu_possible_map.
>> +			Thus keeping the numbers of bits set constant even if
>> +			the machine gets rebooted.
> 
> That description, esp. the last sentence, doesn't make any kind of sense
> to me. What?
> 

Ah, sure enough, I can't be lazy. :-) I stole that from the commit
   3b11ce7f542e ("x86: use possible_cpus=NUM to extend the possible cpus 
allowed")

How about:

possible_cpus=	[s390,x86_64] Set the number of possible CPUs which
		are determined by the ACPI tables MADT or mptables by
		default. possible_cpus=n : n >= 1 enforces the possible
		number to be 'n'.
		While nr_cpus is also be set: nr_cpus=m, choice the
		minimum one for the number of possible CPUs.

Thank,
	dou

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
  2018-03-20 12:39   ` Peter Zijlstra
@ 2018-03-21  5:38     ` Dou Liyang
  0 siblings, 0 replies; 18+ messages in thread
From: Dou Liyang @ 2018-03-21  5:38 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

Hi Peter,

At 03/20/2018 08:39 PM, Peter Zijlstra wrote:
> On Tue, Mar 20, 2018 at 07:04:30PM +0800, Dou Liyang wrote:
>> case 1: no  | no  | no | -->  min (setup_possible_cpus, nr_cpu_ids, setup_max_cpus)
>> case 2: no  | no  | yes| -->  min (setup_possible_cpus, nr_cpu_ids)
>> case 3: no  | yes | no | -->  1
>> case 4: no  | yes | yes| -->  1
>> case 5: yes | no  | no | -->  min (num_processors, nr_cpu_ids, setup_max_cpus)
>> case 6: yes | no  | yes| -->  min (num_processors + disabled_cpus, nr_cpu_ids)
>> case 7: yes | yes | no | -->  1
>> case 8: yes | yes | yes| -->  1
> 
> The case number is off by one ;-)
> 

I got it! ;-)

Thanks
	dou
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-21  5:33     ` Dou Liyang
@ 2018-03-21  9:08       ` Peter Zijlstra
  2018-03-21  9:34         ` Dou Liyang
  2018-03-21  9:41         ` Thomas Gleixner
  0 siblings, 2 replies; 18+ messages in thread
From: Peter Zijlstra @ 2018-03-21  9:08 UTC (permalink / raw)
  To: Dou Liyang
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

On Wed, Mar 21, 2018 at 01:33:24PM +0800, Dou Liyang wrote:
> How about:
> 
> possible_cpus=	[s390,x86_64] Set the number of possible CPUs which
> 		are determined by the ACPI tables MADT or mptables by
> 		default. possible_cpus=n : n >= 1 enforces the possible
> 		number to be 'n'.
> 		While nr_cpus is also be set: nr_cpus=m, choice the
> 		minimum one for the number of possible CPUs.

So what is the exact difference between possible_cpus and nr_cpus ? I
konw maxcpus= limits the number of CPUs we bring up, and possible_cpus
limits the possible_map, but I'm not entirely sure what nr_cpus does
here.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-21  9:08       ` Peter Zijlstra
@ 2018-03-21  9:34         ` Dou Liyang
  2018-03-21  9:41           ` Dou Liyang
  2018-03-21  9:41         ` Thomas Gleixner
  1 sibling, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-03-21  9:34 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa

Hi Peter,

At 03/21/2018 05:08 PM, Peter Zijlstra wrote:
> On Wed, Mar 21, 2018 at 01:33:24PM +0800, Dou Liyang wrote:
>> How about:
>>
>> possible_cpus=	[s390,x86_64] Set the number of possible CPUs which
>> 		are determined by the ACPI tables MADT or mptables by
>> 		default. possible_cpus=n : n >= 1 enforces the possible
>> 		number to be 'n'.
>> 		While nr_cpus is also be set: nr_cpus=m, choice the
>> 		minimum one for the number of possible CPUs.
> 
> So what is the exact difference between possible_cpus and nr_cpus ? I

the possible_cpus= can reset the number of possible CPUs, even bigger
than 'num_processors + disabled_cpus', But nr_cpus= can't.

> konw maxcpus= limits the number of CPUs we bring up, and possible_cpus
> limits the possible_map, but I'm not entirely sure what nr_cpus does
> here.
> 

nr_cpus can limited the maximum CPUs that the kernel could support.

Here is a double check in case of using them at the same time, even if I
think just using possible_cpus= is enough. :-)

Thanks,
	dou.
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-21  9:08       ` Peter Zijlstra
  2018-03-21  9:34         ` Dou Liyang
@ 2018-03-21  9:41         ` Thomas Gleixner
  1 sibling, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2018-03-21  9:41 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Dou Liyang, linux-kernel, x86, linux-acpi, linux-doc, mingo,
	corbet, rjw, lenb, hpa

On Wed, 21 Mar 2018, Peter Zijlstra wrote:
> On Wed, Mar 21, 2018 at 01:33:24PM +0800, Dou Liyang wrote:
> > How about:
> > 
> > possible_cpus=	[s390,x86_64] Set the number of possible CPUs which
> > 		are determined by the ACPI tables MADT or mptables by
> > 		default. possible_cpus=n : n >= 1 enforces the possible
> > 		number to be 'n'.
> > 		While nr_cpus is also be set: nr_cpus=m, choice the
> > 		minimum one for the number of possible CPUs.
> 
> So what is the exact difference between possible_cpus and nr_cpus ? I
> konw maxcpus= limits the number of CPUs we bring up, and possible_cpus
> limits the possible_map, but I'm not entirely sure what nr_cpus does
> here.

nr_cpus limits the number of CPUs the kernel will handle. Think of it as a
boot time override of NR_CPUs.

Way too many commandline switches though.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
  2018-03-21  9:34         ` Dou Liyang
@ 2018-03-21  9:41           ` Dou Liyang
  0 siblings, 0 replies; 18+ messages in thread
From: Dou Liyang @ 2018-03-21  9:41 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, x86, linux-acpi, linux-doc, tglx, mingo, corbet,
	rjw, lenb, hpa



At 03/21/2018 05:34 PM, Dou Liyang wrote:
> Hi Peter,
> 
> At 03/21/2018 05:08 PM, Peter Zijlstra wrote:
>> On Wed, Mar 21, 2018 at 01:33:24PM +0800, Dou Liyang wrote:
>>> How about:
>>>
>>> possible_cpus=    [s390,x86_64] Set the number of possible CPUs which
>>>         are determined by the ACPI tables MADT or mptables by
>>>         default. possible_cpus=n : n >= 1 enforces the possible
>>>         number to be 'n'.
>>>         While nr_cpus is also be set: nr_cpus=m, choice the
>>>         minimum one for the number of possible CPUs.
>>
>> So what is the exact difference between possible_cpus and nr_cpus ? I
> 
> the possible_cpus= can reset the number of possible CPUs, even bigger
> than 'num_processors + disabled_cpus', But nr_cpus= can't.
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       the maximum number kernel gets from ACPI/mptables, no matter what
number nr_cpus= is, the number of possible CPUs will not bigger than it.

>
>> konw maxcpus= limits the number of CPUs we bring up, and possible_cpus
>> limits the possible_map, but I'm not entirely sure what nr_cpus does
>> here.
>>
> 
> nr_cpus can limited the maximum CPUs that the kernel could support.
> 
> Here is a double check in case of using them at the same time, even if I
> think just using possible_cpus= is enough. :-)
> 
> Thanks,
>      dou.
>>
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
  2018-03-20 11:04 ` [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk() Dou Liyang
@ 2018-05-19 15:06   ` Thomas Gleixner
  2018-05-22  1:47     ` Dou Liyang
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2018-05-19 15:06 UTC (permalink / raw)
  To: Dou Liyang
  Cc: LKML, x86, linux-acpi, linux-doc, Ingo Molnar, Jonathan Corbet,
	Rafael J. Wysocki, Len Brown, H. Peter Anvin, Peter Zijlstra

On Tue, 20 Mar 2018, Dou Liyang wrote:

> ACPI driver should make sure all the processor IDs in their ACPI Namespace
> are unique for CPU hotplug. the driver performs a depth-first walk of the
> namespace tree and calls the acpi_processor_ids_walk().
> 
> But, the acpi_processor_ids_walk() will return true if one processor is
> checked, that cause the walk break after walking pass the first processor.
> 
> Repace the value with AE_OK which is the standard acpi_status value.
> 
> Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for processor enumeration")
> 
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> ---
>  drivers/acpi/acpi_processor.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 449d86d39965..db5bdb59639c 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -663,11 +663,11 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
>  	}
>  
>  	processor_validated_ids_update(uid);
> -	return true;
> +	return AE_OK;
>  
>  err:
>  	acpi_handle_info(handle, "Invalid processor object\n");
> -	return false;
> +	return AE_OK;

I'm not sure whether this is the right return value here. Rafael?

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
  2018-05-19 15:06   ` Thomas Gleixner
@ 2018-05-22  1:47     ` Dou Liyang
  2018-05-23  1:34       ` Dou Liyang
  0 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-05-22  1:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, x86, linux-acpi, linux-doc, Ingo Molnar, Jonathan Corbet,
	Rafael J. Wysocki, Len Brown, H. Peter Anvin, Peter Zijlstra



At 05/19/2018 11:06 PM, Thomas Gleixner wrote:
> On Tue, 20 Mar 2018, Dou Liyang wrote:
> 
>> ACPI driver should make sure all the processor IDs in their ACPI Namespace
>> are unique for CPU hotplug. the driver performs a depth-first walk of the
>> namespace tree and calls the acpi_processor_ids_walk().
>>
>> But, the acpi_processor_ids_walk() will return true if one processor is
>> checked, that cause the walk break after walking pass the first processor.
>>
>> Repace the value with AE_OK which is the standard acpi_status value.
>>
>> Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for processor enumeration")
>>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>> ---
>>   drivers/acpi/acpi_processor.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
>> index 449d86d39965..db5bdb59639c 100644
>> --- a/drivers/acpi/acpi_processor.c
>> +++ b/drivers/acpi/acpi_processor.c
>> @@ -663,11 +663,11 @@ static acpi_status __init (acpi_handle handle,
>>   	}
>>   
>>   	processor_validated_ids_update(uid);
>> -	return true;
>> +	return AE_OK;
>>   
>>   err:
>>   	acpi_handle_info(handle, "Invalid processor object\n");
>> -	return false;
>> +	return AE_OK;
> 
> I'm not sure whether this is the right return value here. Rafael?
> 
Hi, Thomas, Rafael,

Yes, I used AE_OK to make sure it can skip the invalid objects and
continue to do the following other objects, I'm also not sure.

For this bug, recently, I sent another patch to remove this check code
away.

    https://lkml.org/lkml/2018/5/17/320

IMO, the duplicate IDs can be avoid by the other code

    if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) ---- 1)

As the mapping of cpu_id(pr->id) and processor_id is fixed, when
hot-plugging a physical CPU, if its processor_id is duplicated with the
present, the above condition 1) will be 0, and Linux will do not add
this CPU.

And, when every time the system starts, this code will be executed, it
will waste more time with the increase in the number of CPU.

So I prefer to remove this code.

Thanks,
	dou

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
  2018-05-22  1:47     ` Dou Liyang
@ 2018-05-23  1:34       ` Dou Liyang
  2018-05-23  8:08         ` Rafael J. Wysocki
  0 siblings, 1 reply; 18+ messages in thread
From: Dou Liyang @ 2018-05-23  1:34 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, x86, linux-acpi, linux-doc, Ingo Molnar, Jonathan Corbet,
	Rafael J. Wysocki, Len Brown, H. Peter Anvin, Peter Zijlstra,
	Rafael J. Wysocki, Rafael J. Wysocki

At 05/22/2018 09:47 AM, Dou Liyang wrote:
> 
> 
> At 05/19/2018 11:06 PM, Thomas Gleixner wrote:
>> On Tue, 20 Mar 2018, Dou Liyang wrote:
>>
>>> ACPI driver should make sure all the processor IDs in their ACPI 
>>> Namespace
>>> are unique for CPU hotplug. the driver performs a depth-first walk of 
>>> the
>>> namespace tree and calls the acpi_processor_ids_walk().
>>>
>>> But, the acpi_processor_ids_walk() will return true if one processor is
>>> checked, that cause the walk break after walking pass the first 
>>> processor.
>>>
>>> Repace the value with AE_OK which is the standard acpi_status value.
>>>
>>> Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for 
>>> processor enumeration")
>>>
>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>> ---
>>>   drivers/acpi/acpi_processor.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/acpi_processor.c 
>>> b/drivers/acpi/acpi_processor.c
>>> index 449d86d39965..db5bdb59639c 100644
>>> --- a/drivers/acpi/acpi_processor.c
>>> +++ b/drivers/acpi/acpi_processor.c
>>> @@ -663,11 +663,11 @@ static acpi_status __init (acpi_handle handle,
>>>       }
>>>       processor_validated_ids_update(uid);
>>> -    return true;
>>> +    return AE_OK;
>>>   err:
>>>       acpi_handle_info(handle, "Invalid processor object\n");
>>> -    return false;
>>> +    return AE_OK;
>>
>> I'm not sure whether this is the right return value here. Rafael?
>>

+Cc Rafael's common used email address.

I am sorry, I created the cc list using ./script/get_maintainers.pl ...
and didn't check it.

Thanks,
	dou

> Hi, Thomas, Rafael,
> 
> Yes, I used AE_OK to make sure it can skip the invalid objects and
> continue to do the following other objects, I'm also not sure.
> 
> For this bug, recently, I sent another patch to remove this check code
> away.
> 
>     https://lkml.org/lkml/2018/5/17/320
> 
> IMO, the duplicate IDs can be avoid by the other code
> 
>     if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) ---- 1)
> 
> As the mapping of cpu_id(pr->id) and processor_id is fixed, when
> hot-plugging a physical CPU, if its processor_id is duplicated with the
> present, the above condition 1) will be 0, and Linux will do not add
> this CPU.
> 
> And, when every time the system starts, this code will be executed, it
> will waste more time with the increase in the number of CPU.
> 
> So I prefer to remove this code.
> 
> Thanks,
>      dou
> 
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
  2018-05-23  1:34       ` Dou Liyang
@ 2018-05-23  8:08         ` Rafael J. Wysocki
  0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2018-05-23  8:08 UTC (permalink / raw)
  To: Dou Liyang
  Cc: Thomas Gleixner, LKML, the arch/x86 maintainers,
	ACPI Devel Maling List, open list:DOCUMENTATION, Ingo Molnar,
	Jonathan Corbet, Rafael J. Wysocki, Len Brown, H. Peter Anvin,
	Peter Zijlstra, Rafael J. Wysocki, Rafael J. Wysocki

On Wed, May 23, 2018 at 3:34 AM, Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> At 05/22/2018 09:47 AM, Dou Liyang wrote:
>>
>>
>>
>> At 05/19/2018 11:06 PM, Thomas Gleixner wrote:
>>>
>>> On Tue, 20 Mar 2018, Dou Liyang wrote:
>>>
>>>> ACPI driver should make sure all the processor IDs in their ACPI
>>>> Namespace
>>>> are unique for CPU hotplug. the driver performs a depth-first walk of
>>>> the
>>>> namespace tree and calls the acpi_processor_ids_walk().
>>>>
>>>> But, the acpi_processor_ids_walk() will return true if one processor is
>>>> checked, that cause the walk break after walking pass the first
>>>> processor.
>>>>
>>>> Repace the value with AE_OK which is the standard acpi_status value.
>>>>
>>>> Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for
>>>> processor enumeration")
>>>>
>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>> ---
>>>>   drivers/acpi/acpi_processor.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/acpi_processor.c
>>>> b/drivers/acpi/acpi_processor.c
>>>> index 449d86d39965..db5bdb59639c 100644
>>>> --- a/drivers/acpi/acpi_processor.c
>>>> +++ b/drivers/acpi/acpi_processor.c
>>>> @@ -663,11 +663,11 @@ static acpi_status __init (acpi_handle handle,
>>>>       }
>>>>       processor_validated_ids_update(uid);
>>>> -    return true;
>>>> +    return AE_OK;
>>>>   err:
>>>>       acpi_handle_info(handle, "Invalid processor object\n");
>>>> -    return false;
>>>> +    return AE_OK;
>>>
>>>
>>> I'm not sure whether this is the right return value here. Rafael?
>>>
>
> +Cc Rafael's common used email address.
>
> I am sorry, I created the cc list using ./script/get_maintainers.pl ...
> and didn't check it.

No worries, I saw your messages, but thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-05-23  8:08 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-20 11:04 [PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup Dou Liyang
2018-03-20 11:04 ` [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus Dou Liyang
2018-03-20 12:37   ` Peter Zijlstra
2018-03-21  5:33     ` Dou Liyang
2018-03-21  9:08       ` Peter Zijlstra
2018-03-21  9:34         ` Dou Liyang
2018-03-21  9:41           ` Dou Liyang
2018-03-21  9:41         ` Thomas Gleixner
2018-03-20 11:04 ` [PATCH 2/5] x86/cpu_hotplug: Update the link of cpu_hotplug.rst Dou Liyang
2018-03-20 11:04 ` [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map() Dou Liyang
2018-03-20 12:39   ` Peter Zijlstra
2018-03-21  5:38     ` Dou Liyang
2018-03-20 11:04 ` [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk() Dou Liyang
2018-05-19 15:06   ` Thomas Gleixner
2018-05-22  1:47     ` Dou Liyang
2018-05-23  1:34       ` Dou Liyang
2018-05-23  8:08         ` Rafael J. Wysocki
2018-03-20 11:04 ` [PATCH 5/5] acpi/processor: Make the acpi_duplicate_processor_id() static Dou Liyang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).