The 'addr' need not be incremented while operating on single entry basis as
BBM is not required for such updates.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/hugetlbpage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 0c8737f4f2ce..1d90a7e75333 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -225,7 +225,7 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
 	ncontig = num_contig_ptes(sz, &pgsize);
 
 	if (!pte_present(pte)) {
-		for (i = 0; i < ncontig; i++, ptep++, addr += pgsize)
+		for (i = 0; i < ncontig; i++, ptep++)
 			__set_ptes_anysz(mm, ptep, pte, 1, pgsize);
 		return;
 	}
-- 
2.30.2

On 15/07/25 10:09 am, Anshuman Khandual wrote:
> The 'addr' need not be incremented while operating on single entry basis as
> BBM is not required for such updates.

"while operating on a single entry basis" should be replaced with
"when transitioning from invalid to valid pte"; BBM is required
for single entry updates when the cont bit is set, as described
in the comment immediately below. In fact, let us not even mention
BBM at all - the reasoning is that the code is written in a way
that, the loop incrementing addr, immediately has a return after
it, and it does not actually use the addr, so let's drop the
increment. It is not immediately obvious from the commit description
that the addr is passed ultimately to flush_tlb_range friends which
is involved in BBM.

Reviewed-by: Dev Jain <dev.jain@arm.com>

>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   arch/arm64/mm/hugetlbpage.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 0c8737f4f2ce..1d90a7e75333 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -225,7 +225,7 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>   	ncontig = num_contig_ptes(sz, &pgsize);
>   
>   	if (!pte_present(pte)) {
> -		for (i = 0; i < ncontig; i++, ptep++, addr += pgsize)
> +		for (i = 0; i < ncontig; i++, ptep++)
>   			__set_ptes_anysz(mm, ptep, pte, 1, pgsize);
>   		return;
>   	}

On 15/07/25 11:28 AM, Dev Jain wrote:
> 
> On 15/07/25 10:09 am, Anshuman Khandual wrote:
>> The 'addr' need not be incremented while operating on single entry basis as
>> BBM is not required for such updates.
> 
> "while operating on a single entry basis" should be replaced with
> "when transitioning from invalid to valid pte"; BBM is required
> for single entry updates when the cont bit is set, as described
> in the comment immediately below. In fact, let us not even mention
> BBM at all - the reasoning is that the code is written in a way
> that, the loop incrementing addr, immediately has a return after
> it, and it does not actually use the addr, so let's drop the
> increment. It is not immediately obvious from the commit description
> that the addr is passed ultimately to flush_tlb_range friends which
> is involved in BBM.

Agreed, the mention about BBM can be dropped as it is not really
relevant. Rather 'addr' variable being unused in the conditional
block, which exits right after is the real reason. I will update
the commit description as required.  

> 
> Reviewed-by: Dev Jain <dev.jain@arm.com>
> 
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   arch/arm64/mm/hugetlbpage.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index 0c8737f4f2ce..1d90a7e75333 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -225,7 +225,7 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>>       ncontig = num_contig_ptes(sz, &pgsize);
>>         if (!pte_present(pte)) {
>> -        for (i = 0; i < ncontig; i++, ptep++, addr += pgsize)
>> +        for (i = 0; i < ncontig; i++, ptep++)
>>               __set_ptes_anysz(mm, ptep, pte, 1, pgsize);
>>           return;
>>       }