LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] selftests: powerpc: Fix online CPU selection
@ 2020-06-09  7:37 Sandipan Das
  2020-07-29 13:47 ` Michael Ellerman
  2020-07-29 16:03 ` Srikar Dronamraju
  0 siblings, 2 replies; 4+ messages in thread
From: Sandipan Das @ 2020-06-09  7:37 UTC (permalink / raw)
  To: mpe; +Cc: srikar, kamalesh, shiganta, nasastry, harish, linuxppc-dev

The size of the CPU affinity mask must be large enough for
systems with a very large number of CPUs. Otherwise, tests
which try to determine the first online CPU by calling
sched_getaffinity() will fail. This makes sure that the size
of the allocated affinity mask is dependent on the number of
CPUs as reported by get_nprocs().

Fixes: 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs")
Reported-by: Shirisha Ganta <shiganta@in.ibm.com>
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
---
Previous versions can be found at:
v1: https://lore.kernel.org/linuxppc-dev/20200608144212.985144-1-sandipan@linux.ibm.com/

Changes in v2:
- Added NULL check for the affinity mask as suggested by Kamalesh.
- Changed "cpu set" to "CPU affinity mask" in the commit message.

---
 tools/testing/selftests/powerpc/utils.c | 37 +++++++++++++++++--------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/powerpc/utils.c b/tools/testing/selftests/powerpc/utils.c
index 933678f1ed0a..798fa8fdd5f4 100644
--- a/tools/testing/selftests/powerpc/utils.c
+++ b/tools/testing/selftests/powerpc/utils.c
@@ -16,6 +16,7 @@
 #include <string.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
+#include <sys/sysinfo.h>
 #include <sys/types.h>
 #include <sys/utsname.h>
 #include <unistd.h>
@@ -88,28 +89,40 @@ void *get_auxv_entry(int type)
 
 int pick_online_cpu(void)
 {
-	cpu_set_t mask;
-	int cpu;
+	int ncpus, cpu = -1;
+	cpu_set_t *mask;
+	size_t size;
+
+	ncpus = get_nprocs();
+	size = CPU_ALLOC_SIZE(ncpus);
+	mask = CPU_ALLOC(ncpus);
+	if (!mask) {
+		perror("malloc");
+		return -1;
+	}
 
-	CPU_ZERO(&mask);
+	CPU_ZERO_S(size, mask);
 
-	if (sched_getaffinity(0, sizeof(mask), &mask)) {
+	if (sched_getaffinity(0, size, mask)) {
 		perror("sched_getaffinity");
-		return -1;
+		goto done;
 	}
 
 	/* We prefer a primary thread, but skip 0 */
-	for (cpu = 8; cpu < CPU_SETSIZE; cpu += 8)
-		if (CPU_ISSET(cpu, &mask))
-			return cpu;
+	for (cpu = 8; cpu < ncpus; cpu += 8)
+		if (CPU_ISSET_S(cpu, size, mask))
+			goto done;
 
 	/* Search for anything, but in reverse */
-	for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--)
-		if (CPU_ISSET(cpu, &mask))
-			return cpu;
+	for (cpu = ncpus - 1; cpu >= 0; cpu--)
+		if (CPU_ISSET_S(cpu, size, mask))
+			goto done;
 
 	printf("No cpus in affinity mask?!\n");
-	return -1;
+
+done:
+	CPU_FREE(mask);
+	return cpu;
 }
 
 bool is_ppc64le(void)
-- 
2.25.1


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

* Re: [PATCH v2] selftests: powerpc: Fix online CPU selection
  2020-06-09  7:37 [PATCH v2] selftests: powerpc: Fix online CPU selection Sandipan Das
@ 2020-07-29 13:47 ` Michael Ellerman
  2020-07-29 16:03 ` Srikar Dronamraju
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-07-29 13:47 UTC (permalink / raw)
  To: Sandipan Das; +Cc: srikar, kamalesh, shiganta, nasastry, harish, linuxppc-dev

Sandipan Das <sandipan@linux.ibm.com> writes:
> The size of the CPU affinity mask must be large enough for
> systems with a very large number of CPUs. Otherwise, tests
> which try to determine the first online CPU by calling
> sched_getaffinity() will fail. This makes sure that the size
> of the allocated affinity mask is dependent on the number of
> CPUs as reported by get_nprocs().
>
> Fixes: 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs")
> Reported-by: Shirisha Ganta <shiganta@in.ibm.com>
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
> ---
> Previous versions can be found at:
> v1: https://lore.kernel.org/linuxppc-dev/20200608144212.985144-1-sandipan@linux.ibm.com/
>
> Changes in v2:
> - Added NULL check for the affinity mask as suggested by Kamalesh.
> - Changed "cpu set" to "CPU affinity mask" in the commit message.

This sometimes breaks, eg:

  # ./count_instructions 
  test: count_instructions
  tags: git_version:v5.8-rc2-327-g9a1d992a7eb7
  sched_getaffinity: Invalid argument
  [FAIL] Test FAILED on line 123
  failure: count_instructions


This system has a messed up SMT setup, but the old code was able to cope
with it:

  # ppc64_cpu --info
  Core   0:    0*    1*    2     3     4     5     6     7  
  Core   1:    8     9    10*   11*   12    13    14    15  
  Core   2:   16    17    18    19    20    21    22    23  
  Core   3:   24    25    26    27    28    29    30    31  
  Core   4:   32    33    34    35    36    37    38    39  
  Core   5:   40    41    42    43    44    45    46    47  
  Core   6:   48    49    50    51    52    53    54    55  
  Core   7:   56    57    58    59    60    61    62    63  
  Core   8:   64    65    66    67    68    69    70    71  
  Core   9:   72    73    74    75    76    77    78    79  
  Core  10:   80    81    82    83    84    85    86    87  
  Core  11:   88    89    90    91    92    93    94    95  
  Core  12:   96    97    98    99   100*  101*  102*  103* 
  Core  13:  104*  105*  106*  107*  108*  109*  110*  111* 
  Core  14:  112*  113*  114*  115*  116*  117*  118*  119* 
  Core  15:  120   121   122   123   124   125   126   127  
  Core  16:  128   129   130   131   132   133   134   135  
  Core  17:  136   137   138   139   140   141   142   143  
  Core  18:  144   145   146   147   148   149   150   151  
  Core  19:  152   153   154   155   156   157   158   159 

cheers

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

* Re: [PATCH v2] selftests: powerpc: Fix online CPU selection
  2020-06-09  7:37 [PATCH v2] selftests: powerpc: Fix online CPU selection Sandipan Das
  2020-07-29 13:47 ` Michael Ellerman
@ 2020-07-29 16:03 ` Srikar Dronamraju
  2020-07-30  4:59   ` Sandipan Das
  1 sibling, 1 reply; 4+ messages in thread
From: Srikar Dronamraju @ 2020-07-29 16:03 UTC (permalink / raw)
  To: Sandipan Das; +Cc: kamalesh, shiganta, nasastry, harish, linuxppc-dev

* Sandipan Das <sandipan@linux.ibm.com> [2020-06-09 13:07:33]:

> The size of the CPU affinity mask must be large enough for
> systems with a very large number of CPUs. Otherwise, tests
> which try to determine the first online CPU by calling
> sched_getaffinity() will fail. This makes sure that the size
> of the allocated affinity mask is dependent on the number of
> CPUs as reported by get_nprocs().
> 
> Fixes: 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs")
> Reported-by: Shirisha Ganta <shiganta@in.ibm.com>
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
> ---
> Previous versions can be found at:
> v1: https://lore.kernel.org/linuxppc-dev/20200608144212.985144-1-sandipan@linux.ibm.com/
> 
> @@ -88,28 +89,40 @@ void *get_auxv_entry(int type)
> 
>  int pick_online_cpu(void)
>  {
> -	cpu_set_t mask;
> -	int cpu;
> +	int ncpus, cpu = -1;
> +	cpu_set_t *mask;
> +	size_t size;
> +
> +	ncpus = get_nprocs();

Please use get_nprocs_conf or sysconf(_SC_NPROCESSORS_CONF). The manpage
seems to suggest the latter. Not sure how accurate the manpage is.

get_nprocs is returning online cpus and when smt is off, the cpu numbers
would be sparse and hence the result from get_nprocs wouldn't be ideal for
allocating cpumask. However get_nprocs_conf would return the max configured
cpus and would be able to handle it. 

I think this was the same situation hit by Michael Ellerman.

> +	size = CPU_ALLOC_SIZE(ncpus);
> +	mask = CPU_ALLOC(ncpus);
> +	if (!mask) {
> +		perror("malloc");
> +		return -1;
> +	}
> 

-- 
Thanks and Regards
Srikar Dronamraju

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

* Re: [PATCH v2] selftests: powerpc: Fix online CPU selection
  2020-07-29 16:03 ` Srikar Dronamraju
@ 2020-07-30  4:59   ` Sandipan Das
  0 siblings, 0 replies; 4+ messages in thread
From: Sandipan Das @ 2020-07-30  4:59 UTC (permalink / raw)
  To: Srikar Dronamraju, mpe; +Cc: shiganta, nasastry, harish, linuxppc-dev, kamalesh

Hi Srikar, Michael,

On 29/07/20 9:33 pm, Srikar Dronamraju wrote:
> * Sandipan Das <sandipan@linux.ibm.com> [2020-06-09 13:07:33]:
> 
>> The size of the CPU affinity mask must be large enough for
>> systems with a very large number of CPUs. Otherwise, tests
>> which try to determine the first online CPU by calling
>> sched_getaffinity() will fail. This makes sure that the size
>> of the allocated affinity mask is dependent on the number of
>> CPUs as reported by get_nprocs().
>>
>> Fixes: 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs")
>> Reported-by: Shirisha Ganta <shiganta@in.ibm.com>
>> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
>> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
>> ---
>> Previous versions can be found at:
>> v1: https://lore.kernel.org/linuxppc-dev/20200608144212.985144-1-sandipan@linux.ibm.com/
>>
>> @@ -88,28 +89,40 @@ void *get_auxv_entry(int type)
>>
>>  int pick_online_cpu(void)
>>  {
>> -	cpu_set_t mask;
>> -	int cpu;
>> +	int ncpus, cpu = -1;
>> +	cpu_set_t *mask;
>> +	size_t size;
>> +
>> +	ncpus = get_nprocs();
> 
> Please use get_nprocs_conf or sysconf(_SC_NPROCESSORS_CONF). The manpage
> seems to suggest the latter. Not sure how accurate the manpage is.
> 
> get_nprocs is returning online cpus and when smt is off, the cpu numbers
> would be sparse and hence the result from get_nprocs wouldn't be ideal for
> allocating cpumask. However get_nprocs_conf would return the max configured
> cpus and would be able to handle it. 
> 
> I think this was the same situation hit by Michael Ellerman.
> 

Yes, that seems to be the case. Thanks for testing this.
Will fix this in v3.

- Sandipan

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09  7:37 [PATCH v2] selftests: powerpc: Fix online CPU selection Sandipan Das
2020-07-29 13:47 ` Michael Ellerman
2020-07-29 16:03 ` Srikar Dronamraju
2020-07-30  4:59   ` Sandipan Das

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git