Re: [PATCH v3 2/9] mm/rmap: refactor hugetlb pte clearing in try_to_unmap_one

[Dev Jain]

On 12/05/26 3:50 am, Barry Song wrote:
> On Mon, May 11, 2026 at 3:10 PM David Hildenbrand (Arm)
> <david@xxxxxxxxxx> wrote:
> [...]
>>> +     *pteval = huge_ptep_clear_flush(vma, pvmw->address, pvmw->pte);
>>> +     if (pte_dirty(*pteval))
>>> +             folio_mark_dirty(folio);
>>> +
>>> +     *exit_walk = false;
>>> +     return true;
>>
>>
>> Can we instead introduce some enum that tells the caller how to proceed?
>>
>> I assume we have
>>
>> (a) Abort walk (ret = false + page_vma_mapped_walk_done())
>>
>> (b) Walk done (ret = true + page_vma_mapped_walk_done())
>>
>> (c) Continue walk (call page_vma_mapped_walk())
>>
>> enum ttu_walk_result {
>>         TTU_WALK_CONTINUE,
>>         TTU_WALK_ABORT,
>>         TTU_WALK_DONE
>> }
>>
> [...]
>>
>> In the old walk_abort case you wouldn't set ret = false?
>>
>> When returning the enum you could simply do something like
>>
>> switch (ret) {
>> case TTU_WALK_ABORT:
>>         goto walk_abort;
>> case TTU_WALK_DONE:
>>         goto walk_done;
>> default:
>>         break;
>> }
>>
> 
> I believe I suggested the exact same approach in v2[1], but Dev didn’t agree :-)
> 
> [1] https://lore.kernel.org/linux-mm/CAGsJ_4zK1W71iSP14gi=8yWSg80EjgYiBXqLChB2i+X+RSTWcw@xxxxxxxxxxxxxx/

I did mention your suggestion in my reply to David haha :)

I'll see if I can do even more refactoring with the tristate - it doesn't feel nice to
do the tristate at one place and then leave all others as it is.