diff mbox series

[kvm-unit-tests,v3,7/9] s390x: smp: Remove unneeded cpu loops

Message ID 20200117104640.1983-8-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: smp: Improve smp code and reset checks | expand

Commit Message

Janosch Frank Jan. 17, 2020, 10:46 a.m. UTC
Now that we have a loop which is executed after we return from the
main function of a secondary cpu, we can remove the surplus loops.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/smp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Cornelia Huck Jan. 20, 2020, 11:29 a.m. UTC | #1
On Fri, 17 Jan 2020 05:46:38 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

> Now that we have a loop which is executed after we return from the
> main function of a secondary cpu, we can remove the surplus loops.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  s390x/smp.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 555ed72..c12a3db 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
>  	}
>  }
>  
> -static void cpu_loop(void)
> -{
> -	for (;;) {}
> -}
> -
>  static void test_func(void)
>  {
>  	testflag = 1;
> -	cpu_loop();
>  }
>  
>  static void test_start(void)
> @@ -234,7 +228,7 @@ int main(void)
>  
>  	/* Setting up the cpu to give it a stack and lowcore */
>  	psw.mask = extract_psw_mask();
> -	psw.addr = (unsigned long)cpu_loop;
> +	psw.addr = (unsigned long)test_func;

Before, you did not set testflag here... intended change?

>  	smp_cpu_setup(1, psw);
>  	smp_cpu_stop(1);
>
Janosch Frank Jan. 20, 2020, 2:41 p.m. UTC | #2
On 1/20/20 12:29 PM, Cornelia Huck wrote:
> On Fri, 17 Jan 2020 05:46:38 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Now that we have a loop which is executed after we return from the
>> main function of a secondary cpu, we can remove the surplus loops.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  s390x/smp.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/s390x/smp.c b/s390x/smp.c
>> index 555ed72..c12a3db 100644
>> --- a/s390x/smp.c
>> +++ b/s390x/smp.c
>> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
>>  	}
>>  }
>>  
>> -static void cpu_loop(void)
>> -{
>> -	for (;;) {}
>> -}
>> -
>>  static void test_func(void)
>>  {
>>  	testflag = 1;
>> -	cpu_loop();
>>  }
>>  
>>  static void test_start(void)
>> @@ -234,7 +228,7 @@ int main(void)
>>  
>>  	/* Setting up the cpu to give it a stack and lowcore */
>>  	psw.mask = extract_psw_mask();
>> -	psw.addr = (unsigned long)cpu_loop;
>> +	psw.addr = (unsigned long)test_func;
> 
> Before, you did not set testflag here... intended change?

Yes
It is set to 0 before the first test, so it shouldn't matter.

> 
>>  	smp_cpu_setup(1, psw);
>>  	smp_cpu_stop(1);
>>  
>
Cornelia Huck Jan. 20, 2020, 4:11 p.m. UTC | #3
On Mon, 20 Jan 2020 15:41:52 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 1/20/20 12:29 PM, Cornelia Huck wrote:
> > On Fri, 17 Jan 2020 05:46:38 -0500
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Now that we have a loop which is executed after we return from the
> >> main function of a secondary cpu, we can remove the surplus loops.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>  s390x/smp.c | 8 +-------
> >>  1 file changed, 1 insertion(+), 7 deletions(-)
> >>
> >> diff --git a/s390x/smp.c b/s390x/smp.c
> >> index 555ed72..c12a3db 100644
> >> --- a/s390x/smp.c
> >> +++ b/s390x/smp.c
> >> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
> >>  	}
> >>  }
> >>  
> >> -static void cpu_loop(void)
> >> -{
> >> -	for (;;) {}
> >> -}
> >> -
> >>  static void test_func(void)
> >>  {
> >>  	testflag = 1;
> >> -	cpu_loop();
> >>  }
> >>  
> >>  static void test_start(void)
> >> @@ -234,7 +228,7 @@ int main(void)
> >>  
> >>  	/* Setting up the cpu to give it a stack and lowcore */
> >>  	psw.mask = extract_psw_mask();
> >> -	psw.addr = (unsigned long)cpu_loop;
> >> +	psw.addr = (unsigned long)test_func;  
> > 
> > Before, you did not set testflag here... intended change?  
> 
> Yes
> It is set to 0 before the first test, so it shouldn't matter.

Hm... I got a bit lost in all those changes, so I checked your branch
on github, and I don't see it being set to 0 before test_start() is
called?

> 
> >   
> >>  	smp_cpu_setup(1, psw);
> >>  	smp_cpu_stop(1);
> >>    
> >   
> 
>
Janosch Frank Jan. 21, 2020, 12:46 p.m. UTC | #4
On 1/20/20 5:11 PM, Cornelia Huck wrote:
> On Mon, 20 Jan 2020 15:41:52 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 1/20/20 12:29 PM, Cornelia Huck wrote:
>>> On Fri, 17 Jan 2020 05:46:38 -0500
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> Now that we have a loop which is executed after we return from the
>>>> main function of a secondary cpu, we can remove the surplus loops.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>  s390x/smp.c | 8 +-------
>>>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>>>
>>>> diff --git a/s390x/smp.c b/s390x/smp.c
>>>> index 555ed72..c12a3db 100644
>>>> --- a/s390x/smp.c
>>>> +++ b/s390x/smp.c
>>>> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
>>>>  	}
>>>>  }
>>>>  
>>>> -static void cpu_loop(void)
>>>> -{
>>>> -	for (;;) {}
>>>> -}
>>>> -
>>>>  static void test_func(void)
>>>>  {
>>>>  	testflag = 1;
>>>> -	cpu_loop();
>>>>  }
>>>>  
>>>>  static void test_start(void)
>>>> @@ -234,7 +228,7 @@ int main(void)
>>>>  
>>>>  	/* Setting up the cpu to give it a stack and lowcore */
>>>>  	psw.mask = extract_psw_mask();
>>>> -	psw.addr = (unsigned long)cpu_loop;
>>>> +	psw.addr = (unsigned long)test_func;  
>>>
>>> Before, you did not set testflag here... intended change?  
>>
>> Yes
>> It is set to 0 before the first test, so it shouldn't matter.
> 
> Hm... I got a bit lost in all those changes, so I checked your branch
> on github, and I don't see it being set to 0 before test_start() is
> called?

Well, that's because test_start doesn't care about the flag.
ecall and emcall are the first users, and they set it to 0 before using it.

> 
>>
>>>   
>>>>  	smp_cpu_setup(1, psw);
>>>>  	smp_cpu_stop(1);
>>>>    
>>>   
>>
>>
>
Cornelia Huck Jan. 21, 2020, 12:59 p.m. UTC | #5
On Tue, 21 Jan 2020 13:46:51 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 1/20/20 5:11 PM, Cornelia Huck wrote:
> > On Mon, 20 Jan 2020 15:41:52 +0100
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> On 1/20/20 12:29 PM, Cornelia Huck wrote:  
> >>> On Fri, 17 Jan 2020 05:46:38 -0500
> >>> Janosch Frank <frankja@linux.ibm.com> wrote:
> >>>     
> >>>> Now that we have a loop which is executed after we return from the
> >>>> main function of a secondary cpu, we can remove the surplus loops.
> >>>>
> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>>> ---
> >>>>  s390x/smp.c | 8 +-------
> >>>>  1 file changed, 1 insertion(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/s390x/smp.c b/s390x/smp.c
> >>>> index 555ed72..c12a3db 100644
> >>>> --- a/s390x/smp.c
> >>>> +++ b/s390x/smp.c
> >>>> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
> >>>>  	}
> >>>>  }
> >>>>  
> >>>> -static void cpu_loop(void)
> >>>> -{
> >>>> -	for (;;) {}
> >>>> -}
> >>>> -
> >>>>  static void test_func(void)
> >>>>  {
> >>>>  	testflag = 1;
> >>>> -	cpu_loop();
> >>>>  }
> >>>>  
> >>>>  static void test_start(void)
> >>>> @@ -234,7 +228,7 @@ int main(void)
> >>>>  
> >>>>  	/* Setting up the cpu to give it a stack and lowcore */
> >>>>  	psw.mask = extract_psw_mask();
> >>>> -	psw.addr = (unsigned long)cpu_loop;
> >>>> +	psw.addr = (unsigned long)test_func;    
> >>>
> >>> Before, you did not set testflag here... intended change?    
> >>
> >> Yes
> >> It is set to 0 before the first test, so it shouldn't matter.  
> > 
> > Hm... I got a bit lost in all those changes, so I checked your branch
> > on github, and I don't see it being set to 0 before test_start() is
> > called?  
> 
> Well, that's because test_start doesn't care about the flag.

But I see a wait_for_flag() in there? What am I missing?

> ecall and emcall are the first users, and they set it to 0 before using it.
> 
> >   
> >>  
> >>>     
> >>>>  	smp_cpu_setup(1, psw);
> >>>>  	smp_cpu_stop(1);
> >>>>      
> >>>     
> >>
> >>  
> >   
> 
>
Janosch Frank Jan. 21, 2020, 1:07 p.m. UTC | #6
On 1/21/20 1:59 PM, Cornelia Huck wrote:
> On Tue, 21 Jan 2020 13:46:51 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 1/20/20 5:11 PM, Cornelia Huck wrote:
>>> On Mon, 20 Jan 2020 15:41:52 +0100
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> On 1/20/20 12:29 PM, Cornelia Huck wrote:  
>>>>> On Fri, 17 Jan 2020 05:46:38 -0500
>>>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>>>     
>>>>>> Now that we have a loop which is executed after we return from the
>>>>>> main function of a secondary cpu, we can remove the surplus loops.
>>>>>>
>>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>>> ---
>>>>>>  s390x/smp.c | 8 +-------
>>>>>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/s390x/smp.c b/s390x/smp.c
>>>>>> index 555ed72..c12a3db 100644
>>>>>> --- a/s390x/smp.c
>>>>>> +++ b/s390x/smp.c
>>>>>> @@ -29,15 +29,9 @@ static void wait_for_flag(void)
>>>>>>  	}
>>>>>>  }
>>>>>>  
>>>>>> -static void cpu_loop(void)
>>>>>> -{
>>>>>> -	for (;;) {}
>>>>>> -}
>>>>>> -
>>>>>>  static void test_func(void)
>>>>>>  {
>>>>>>  	testflag = 1;
>>>>>> -	cpu_loop();
>>>>>>  }
>>>>>>  
>>>>>>  static void test_start(void)
>>>>>> @@ -234,7 +228,7 @@ int main(void)
>>>>>>  
>>>>>>  	/* Setting up the cpu to give it a stack and lowcore */
>>>>>>  	psw.mask = extract_psw_mask();
>>>>>> -	psw.addr = (unsigned long)cpu_loop;
>>>>>> +	psw.addr = (unsigned long)test_func;    
>>>>>
>>>>> Before, you did not set testflag here... intended change?    
>>>>
>>>> Yes
>>>> It is set to 0 before the first test, so it shouldn't matter.  
>>>
>>> Hm... I got a bit lost in all those changes, so I checked your branch
>>> on github, and I don't see it being set to 0 before test_start() is
>>> called?  
>>
>> Well, that's because test_start doesn't care about the flag.
> 
> But I see a wait_for_flag() in there? What am I missing?
> 
>> ecall and emcall are the first users, and they set it to 0 before using it.

Well, cpu #1 will update tesflag to 1 in ecall() and emcall()

>>
>>>   
>>>>  
>>>>>     
>>>>>>  	smp_cpu_setup(1, psw);
>>>>>>  	smp_cpu_stop(1);
>>>>>>      
>>>>>     
>>>>
>>>>  
>>>   
>>
>>
>
diff mbox series

Patch

diff --git a/s390x/smp.c b/s390x/smp.c
index 555ed72..c12a3db 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -29,15 +29,9 @@  static void wait_for_flag(void)
 	}
 }
 
-static void cpu_loop(void)
-{
-	for (;;) {}
-}
-
 static void test_func(void)
 {
 	testflag = 1;
-	cpu_loop();
 }
 
 static void test_start(void)
@@ -234,7 +228,7 @@  int main(void)
 
 	/* Setting up the cpu to give it a stack and lowcore */
 	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)cpu_loop;
+	psw.addr = (unsigned long)test_func;
 	smp_cpu_setup(1, psw);
 	smp_cpu_stop(1);