All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
@ 2026-07-01 18:27 Nikhil Kumar Singh
  2026-07-01 18:38 ` Aditya Gupta
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nikhil Kumar Singh @ 2026-07-01 18:27 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-kernel, maddy, mpe, npiggin, chleroy, adityag, mahesh,
	Nikhil Kumar Singh

The value loaded into r0 in copy_and_flush() represents the number of
8-byte words processed between cache synchronization operations.

The existing comment refers to cache line size, which can make it appear
that the value is a cache line size in bytes rather than a loop count.
Clarify the comment to explain that the loop processes 8 words (64 bytes)
per cache synchronization iteration, and that increasing the value would
skip cache maintenance for intermediate cache lines.

This is a comment-only change with no functional impact.

Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
---
 arch/powerpc/kernel/head_64.S | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 63432a33ec49..e21c2bce8f7e 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -713,14 +713,14 @@ p_end: .8byte _end - copy_to_here
 _GLOBAL(copy_and_flush)
 	addi	r5,r5,-8
 	addi	r6,r6,-8
-4:	li	r0,8			/* Use the smallest common	*/
-					/* denominator cache line	*/
-					/* size.  This results in	*/
-					/* extra cache line flushes	*/
-					/* but operation is correct.	*/
-					/* Can't get cache line size	*/
-					/* from NACA as it is being	*/
-					/* moved too.			*/
+4:	li	r0,8			/* r0 is the number of 8-byte words       */
+					/* to copy per cache sync iteration.      */
+					/* 8 words * 8 bytes = 64 bytes. 64B is   */
+					/* the current default cache line size.   */
+					/* This is a loop count, not a byte       */
+					/* count. Increasing it will skip         */
+					/* dcbst/icbi for lines in between and    */
+					/* leave stale instructions in icache.    */
 
 	mtctr	r0			/* put # words/line in ctr	*/
 3:	addi	r6,r6,8			/* copy a cache line		*/
-- 
2.43.5



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

* Re: [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-01 18:27 [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment Nikhil Kumar Singh
@ 2026-07-01 18:38 ` Aditya Gupta
  2026-07-02  5:28 ` Mukesh Kumar Chaurasiya
  2026-07-06  8:27 ` [PATCH v2] " Nikhil Kumar Singh
  2 siblings, 0 replies; 10+ messages in thread
From: Aditya Gupta @ 2026-07-01 18:38 UTC (permalink / raw)
  To: Nikhil Kumar Singh, linuxppc-dev
  Cc: linux-kernel, maddy, mpe, npiggin, chleroy, mahesh


On 01/07/26 23:57, Nikhil Kumar Singh wrote:
> The value loaded into r0 in copy_and_flush() represents the number of
> 8-byte words processed between cache synchronization operations.
>
> The existing comment refers to cache line size, which can make it appear
> that the value is a cache line size in bytes rather than a loop count.
> Clarify the comment to explain that the loop processes 8 words (64 bytes)
> per cache synchronization iteration, and that increasing the value would
> skip cache maintenance for intermediate cache lines.
>
> This is a comment-only change with no functional impact.
>
> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> ---
>   arch/powerpc/kernel/head_64.S | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 63432a33ec49..e21c2bce8f7e 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -713,14 +713,14 @@ p_end: .8byte _end - copy_to_here
>   _GLOBAL(copy_and_flush)
>   	addi	r5,r5,-8
>   	addi	r6,r6,-8
> -4:	li	r0,8			/* Use the smallest common	*/
> -					/* denominator cache line	*/
> -					/* size.  This results in	*/
> -					/* extra cache line flushes	*/
> -					/* but operation is correct.	*/
> -					/* Can't get cache line size	*/
> -					/* from NACA as it is being	*/
> -					/* moved too.			*/
> +4:	li	r0,8			/* r0 is the number of 8-byte words       */
> +					/* to copy per cache sync iteration.      */
> +					/* 8 words * 8 bytes = 64 bytes. 64B is   */
> +					/* the current default cache line size.   */
> +					/* This is a loop count, not a byte       */
> +					/* count. Increasing it will skip         */
> +					/* dcbst/icbi for lines in between and    */
> +					/* leave stale instructions in icache.    */

Looks good to me, previous comment stating cache line size may make it 
look like
r0 is number of bytes being copied which isn't the case when the code is 
read.
Hence:

Reviewed-by: Aditya Gupta <adityag@linux.ibm.com>


Thanks,
- Aditya G

>   
>   	mtctr	r0			/* put # words/line in ctr	*/
>   3:	addi	r6,r6,8			/* copy a cache line		*/


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

* Re: [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-01 18:27 [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment Nikhil Kumar Singh
  2026-07-01 18:38 ` Aditya Gupta
@ 2026-07-02  5:28 ` Mukesh Kumar Chaurasiya
  2026-07-06  8:27 ` [PATCH v2] " Nikhil Kumar Singh
  2 siblings, 0 replies; 10+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-07-02  5:28 UTC (permalink / raw)
  To: Nikhil Kumar Singh
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

On Wed, Jul 01, 2026 at 11:57:56PM +0530, Nikhil Kumar Singh wrote:
> The value loaded into r0 in copy_and_flush() represents the number of
> 8-byte words processed between cache synchronization operations.
> 
> The existing comment refers to cache line size, which can make it appear
> that the value is a cache line size in bytes rather than a loop count.
> Clarify the comment to explain that the loop processes 8 words (64 bytes)
> per cache synchronization iteration, and that increasing the value would
> skip cache maintenance for intermediate cache lines.
> 
> This is a comment-only change with no functional impact.
> 
> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> ---
>  arch/powerpc/kernel/head_64.S | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 63432a33ec49..e21c2bce8f7e 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -713,14 +713,14 @@ p_end: .8byte _end - copy_to_here
>  _GLOBAL(copy_and_flush)
>  	addi	r5,r5,-8
>  	addi	r6,r6,-8
> -4:	li	r0,8			/* Use the smallest common	*/
> -					/* denominator cache line	*/
> -					/* size.  This results in	*/
> -					/* extra cache line flushes	*/
> -					/* but operation is correct.	*/
> -					/* Can't get cache line size	*/
> -					/* from NACA as it is being	*/
> -					/* moved too.			*/
> +4:	li	r0,8			/* r0 is the number of 8-byte words       */
> +					/* to copy per cache sync iteration.      */
> +					/* 8 words * 8 bytes = 64 bytes. 64B is   */
> +					/* the current default cache line size.   */
> +					/* This is a loop count, not a byte       */
> +					/* count. Increasing it will skip         */
> +					/* dcbst/icbi for lines in between and    */
> +					/* leave stale instructions in icache.    */
>  
You dropped 2 pieces of information here, That we can't use NACA and
that we are doing an extra cache line flush(atleast in case of P10).

Also use tab instead of spaces before commment close per line "*/" can
you keep the last character aligned to rest of the file. It looks
little nicer.

Regards,
Mukesh
>  	mtctr	r0			/* put # words/line in ctr	*/
>  3:	addi	r6,r6,8			/* copy a cache line		*/
> -- 
> 2.43.5
> 

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

* [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-01 18:27 [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment Nikhil Kumar Singh
  2026-07-01 18:38 ` Aditya Gupta
  2026-07-02  5:28 ` Mukesh Kumar Chaurasiya
@ 2026-07-06  8:27 ` Nikhil Kumar Singh
  2026-07-07  5:38   ` Mukesh Kumar Chaurasiya
  2 siblings, 1 reply; 10+ messages in thread
From: Nikhil Kumar Singh @ 2026-07-06  8:27 UTC (permalink / raw)
  To: linuxppc-dev, mkchauras
  Cc: linux-kernel, maddy, mpe, npiggin, chleroy, adityag, mahesh,
	Nikhil Kumar Singh

The value loaded into r0 in copy_and_flush() represents the number of
8-byte words processed between cache synchronization operations.

The existing comment refers to cache line size, which can make it appear
that the value is a cache line size in bytes rather than a loop count.
Clarify the comment to explain that the loop processes 8 words (64 bytes)
per cache synchronization iteration, and that increasing the value would
skip cache maintenance for intermediate cache lines.

This is a comment-only change with no functional impact.

Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
---
 arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 63432a33ec49..a54f6f979173 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
 _GLOBAL(copy_and_flush)
 	addi	r5,r5,-8
 	addi	r6,r6,-8
-4:	li	r0,8			/* Use the smallest common	*/
-					/* denominator cache line	*/
-					/* size.  This results in	*/
-					/* extra cache line flushes	*/
-					/* but operation is correct.	*/
-					/* Can't get cache line size	*/
-					/* from NACA as it is being	*/
-					/* moved too.			*/
+4:	li	r0,8			/* r0 is the number of 8-byte words	*/
+					/* to copy per cache sync iteration.	*/
+					/* 8 words * 8 bytes = 64 bytes. 64B is	*/
+					/* the current default cache line size.	*/
+					/* This is a loop count, not a byte	*/
+					/* count. Increasing it may skip	*/
+					/* dcbst/icbi for lines in between and	*/
+					/* leave stale instructions in icache.	*/
+					/* This results in extra cache line	*/
+					/* flushes but operation is correct.	*/
+					/* Can't get cache line size from NACA	*/
+					/* as it is being moved too.		*/
 
 	mtctr	r0			/* put # words/line in ctr	*/
 3:	addi	r6,r6,8			/* copy a cache line		*/
-- 
2.43.5



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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-06  8:27 ` [PATCH v2] " Nikhil Kumar Singh
@ 2026-07-07  5:38   ` Mukesh Kumar Chaurasiya
  2026-07-07  7:18     ` Nikhil Kumar Singh
  0 siblings, 1 reply; 10+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-07-07  5:38 UTC (permalink / raw)
  To: Nikhil Kumar Singh
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
> The value loaded into r0 in copy_and_flush() represents the number of
> 8-byte words processed between cache synchronization operations.
> 
> The existing comment refers to cache line size, which can make it appear
> that the value is a cache line size in bytes rather than a loop count.
> Clarify the comment to explain that the loop processes 8 words (64 bytes)
> per cache synchronization iteration, and that increasing the value would
> skip cache maintenance for intermediate cache lines.
> 
> This is a comment-only change with no functional impact.
> 
> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> ---
>  arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 63432a33ec49..a54f6f979173 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
>  _GLOBAL(copy_and_flush)
>  	addi	r5,r5,-8
>  	addi	r6,r6,-8
> -4:	li	r0,8			/* Use the smallest common	*/
> -					/* denominator cache line	*/
> -					/* size.  This results in	*/
> -					/* extra cache line flushes	*/
> -					/* but operation is correct.	*/
> -					/* Can't get cache line size	*/
> -					/* from NACA as it is being	*/
> -					/* moved too.			*/
> +4:	li	r0,8			/* r0 is the number of 8-byte words	*/
> +					/* to copy per cache sync iteration.	*/
> +					/* 8 words * 8 bytes = 64 bytes. 64B is	*/
> +					/* the current default cache line size.	*/
> +					/* This is a loop count, not a byte	*/
> +					/* count. Increasing it may skip	*/
> +					/* dcbst/icbi for lines in between and	*/
> +					/* leave stale instructions in icache.	*/
> +					/* This results in extra cache line	*/
> +					/* flushes but operation is correct.	*/
> +					/* Can't get cache line size from NACA	*/
> +					/* as it is being moved too.		*/
The comment looks fine. But the alignement of the comment is still
different from earlier. Keep the alignment same as earlier, it looks a
little nicer.

Regards,
Mukesh
>  
>  	mtctr	r0			/* put # words/line in ctr	*/
>  3:	addi	r6,r6,8			/* copy a cache line		*/
> -- 
> 2.43.5
> 


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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-07  5:38   ` Mukesh Kumar Chaurasiya
@ 2026-07-07  7:18     ` Nikhil Kumar Singh
  2026-07-07  7:24       ` Mukesh Kumar Chaurasiya
  2026-07-09  6:10       ` Nikhil Kumar Singh
  0 siblings, 2 replies; 10+ messages in thread
From: Nikhil Kumar Singh @ 2026-07-07  7:18 UTC (permalink / raw)
  To: Mukesh Kumar Chaurasiya
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

Hi Mukesh,

Thanks for reviewing the patch and for the suggestion.
I'd prefer to keep the current alignment as I find it a bit more readable.

Thanks,
~ Nikhil

On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote:
> On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
>> The value loaded into r0 in copy_and_flush() represents the number of
>> 8-byte words processed between cache synchronization operations.
>>
>> The existing comment refers to cache line size, which can make it appear
>> that the value is a cache line size in bytes rather than a loop count.
>> Clarify the comment to explain that the loop processes 8 words (64 bytes)
>> per cache synchronization iteration, and that increasing the value would
>> skip cache maintenance for intermediate cache lines.
>>
>> This is a comment-only change with no functional impact.
>>
>> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
>> ---
>>   arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
>>   1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
>> index 63432a33ec49..a54f6f979173 100644
>> --- a/arch/powerpc/kernel/head_64.S
>> +++ b/arch/powerpc/kernel/head_64.S
>> @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
>>   _GLOBAL(copy_and_flush)
>>   	addi	r5,r5,-8
>>   	addi	r6,r6,-8
>> -4:	li	r0,8			/* Use the smallest common	*/
>> -					/* denominator cache line	*/
>> -					/* size.  This results in	*/
>> -					/* extra cache line flushes	*/
>> -					/* but operation is correct.	*/
>> -					/* Can't get cache line size	*/
>> -					/* from NACA as it is being	*/
>> -					/* moved too.			*/
>> +4:	li	r0,8			/* r0 is the number of 8-byte words	*/
>> +					/* to copy per cache sync iteration.	*/
>> +					/* 8 words * 8 bytes = 64 bytes. 64B is	*/
>> +					/* the current default cache line size.	*/
>> +					/* This is a loop count, not a byte	*/
>> +					/* count. Increasing it may skip	*/
>> +					/* dcbst/icbi for lines in between and	*/
>> +					/* leave stale instructions in icache.	*/
>> +					/* This results in extra cache line	*/
>> +					/* flushes but operation is correct.	*/
>> +					/* Can't get cache line size from NACA	*/
>> +					/* as it is being moved too.		*/
> The comment looks fine. But the alignement of the comment is still
> different from earlier. Keep the alignment same as earlier, it looks a
> little nicer.
>
> Regards,
> Mukesh
>>   
>>   	mtctr	r0			/* put # words/line in ctr	*/
>>   3:	addi	r6,r6,8			/* copy a cache line		*/
>> -- 
>> 2.43.5
>>

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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-07  7:18     ` Nikhil Kumar Singh
@ 2026-07-07  7:24       ` Mukesh Kumar Chaurasiya
  2026-07-07  7:32         ` Nikhil Kumar Singh
  2026-07-09  6:10       ` Nikhil Kumar Singh
  1 sibling, 1 reply; 10+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-07-07  7:24 UTC (permalink / raw)
  To: Nikhil Kumar Singh
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

On Tue, Jul 07, 2026 at 12:48:57PM +0530, Nikhil Kumar Singh wrote:
> Hi Mukesh,
> 
> Thanks for reviewing the patch and for the suggestion.
> I'd prefer to keep the current alignment as I find it a bit more readable.
> 
> Thanks,
> ~ Nikhil
> 
There is a reason we keep the character limit to 80 chars. Please go
through these coding guidelines.

https://docs.kernel.org/process/coding-style.html

Regards,
Mukesh

> On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote:
> > On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
> > > The value loaded into r0 in copy_and_flush() represents the number of
> > > 8-byte words processed between cache synchronization operations.
> > > 
> > > The existing comment refers to cache line size, which can make it appear
> > > that the value is a cache line size in bytes rather than a loop count.
> > > Clarify the comment to explain that the loop processes 8 words (64 bytes)
> > > per cache synchronization iteration, and that increasing the value would
> > > skip cache maintenance for intermediate cache lines.
> > > 
> > > This is a comment-only change with no functional impact.
> > > 
> > > Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> > > ---
> > >   arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
> > >   1 file changed, 12 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> > > index 63432a33ec49..a54f6f979173 100644
> > > --- a/arch/powerpc/kernel/head_64.S
> > > +++ b/arch/powerpc/kernel/head_64.S
> > > @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
> > >   _GLOBAL(copy_and_flush)
> > >   	addi	r5,r5,-8
> > >   	addi	r6,r6,-8
> > > -4:	li	r0,8			/* Use the smallest common	*/
> > > -					/* denominator cache line	*/
> > > -					/* size.  This results in	*/
> > > -					/* extra cache line flushes	*/
> > > -					/* but operation is correct.	*/
> > > -					/* Can't get cache line size	*/
> > > -					/* from NACA as it is being	*/
> > > -					/* moved too.			*/
> > > +4:	li	r0,8			/* r0 is the number of 8-byte words	*/
> > > +					/* to copy per cache sync iteration.	*/
> > > +					/* 8 words * 8 bytes = 64 bytes. 64B is	*/
> > > +					/* the current default cache line size.	*/
> > > +					/* This is a loop count, not a byte	*/
> > > +					/* count. Increasing it may skip	*/
> > > +					/* dcbst/icbi for lines in between and	*/
> > > +					/* leave stale instructions in icache.	*/
> > > +					/* This results in extra cache line	*/
> > > +					/* flushes but operation is correct.	*/
> > > +					/* Can't get cache line size from NACA	*/
> > > +					/* as it is being moved too.		*/
> > The comment looks fine. But the alignement of the comment is still
> > different from earlier. Keep the alignment same as earlier, it looks a
> > little nicer.
> > 
> > Regards,
> > Mukesh
> > >   	mtctr	r0			/* put # words/line in ctr	*/
> > >   3:	addi	r6,r6,8			/* copy a cache line		*/
> > > -- 
> > > 2.43.5
> > > 
> 


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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-07  7:24       ` Mukesh Kumar Chaurasiya
@ 2026-07-07  7:32         ` Nikhil Kumar Singh
  0 siblings, 0 replies; 10+ messages in thread
From: Nikhil Kumar Singh @ 2026-07-07  7:32 UTC (permalink / raw)
  To: linuxppc-dev

Hi Mukesh,

Thanks for the clarification, sorry I missed that part. I'll update the 
comment to follow the 80-column limit and keep the alignment consistent 
in the next revision.

Thanks,
~ Nikhil

On 07/07/26 12:54, Mukesh Kumar Chaurasiya wrote:
> On Tue, Jul 07, 2026 at 12:48:57PM +0530, Nikhil Kumar Singh wrote:
>> Hi Mukesh,
>>
>> Thanks for reviewing the patch and for the suggestion.
>> I'd prefer to keep the current alignment as I find it a bit more readable.
>>
>> Thanks,
>> ~ Nikhil
>>
> There is a reason we keep the character limit to 80 chars. Please go
> through these coding guidelines.
>
> https://docs.kernel.org/process/coding-style.html
>
> Regards,
> Mukesh
>
>> On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote:
>>> On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
>>>> The value loaded into r0 in copy_and_flush() represents the number of
>>>> 8-byte words processed between cache synchronization operations.
>>>>
>>>> The existing comment refers to cache line size, which can make it appear
>>>> that the value is a cache line size in bytes rather than a loop count.
>>>> Clarify the comment to explain that the loop processes 8 words (64 bytes)
>>>> per cache synchronization iteration, and that increasing the value would
>>>> skip cache maintenance for intermediate cache lines.
>>>>
>>>> This is a comment-only change with no functional impact.
>>>>
>>>> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
>>>> ---
>>>>    arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
>>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
>>>> index 63432a33ec49..a54f6f979173 100644
>>>> --- a/arch/powerpc/kernel/head_64.S
>>>> +++ b/arch/powerpc/kernel/head_64.S
>>>> @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
>>>>    _GLOBAL(copy_and_flush)
>>>>    	addi	r5,r5,-8
>>>>    	addi	r6,r6,-8
>>>> -4:	li	r0,8			/* Use the smallest common	*/
>>>> -					/* denominator cache line	*/
>>>> -					/* size.  This results in	*/
>>>> -					/* extra cache line flushes	*/
>>>> -					/* but operation is correct.	*/
>>>> -					/* Can't get cache line size	*/
>>>> -					/* from NACA as it is being	*/
>>>> -					/* moved too.			*/
>>>> +4:	li	r0,8			/* r0 is the number of 8-byte words	*/
>>>> +					/* to copy per cache sync iteration.	*/
>>>> +					/* 8 words * 8 bytes = 64 bytes. 64B is	*/
>>>> +					/* the current default cache line size.	*/
>>>> +					/* This is a loop count, not a byte	*/
>>>> +					/* count. Increasing it may skip	*/
>>>> +					/* dcbst/icbi for lines in between and	*/
>>>> +					/* leave stale instructions in icache.	*/
>>>> +					/* This results in extra cache line	*/
>>>> +					/* flushes but operation is correct.	*/
>>>> +					/* Can't get cache line size from NACA	*/
>>>> +					/* as it is being moved too.		*/
>>> The comment looks fine. But the alignement of the comment is still
>>> different from earlier. Keep the alignment same as earlier, it looks a
>>> little nicer.
>>>
>>> Regards,
>>> Mukesh
>>>>    	mtctr	r0			/* put # words/line in ctr	*/
>>>>    3:	addi	r6,r6,8			/* copy a cache line		*/
>>>> -- 
>>>> 2.43.5
>>>>


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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-07  7:18     ` Nikhil Kumar Singh
  2026-07-07  7:24       ` Mukesh Kumar Chaurasiya
@ 2026-07-09  6:10       ` Nikhil Kumar Singh
  2026-07-09  8:32         ` Mukesh Kumar Chaurasiya
  1 sibling, 1 reply; 10+ messages in thread
From: Nikhil Kumar Singh @ 2026-07-09  6:10 UTC (permalink / raw)
  To: Mukesh Kumar Chaurasiya
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

Hi Mukesh,

Ideally, checkpatch should take care of this, I see no checkpatch errors.

```
./scripts/checkpatch.pl 
v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch
total: 0 errors, 0 warnings, 26 lines checked

v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch has no 
obvious style problems and is ready for submission.
```

Thanks
~ Nikhil

On 07/07/26 12:48, Nikhil Kumar Singh wrote:
> Hi Mukesh,
>
> Thanks for reviewing the patch and for the suggestion.
> I'd prefer to keep the current alignment as I find it a bit more 
> readable.
>
> Thanks,
> ~ Nikhil
>
> On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote:
>> On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
>>> The value loaded into r0 in copy_and_flush() represents the number of
>>> 8-byte words processed between cache synchronization operations.
>>>
>>> The existing comment refers to cache line size, which can make it 
>>> appear
>>> that the value is a cache line size in bytes rather than a loop count.
>>> Clarify the comment to explain that the loop processes 8 words (64 
>>> bytes)
>>> per cache synchronization iteration, and that increasing the value 
>>> would
>>> skip cache maintenance for intermediate cache lines.
>>>
>>> This is a comment-only change with no functional impact.
>>>
>>> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
>>> ---
>>>   arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
>>>   1 file changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/head_64.S 
>>> b/arch/powerpc/kernel/head_64.S
>>> index 63432a33ec49..a54f6f979173 100644
>>> --- a/arch/powerpc/kernel/head_64.S
>>> +++ b/arch/powerpc/kernel/head_64.S
>>> @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
>>>   _GLOBAL(copy_and_flush)
>>>       addi    r5,r5,-8
>>>       addi    r6,r6,-8
>>> -4:    li    r0,8            /* Use the smallest common    */
>>> -                    /* denominator cache line    */
>>> -                    /* size.  This results in    */
>>> -                    /* extra cache line flushes    */
>>> -                    /* but operation is correct.    */
>>> -                    /* Can't get cache line size    */
>>> -                    /* from NACA as it is being    */
>>> -                    /* moved too.            */
>>> +4:    li    r0,8            /* r0 is the number of 8-byte words    */
>>> +                    /* to copy per cache sync iteration. */
>>> +                    /* 8 words * 8 bytes = 64 bytes. 64B is    */
>>> +                    /* the current default cache line size.    */
>>> +                    /* This is a loop count, not a byte    */
>>> +                    /* count. Increasing it may skip    */
>>> +                    /* dcbst/icbi for lines in between and */
>>> +                    /* leave stale instructions in icache. */
>>> +                    /* This results in extra cache line    */
>>> +                    /* flushes but operation is correct. */
>>> +                    /* Can't get cache line size from NACA */
>>> +                    /* as it is being moved too.        */
>> The comment looks fine. But the alignement of the comment is still
>> different from earlier. Keep the alignment same as earlier, it looks a
>> little nicer.
>>
>> Regards,
>> Mukesh
>>>         mtctr    r0            /* put # words/line in ctr    */
>>>   3:    addi    r6,r6,8            /* copy a cache line */
>>> -- 
>>> 2.43.5
>>>


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

* Re: [PATCH v2] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
  2026-07-09  6:10       ` Nikhil Kumar Singh
@ 2026-07-09  8:32         ` Mukesh Kumar Chaurasiya
  0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-07-09  8:32 UTC (permalink / raw)
  To: Nikhil Kumar Singh
  Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag,
	mahesh

On Thu, Jul 09, 2026 at 11:40:44AM +0530, Nikhil Kumar Singh wrote:
> Hi Mukesh,
> 
> Ideally, checkpatch should take care of this, I see no checkpatch errors.
> 
> ```
> ./scripts/checkpatch.pl
> v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch
> total: 0 errors, 0 warnings, 26 lines checked
> 
> v2-0001-powerpc-64s-Clarify-copy_and_flush-cache-sync-loo.patch has no
> obvious style problems and is ready for submission.
> ```
> 
> Thanks
> ~ Nikhil
> 
I am saying it looks nicer and easy to read for everyone if they are
aligned. If you think it's not readable with the 80 char limit, put the
whole comment before the function start. That will also makes it a
little nicer. Right now it's getting wrapped around and it's difficult
to read.

Regards,
Mukesh
> On 07/07/26 12:48, Nikhil Kumar Singh wrote:
> > Hi Mukesh,
> > 
> > Thanks for reviewing the patch and for the suggestion.
> > I'd prefer to keep the current alignment as I find it a bit more
> > readable.
> > 
> > Thanks,
> > ~ Nikhil
> > 
> > On 07/07/26 11:08, Mukesh Kumar Chaurasiya wrote:
> > > On Mon, Jul 06, 2026 at 01:57:08PM +0530, Nikhil Kumar Singh wrote:
> > > > The value loaded into r0 in copy_and_flush() represents the number of
> > > > 8-byte words processed between cache synchronization operations.
> > > > 
> > > > The existing comment refers to cache line size, which can make
> > > > it appear
> > > > that the value is a cache line size in bytes rather than a loop count.
> > > > Clarify the comment to explain that the loop processes 8 words
> > > > (64 bytes)
> > > > per cache synchronization iteration, and that increasing the
> > > > value would
> > > > skip cache maintenance for intermediate cache lines.
> > > > 
> > > > This is a comment-only change with no functional impact.
> > > > 
> > > > Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> > > > ---
> > > >   arch/powerpc/kernel/head_64.S | 20 ++++++++++++--------
> > > >   1 file changed, 12 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/arch/powerpc/kernel/head_64.S
> > > > b/arch/powerpc/kernel/head_64.S
> > > > index 63432a33ec49..a54f6f979173 100644
> > > > --- a/arch/powerpc/kernel/head_64.S
> > > > +++ b/arch/powerpc/kernel/head_64.S
> > > > @@ -713,14 +713,18 @@ p_end: .8byte _end - copy_to_here
> > > >   _GLOBAL(copy_and_flush)
> > > >       addi    r5,r5,-8
> > > >       addi    r6,r6,-8
> > > > -4:    li    r0,8            /* Use the smallest common    */
> > > > -                    /* denominator cache line    */
> > > > -                    /* size.  This results in    */
> > > > -                    /* extra cache line flushes    */
> > > > -                    /* but operation is correct.    */
> > > > -                    /* Can't get cache line size    */
> > > > -                    /* from NACA as it is being    */
> > > > -                    /* moved too.            */
> > > > +4:    li    r0,8            /* r0 is the number of 8-byte words    */
> > > > +                    /* to copy per cache sync iteration. */
> > > > +                    /* 8 words * 8 bytes = 64 bytes. 64B is    */
> > > > +                    /* the current default cache line size.    */
> > > > +                    /* This is a loop count, not a byte    */
> > > > +                    /* count. Increasing it may skip    */
> > > > +                    /* dcbst/icbi for lines in between and */
> > > > +                    /* leave stale instructions in icache. */
> > > > +                    /* This results in extra cache line    */
> > > > +                    /* flushes but operation is correct. */
> > > > +                    /* Can't get cache line size from NACA */
> > > > +                    /* as it is being moved too.        */
> > > The comment looks fine. But the alignement of the comment is still
> > > different from earlier. Keep the alignment same as earlier, it looks a
> > > little nicer.
> > > 
> > > Regards,
> > > Mukesh
> > > >         mtctr    r0            /* put # words/line in ctr    */
> > > >   3:    addi    r6,r6,8            /* copy a cache line */
> > > > -- 
> > > > 2.43.5
> > > > 


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

end of thread, other threads:[~2026-07-09  8:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 18:27 [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment Nikhil Kumar Singh
2026-07-01 18:38 ` Aditya Gupta
2026-07-02  5:28 ` Mukesh Kumar Chaurasiya
2026-07-06  8:27 ` [PATCH v2] " Nikhil Kumar Singh
2026-07-07  5:38   ` Mukesh Kumar Chaurasiya
2026-07-07  7:18     ` Nikhil Kumar Singh
2026-07-07  7:24       ` Mukesh Kumar Chaurasiya
2026-07-07  7:32         ` Nikhil Kumar Singh
2026-07-09  6:10       ` Nikhil Kumar Singh
2026-07-09  8:32         ` Mukesh Kumar Chaurasiya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.