On 7/9/26 23:00, Lorenzo Stoakes wrote:
> On Thu, Jul 09, 2026 at 04:25:27PM +0800, Wandun wrote:
>>
>>
>> On 7/7/26 21:55, Lorenzo Stoakes wrote:
>>> On Tue, Jul 07, 2026 at 02:44:50PM +0100, Lorenzo Stoakes wrote:
>>>> See above about deduplicating.
>>>>
>>>>> +                 ttu |= TTU_RESPECT_MLOCK;
>>>>
>>>> Hmm. I don't love 'respect mlock'. I guess we only know about the reason
>>>> being compaction here.
>>>>
>>>> But I'm confused anyway. We have the folio, why aren't we just checking for
>>>> PG_mlocked() here instead of getting the rmap to see if it's mapped
>>>> anywhere with VMA_LOCKED_BIT?
>>>
>>> Also, since compaction_allow_unevictable() is a function that is accessible
>>> elsewhere, you could literally just have a TTU_MIGRATION here instead and 
>>> have
>>> the rmap logic call compaction_allow_unevictable() instead rather than this.
>> Do you mean:
>> 1. change TTU_RESPECT_MLOCK to TTU_MIGRATION, that'is OK.
>> 2. move call compaction_allow_unevictable() to try_to_migrate_one? but as 
>> you suggested
>>    migrate_mlock_allowed function, it already called 
>> compaction_allow_unevictable()
>>    in order to determine whether TTU_MIGRATION needs to be added. Did I 
>> misunderstand
>>    something somewhere?
> 
> The next paragraph addresses this.
> 
>>
>>>
>>> And then you could adapt the function I suggested before not to take a 
>>> reason
>>> parameter but rather a 'is_migration' one instead possibly and then pass 
>>> (ttu &
>>> TTU_MIGRATION) in.
> 
> 
>>>
>>> BUT. I still question whether this is at all needed since you have the 
>>> folio you
>>> can check for PG_mlocked...
>>
>> I described a race scenario at this link:
>> https://lore.kernel.org/lkml/[email protected]/
> 
> OK thanks. You really have to spell this out in the code in a comment, this is
> really non-obvious.

I will address these in the next version.

Thanks,
Wandun

> 
> I will try to look at your reply in due course (am between jobs atm).
> 
>>
>>
>>>
>>> Cheers, Lorenzo
>>
> 
> Thanks, Lorenzo


Reply via email to