Re: [PATCH V2] mm: swap: check for stable address space before operating on the VMA

[Charan Teja Kalla <charan.kalla@oss.qualcomm.com>]

Thanks Barry/Chris for checking it.

On 9/26/2025 2:00 PM, Barry Song wrote:
>> 2)  __oom_reap_task_mm()
>> It holds the mmap read lock when setting the MMF_UNSTABLE as far as I can tell.
>> So checking the MMF_UNSTABLE with another __oom_reap_task_mm() does
>> not exclude each other.
>> This is more of a question for oom reaping.
>> Does MMF_UNSTABLE have the test vs set racing here? It seems this
>> check does not protect against  __oom_reap_task_mm(). I have no idea
>> if this race is triggerable. Just want someone else to double check if
>> my understanding is correct or not.
> I haven’t actually run the code.
> My guess is there’s a race when checking MMF_UNSTABLE against the
> OOM reaper. I think it’s fine either way—whether we skip an OOM-reaped
> mm upfront or take a middle path—since the OOM reaper will handle those
> PTEs with the PTL just like unuse_pte() does and eventually free the mm
> of the reaped process. It’s probably better to skip it early and avoid
> unnecessary unuse_pte() calls.
> 

I am sorry that I can't really see any stability issue b/n oom reaper 
and unuse_mm(), but yes that unnecessary unuse_pte() calls, as Barry 
mentioned, after reaping.

>> I can see this patch does protect the intended race in dup_mm() vs
>> unuse_mm(), it adds value.
> This also seems to add values for OOM-reaped processes to avoid a
> useless unuse(), in case we aren’t skipping this mm right now. I’m
> not sure if we’ve been skipping OOM-reaped processes elsewhere.
> 
I don't see any explicit flags that tells the process is already 
oom-reaped/under it. There is MMF_OOM_REAP_QUEUED, but this doesn't tell 
if it is already reaped.

If the unnecessary calls to unuse_vma() is really of a concern, then 
check the MMF_UNSTABLE while traversing VMA may be a solution but this 
looks ugly.

> Hi Charan, do you have any observations on this? If an additional value is
> added, could we record it in the changelog? Otherwise, can we add some
> description in the changelog to address Chris’ concern?
I do see that Chris ask can go as completely different change as the 
mentioned problem exist even before this change, please CMIW.

Thanks,
Charan