diff mbox series

[2/4] arm64: mm: fix the count comments in compute_indices

Message ID 20210518101405.1048860-3-aisheng.dong@nxp.com (mailing list archive)
State New, archived
Headers show
Series arm64: mm: a few small cleanups | expand

Commit Message

Dong Aisheng May 18, 2021, 10:14 a.m. UTC
'count - 1' is confusing and not comply with the real code running.
'count' actually represents the extra entries required, no need minus 1.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 arch/arm64/kernel/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Rutland May 18, 2021, 12:35 p.m. UTC | #1
On Tue, May 18, 2021 at 06:14:03PM +0800, Dong Aisheng wrote:
> 'count - 1' is confusing and not comply with the real code running.
> 'count' actually represents the extra entries required, no need minus 1.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

This looks right to me, but I'm not all that familiar with this code.

Steve, does this make sense to you?

Mark.

> ---
>  arch/arm64/kernel/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 96873dfa67fd..b70db34458ec 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -195,7 +195,7 @@ SYM_CODE_END(preserve_boot_args)
>  	and	\iend, \iend, \istart	// iend = (vend >> shift) & (ptrs - 1)
>  	mov	\istart, \ptrs
>  	mul	\istart, \istart, \count
> -	add	\iend, \iend, \istart	// iend += (count - 1) * ptrs
> +	add	\iend, \iend, \istart	// iend += count * ptrs
>  					// our entries span multiple tables
>  
>  	lsr	\istart, \vstart, \shift
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas June 15, 2021, 1:29 p.m. UTC | #2
On Tue, May 18, 2021 at 01:35:11PM +0100, Mark Rutland wrote:
> On Tue, May 18, 2021 at 06:14:03PM +0800, Dong Aisheng wrote:
> > 'count - 1' is confusing and not comply with the real code running.
> > 'count' actually represents the extra entries required, no need minus 1.
> > 
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> 
> This looks right to me, but I'm not all that familiar with this code.
> 
> Steve, does this make sense to you?
> 
> Mark.
> 
> > ---
> >  arch/arm64/kernel/head.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 96873dfa67fd..b70db34458ec 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -195,7 +195,7 @@ SYM_CODE_END(preserve_boot_args)
> >  	and	\iend, \iend, \istart	// iend = (vend >> shift) & (ptrs - 1)
> >  	mov	\istart, \ptrs
> >  	mul	\istart, \istart, \count
> > -	add	\iend, \iend, \istart	// iend += (count - 1) * ptrs
> > +	add	\iend, \iend, \istart	// iend += count * ptrs
> >  					// our entries span multiple tables

The comment was updated to match the code. The code also looks ok to me:
iend is inclusive and count denotes the extra entries required rather
than the actual number of entries.

Also cc'ing Will, I think he missed this series.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
diff mbox series

Patch

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 96873dfa67fd..b70db34458ec 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -195,7 +195,7 @@  SYM_CODE_END(preserve_boot_args)
 	and	\iend, \iend, \istart	// iend = (vend >> shift) & (ptrs - 1)
 	mov	\istart, \ptrs
 	mul	\istart, \istart, \count
-	add	\iend, \iend, \istart	// iend += (count - 1) * ptrs
+	add	\iend, \iend, \istart	// iend += count * ptrs
 					// our entries span multiple tables
 
 	lsr	\istart, \vstart, \shift