Initialize nr_pages to 1 at the start of the loop, similar to what is being
done in folio_referenced_one(). It may happen that the nr_pages computed
from a previous call to folio_unmap_pte_batch gets reused without again
going through folio_unmap_pte_batch, messing up things. Although, I don't
think there is any bug right now; a bug would have been there, if in the
same instance of a call to try_to_unmap_one, we end up in the
pte_present(pteval) branch, then in the else branch doing pte_clear() for
device-exclusive ptes. This means that a lazyfree folio has some present
entries and some device entries mapping it. Since a pte being
device-exclusive means that a GUP reference on the underlying folio is
held, the lazyfree unmapping path upon witnessing this will abort
try_to_unmap_one.

Signed-off-by: Dev Jain <dev.jain@arm.com>
---
 mm/rmap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 087c9f5b884fe..1fa020edd954a 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1982,7 +1982,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 	unsigned long end_addr;
 	unsigned long pfn;
 	unsigned long hsz = 0;
-	long nr_pages = 1;
+	long nr_pages;
 	int ptes = 0;
 
 	/*
@@ -2019,6 +2019,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 	mmu_notifier_invalidate_range_start(&range);
 
 	while (page_vma_mapped_walk(&pvmw)) {
+		nr_pages = 1;
+
 		/*
 		 * If the folio is in an mlock()d vma, we must not swap it out.
 		 */
-- 
2.34.1

On Tue, Mar 10, 2026 at 01:00:06PM +0530, Dev Jain wrote:
> Initialize nr_pages to 1 at the start of the loop, similar to what is being
> done in folio_referenced_one(). It may happen that the nr_pages computed
> from a previous call to folio_unmap_pte_batch gets reused without again
> going through folio_unmap_pte_batch, messing up things. Although, I don't
> think there is any bug right now; a bug would have been there, if in the
> same instance of a call to try_to_unmap_one, we end up in the
> pte_present(pteval) branch, then in the else branch doing pte_clear() for
> device-exclusive ptes. This means that a lazyfree folio has some present
> entries and some device entries mapping it. Since a pte being
> device-exclusive means that a GUP reference on the underlying folio is
> held, the lazyfree unmapping path upon witnessing this will abort
> try_to_unmap_one.

Dude, paragraphs. PARAGRAPHS :) this is one dense set of words.

It's also a very compressed 'stream of consciousness' hard-to-read block here.

I'm not sure it's really worth having this as a separate commit either, it's
pretty trivial.

>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  mm/rmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 087c9f5b884fe..1fa020edd954a 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1982,7 +1982,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  	unsigned long end_addr;
>  	unsigned long pfn;
>  	unsigned long hsz = 0;
> -	long nr_pages = 1;
> +	long nr_pages;
>  	int ptes = 0;
>
>  	/*
> @@ -2019,6 +2019,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  	mmu_notifier_invalidate_range_start(&range);
>
>  	while (page_vma_mapped_walk(&pvmw)) {
> +		nr_pages = 1;
> +

This seems valid but I really hate how we default-set it then in some branch set
it to something else.

But I think fixing that would be part of a bigger cleanup...

>  		/*
>  		 * If the folio is in an mlock()d vma, we must not swap it out.
>  		 */
> --
> 2.34.1
>

On 10/03/26 1:40 pm, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 10, 2026 at 01:00:06PM +0530, Dev Jain wrote:
>> Initialize nr_pages to 1 at the start of the loop, similar to what is being
>> done in folio_referenced_one(). It may happen that the nr_pages computed
>> from a previous call to folio_unmap_pte_batch gets reused without again
>> going through folio_unmap_pte_batch, messing up things. Although, I don't
>> think there is any bug right now; a bug would have been there, if in the
>> same instance of a call to try_to_unmap_one, we end up in the
>> pte_present(pteval) branch, then in the else branch doing pte_clear() for
>> device-exclusive ptes. This means that a lazyfree folio has some present
>> entries and some device entries mapping it. Since a pte being
>> device-exclusive means that a GUP reference on the underlying folio is
>> held, the lazyfree unmapping path upon witnessing this will abort
>> try_to_unmap_one.
> 
> Dude, paragraphs. PARAGRAPHS :) this is one dense set of words.
> 
> It's also a very compressed 'stream of consciousness' hard-to-read block here.

Sure :) I'll try to break this down.

> 
> I'm not sure it's really worth having this as a separate commit either, it's
> pretty trivial.

Hmm...well, as I explain above, it's not trivial for me :) it is difficult
for me to reason here whether nr_pages can be reused without a reset in
a future iteration.

> 
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
>>  mm/rmap.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 087c9f5b884fe..1fa020edd954a 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -1982,7 +1982,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>  	unsigned long end_addr;
>>  	unsigned long pfn;
>>  	unsigned long hsz = 0;
>> -	long nr_pages = 1;
>> +	long nr_pages;
>>  	int ptes = 0;
>>
>>  	/*
>> @@ -2019,6 +2019,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>  	mmu_notifier_invalidate_range_start(&range);
>>
>>  	while (page_vma_mapped_walk(&pvmw)) {
>> +		nr_pages = 1;
>> +
> 
> This seems valid but I really hate how we default-set it then in some branch set
> it to something else.
> 
> But I think fixing that would be part of a bigger cleanup...
> 
>>  		/*
>>  		 * If the folio is in an mlock()d vma, we must not swap it out.
>>  		 */
>> --
>> 2.34.1
>>

On Tue, Mar 10, 2026 at 02:01:44PM +0530, Dev Jain wrote:
>
>
> On 10/03/26 1:40 pm, Lorenzo Stoakes (Oracle) wrote:
> > On Tue, Mar 10, 2026 at 01:00:06PM +0530, Dev Jain wrote:
> >> Initialize nr_pages to 1 at the start of the loop, similar to what is being
> >> done in folio_referenced_one(). It may happen that the nr_pages computed
> >> from a previous call to folio_unmap_pte_batch gets reused without again
> >> going through folio_unmap_pte_batch, messing up things. Although, I don't
> >> think there is any bug right now; a bug would have been there, if in the
> >> same instance of a call to try_to_unmap_one, we end up in the
> >> pte_present(pteval) branch, then in the else branch doing pte_clear() for
> >> device-exclusive ptes. This means that a lazyfree folio has some present
> >> entries and some device entries mapping it. Since a pte being
> >> device-exclusive means that a GUP reference on the underlying folio is
> >> held, the lazyfree unmapping path upon witnessing this will abort
> >> try_to_unmap_one.
> >
> > Dude, paragraphs. PARAGRAPHS :) this is one dense set of words.
> >
> > It's also a very compressed 'stream of consciousness' hard-to-read block here.
>
> Sure :) I'll try to break this down.
>
> >
> > I'm not sure it's really worth having this as a separate commit either, it's
> > pretty trivial.
>
> Hmm...well, as I explain above, it's not trivial for me :) it is difficult
> for me to reason here whether nr_pages can be reused without a reset in
> a future iteration.

OK you can have it be separate, let's just really clean up the commit message
then please :)

Thanks, Lorenzo

On 10/03/26 2:09 pm, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 10, 2026 at 02:01:44PM +0530, Dev Jain wrote:
>>
>>
>> On 10/03/26 1:40 pm, Lorenzo Stoakes (Oracle) wrote:
>>> On Tue, Mar 10, 2026 at 01:00:06PM +0530, Dev Jain wrote:
>>>> Initialize nr_pages to 1 at the start of the loop, similar to what is being
>>>> done in folio_referenced_one(). It may happen that the nr_pages computed
>>>> from a previous call to folio_unmap_pte_batch gets reused without again
>>>> going through folio_unmap_pte_batch, messing up things. Although, I don't
>>>> think there is any bug right now; a bug would have been there, if in the
>>>> same instance of a call to try_to_unmap_one, we end up in the
>>>> pte_present(pteval) branch, then in the else branch doing pte_clear() for
>>>> device-exclusive ptes. This means that a lazyfree folio has some present
>>>> entries and some device entries mapping it. Since a pte being
>>>> device-exclusive means that a GUP reference on the underlying folio is
>>>> held, the lazyfree unmapping path upon witnessing this will abort
>>>> try_to_unmap_one.
>>>
>>> Dude, paragraphs. PARAGRAPHS :) this is one dense set of words.
>>>
>>> It's also a very compressed 'stream of consciousness' hard-to-read block here.
>>
>> Sure :) I'll try to break this down.
>>
>>>
>>> I'm not sure it's really worth having this as a separate commit either, it's
>>> pretty trivial.
>>
>> Hmm...well, as I explain above, it's not trivial for me :) it is difficult
>> for me to reason here whether nr_pages can be reused without a reset in
>> a future iteration.
> 
> OK you can have it be separate, let's just really clean up the commit message
> then please :)

Okay, thanks.

> 
> Thanks, Lorenzo