Re: [PATCH v9 11/17] mm: replace vm_lock and detached flag with a reference count

Wei Yang
2025-01-16T09:10:25Z

On Wed, Jan 15, 2025 at 05:41:27PM -0800, Suren Baghdasaryan wrote:
[...]
>> >> >the case of EAGAIN.
>> >> >
>> >>
>> >> Looks good to me.
>> >>
>> >> >>
>> >> >> Maybe we can compare the event VMA_LOCK_MISS and VMA_LOCK_ABORT
>> >> >> to see the percentage of this case. If it shows this is a too rare
>> >> >> case to impact performance, we can ignore it.
>> >> >>
>> >> >> Also the event VMA_LOCK_MISS recording is removed, but the definition is
>> >> >> there. We may record it in the vma_start_read() when oldcnt is 0.
>> >> >>
>> >> >> BTW, the name of VMA_LOCK_SUCCESS confuse me a little. I thought it indicates
>> >> >> lock_vma_under_rcu() successfully get a valid vma. But seems not. Sounds we
>> >> >> don't have an overall success/failure statistic in vmstat.
>> >> >
>> >> >Are you referring to the fact that we do not increment
>> >> >VMA_LOCK_SUCCESS if we successfully locked a vma but have to retry the
>> >>
>> >> Something like this. I thought we would increase VMA_LOCK_SUCCESS on success.
>> >>
>> >> >page fault (in which we increment VMA_LOCK_RETRY instead)?
>> >> >
>> >>
>> >> I don't follow this.
>> >
>> >Sorry, I meant to say "in which case we increment VMA_LOCK_RETRY
>> >instead". IOW, when we successfully lock the vma but have to retry the
>> >pagefault, we increment VMA_LOCK_RETRY without incrementing
>> >VMA_LOCK_SUCCESS.
>> >
>>
>> Yes, this makes me confused about what VMA_LOCK_SUCCESS represents.
>
>I'll need to look into the history of why we account it this way but
>this is out of scope for this patchset.
>

Agree.

--
Wei Yang
Help you, Help me