diff mbox series

realtime/prio-preempt: take cpu isolation into consideration

Message ID 20240710032434.123315-1-shichen@redhat.com
State Superseded
Headers show
Series realtime/prio-preempt: take cpu isolation into consideration | expand

Commit Message

Shizhao Chen July 10, 2024, 3:24 a.m. UTC 
By default the test starts N-1 busy threads to monopolize N-1 available CPUs,
where N is the number of all available CPUs. However, when CPU isolation is
applied, this will lead to a hang scenario where all housekeeping CPUs are
hogged as isolated CPUs do not share the N-1 busy threads.

Signed-off-by: Shizhao Chen <shichen@redhat.com>
---
 .../realtime/func/prio-preempt/prio-preempt.c  | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Li Wang July 10, 2024, 6:13 a.m. UTC | #1 
Hi Shizhao,

On Wed, Jul 10, 2024 at 11:24 AM Shizhao Chen <shichen@redhat.com> wrote:

> By default the test starts N-1 busy threads to monopolize N-1 available
> CPUs,
> where N is the number of all available CPUs. However, when CPU isolation is
> applied, this will lead to a hang scenario where all housekeeping CPUs are
> hogged as isolated CPUs do not share the N-1 busy threads.
>
> Signed-off-by: Shizhao Chen <shichen@redhat.com>
> ---
>  .../realtime/func/prio-preempt/prio-preempt.c  | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/realtime/func/prio-preempt/prio-preempt.c
> b/testcases/realtime/func/prio-preempt/prio-preempt.c
> index 9bd5e7ad8..79d2115a7 100644
> --- a/testcases/realtime/func/prio-preempt/prio-preempt.c
> +++ b/testcases/realtime/func/prio-preempt/prio-preempt.c
> @@ -59,6 +59,7 @@
>   *
>
> *****************************************************************************/
>
> +#define _GNU_SOURCE
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <signal.h>
> @@ -287,6 +288,17 @@ void *master_thread(void *arg)
>         return NULL;
>  }
>
> +int get_numcpus(void)
> +{
> +       cpu_set_t mask;
> +       CPU_ZERO(&mask);
> +
> +       /* Get the number of cpus accessible to the current process */
> +       sched_getaffinity(getpid(), sizeof(mask), &mask);
> +
> +       return CPU_COUNT(&mask);
> +}

+
>  int main(int argc, char *argv[])
>  {
>         int pri_boost, numcpus;
> @@ -295,10 +307,10 @@ int main(int argc, char *argv[])
>         pass_criteria = CHECK_LIMIT;
>         rt_init("hin:", parse_args, argc, argv);
>
> -       numcpus = sysconf(_SC_NPROCESSORS_ONLN);
> +       numcpus = get_numcpus();
>

Maybe we could make use of LTP lib function tst_ncpus_available directly?
Seems the new get_numcpus() did a dup work on that.

Otherwise, looks good to me.


> -       /* Max no. of busy threads should always be less than/equal the
> no. of cpus
> -          Otherwise, the box will hang */
> +       /* Max no. of busy threads should always be less than/equal the
> no. of
> +          housekeeping cpus. Otherwise, the box will hang */
>
>         if (rt_threads == -1 || rt_threads > numcpus) {
>                 rt_threads = numcpus;
> --
> 2.45.2
>
>
Li Wang July 10, 2024, 6:27 a.m. UTC | #2 
On Wed, Jul 10, 2024 at 2:13 PM Li Wang <liwang@redhat.com> wrote:

> Hi Shizhao,
>
> On Wed, Jul 10, 2024 at 11:24 AM Shizhao Chen <shichen@redhat.com> wrote:
>
>> By default the test starts N-1 busy threads to monopolize N-1 available
>> CPUs,
>> where N is the number of all available CPUs. However, when CPU isolation
>> is
>> applied, this will lead to a hang scenario where all housekeeping CPUs are
>> hogged as isolated CPUs do not share the N-1 busy threads.
>>
>> Signed-off-by: Shizhao Chen <shichen@redhat.com>
>> ---
>>  .../realtime/func/prio-preempt/prio-preempt.c  | 18 +++++++++++++++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/testcases/realtime/func/prio-preempt/prio-preempt.c
>> b/testcases/realtime/func/prio-preempt/prio-preempt.c
>> index 9bd5e7ad8..79d2115a7 100644
>> --- a/testcases/realtime/func/prio-preempt/prio-preempt.c
>> +++ b/testcases/realtime/func/prio-preempt/prio-preempt.c
>> @@ -59,6 +59,7 @@
>>   *
>>
>> *****************************************************************************/
>>
>> +#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <signal.h>
>> @@ -287,6 +288,17 @@ void *master_thread(void *arg)
>>         return NULL;
>>  }
>>
>> +int get_numcpus(void)
>> +{
>> +       cpu_set_t mask;
>> +       CPU_ZERO(&mask);
>> +
>> +       /* Get the number of cpus accessible to the current process */
>> +       sched_getaffinity(getpid(), sizeof(mask), &mask);
>> +
>> +       return CPU_COUNT(&mask);
>> +}
>
> +
>>  int main(int argc, char *argv[])
>>  {
>>         int pri_boost, numcpus;
>> @@ -295,10 +307,10 @@ int main(int argc, char *argv[])
>>         pass_criteria = CHECK_LIMIT;
>>         rt_init("hin:", parse_args, argc, argv);
>>
>> -       numcpus = sysconf(_SC_NPROCESSORS_ONLN);
>> +       numcpus = get_numcpus();
>>
>
> Maybe we could make use of LTP lib function tst_ncpus_available directly?
> Seems the new get_numcpus() did a dup work on that.
>

Oops, seems the real-time tests does not include ltp library.
So you patch make sense from this point, but maybe we
need to refine the function to consider more situations.

e.g. uses a fixed-size cpu_set_t structure, which might not be
sufficient on systems with a large number of CPUs.



>
> Otherwise, looks good to me.
>
>
>> -       /* Max no. of busy threads should always be less than/equal the
>> no. of cpus
>> -          Otherwise, the box will hang */
>> +       /* Max no. of busy threads should always be less than/equal the
>> no. of
>> +          housekeeping cpus. Otherwise, the box will hang */
>>
>>         if (rt_threads == -1 || rt_threads > numcpus) {
>>                 rt_threads = numcpus;
>> --
>> 2.45.2
>>
>>
>
> --
> Regards,
> Li Wang
>
Shizhao Chen July 10, 2024, 1:16 p.m. UTC | #3 
Thanks for the review! I just sent a v2 which is basically a minimized
version of tst_ncpus_available.

On Wed, Jul 10, 2024 at 2:27 PM Li Wang <liwang@redhat.com> wrote:
>
>
>
> On Wed, Jul 10, 2024 at 2:13 PM Li Wang <liwang@redhat.com> wrote:
>>
>> Hi Shizhao,
>>
>> On Wed, Jul 10, 2024 at 11:24 AM Shizhao Chen <shichen@redhat.com> wrote:
>>>
>>> By default the test starts N-1 busy threads to monopolize N-1 available CPUs,
>>> where N is the number of all available CPUs. However, when CPU isolation is
>>> applied, this will lead to a hang scenario where all housekeeping CPUs are
>>> hogged as isolated CPUs do not share the N-1 busy threads.
>>>
>>> Signed-off-by: Shizhao Chen <shichen@redhat.com>
>>> ---
>>>  .../realtime/func/prio-preempt/prio-preempt.c  | 18 +++++++++++++++---
>>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/testcases/realtime/func/prio-preempt/prio-preempt.c b/testcases/realtime/func/prio-preempt/prio-preempt.c
>>> index 9bd5e7ad8..79d2115a7 100644
>>> --- a/testcases/realtime/func/prio-preempt/prio-preempt.c
>>> +++ b/testcases/realtime/func/prio-preempt/prio-preempt.c
>>> @@ -59,6 +59,7 @@
>>>   *
>>>   *****************************************************************************/
>>>
>>> +#define _GNU_SOURCE
>>>  #include <stdio.h>
>>>  #include <stdlib.h>
>>>  #include <signal.h>
>>> @@ -287,6 +288,17 @@ void *master_thread(void *arg)
>>>         return NULL;
>>>  }
>>>
>>> +int get_numcpus(void)
>>> +{
>>> +       cpu_set_t mask;
>>> +       CPU_ZERO(&mask);
>>> +
>>> +       /* Get the number of cpus accessible to the current process */
>>> +       sched_getaffinity(getpid(), sizeof(mask), &mask);
>>> +
>>> +       return CPU_COUNT(&mask);
>>> +}
>>>
>>> +
>>>  int main(int argc, char *argv[])
>>>  {
>>>         int pri_boost, numcpus;
>>> @@ -295,10 +307,10 @@ int main(int argc, char *argv[])
>>>         pass_criteria = CHECK_LIMIT;
>>>         rt_init("hin:", parse_args, argc, argv);
>>>
>>> -       numcpus = sysconf(_SC_NPROCESSORS_ONLN);
>>> +       numcpus = get_numcpus();
>>
>>
>> Maybe we could make use of LTP lib function tst_ncpus_available directly?
>> Seems the new get_numcpus() did a dup work on that.
>
>
> Oops, seems the real-time tests does not include ltp library.
> So you patch make sense from this point, but maybe we
> need to refine the function to consider more situations.
>
> e.g. uses a fixed-size cpu_set_t structure, which might not be
> sufficient on systems with a large number of CPUs.
>
>
>>
>>
>> Otherwise, looks good to me.
>>
>>>
>>> -       /* Max no. of busy threads should always be less than/equal the no. of cpus
>>> -          Otherwise, the box will hang */
>>> +       /* Max no. of busy threads should always be less than/equal the no. of
>>> +          housekeeping cpus. Otherwise, the box will hang */
>>>
>>>         if (rt_threads == -1 || rt_threads > numcpus) {
>>>                 rt_threads = numcpus;
>>> --
>>> 2.45.2
>>>
>>
>>
>> --
>> Regards,
>> Li Wang
>
>
>
> --
> Regards,
> Li Wang
diff mbox series

Patch

diff --git a/testcases/realtime/func/prio-preempt/prio-preempt.c b/testcases/realtime/func/prio-preempt/prio-preempt.c
index 9bd5e7ad8..79d2115a7 100644
--- a/testcases/realtime/func/prio-preempt/prio-preempt.c
+++ b/testcases/realtime/func/prio-preempt/prio-preempt.c
@@ -59,6 +59,7 @@ 
  *
  *****************************************************************************/
 
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
 #include <signal.h>
@@ -287,6 +288,17 @@  void *master_thread(void *arg)
 	return NULL;
 }
 
+int get_numcpus(void)
+{
+	cpu_set_t mask;
+	CPU_ZERO(&mask);
+
+	/* Get the number of cpus accessible to the current process */
+	sched_getaffinity(getpid(), sizeof(mask), &mask);
+
+	return CPU_COUNT(&mask);
+}
+
 int main(int argc, char *argv[])
 {
 	int pri_boost, numcpus;
@@ -295,10 +307,10 @@  int main(int argc, char *argv[])
 	pass_criteria = CHECK_LIMIT;
 	rt_init("hin:", parse_args, argc, argv);
 
-	numcpus = sysconf(_SC_NPROCESSORS_ONLN);
+	numcpus = get_numcpus();
 
-	/* Max no. of busy threads should always be less than/equal the no. of cpus
-	   Otherwise, the box will hang */
+	/* Max no. of busy threads should always be less than/equal the no. of
+	   housekeeping cpus. Otherwise, the box will hang */
 
 	if (rt_threads == -1 || rt_threads > numcpus) {
 		rt_threads = numcpus;