lkml.org 
[lkml]   [2024]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH] mm:vmscan: shrink skip folios in the exiting task
From


在 2024/1/28 14:35, Matthew Wilcox 写道:
> On Thu, Jan 25, 2024 at 09:34:53AM +0800, zhiguojiang wrote:
>>>> In the scenarios of the low memory system and mutil backed-applications,
>>>> the time-consuming problem caused by shrinking the exiting task's folios
>>>> will be more severe.
>>> What testing have you done of this patch? How often does it happen?
>>> Are there particular workloads that benefit from this? (I'm not sure
>>> what "mutil backed-applications" are?)
>> 1 Yes, this patch has been tested.
>>
>> 2 When the exiting tasks and shrink_inactive_list occur at the same time,
>>    the folios which shrink_inactive_list reclaims may be the exiting tasks's
>> folios
>>    in lruvecs. And when system is low memory, it more likely to occur,
>> because
>>    more backend applidatuions will be killed.
>>    The shrink_inactive_list reclaims the exiting tasks's folios in lruvecs
>> and
>>    transforms the exiting tasks's anon folios into swap memory, which leads
>>    to the increasing load of the current exiting tasks.
> Ah, we're talking about an OOM scenario. OK, that makes some sense.
> You should have mentioned that in the patch description.
Hi,

1 Ok, I will update a more comprehensive description in next version.

2 I think this issue can occur not only in OOM scenario, but also in
  normal task exit scenario. So:

1) if (unlikely(!atomic_read(&vma->vm_mm->mm_users))) represents
  the scenario where the task exits normally.

2) if(test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) represents the
   OOM Reaper scenario.

3 MMF_OOM_SKIP can only represent OOM scenario and cannot represent
  normal task exit scenario, as when MMF_OOM_SKIP is set in normal
  task exit scenario, the memory folios of the task have already been
  released. And the shrink_inactive_list should recognize these lru folios
  in exiting task before this exiting task releases its memory folios.

    tlb_gather_mmu_fullmm(&tlb, mm);
    /* update_hiwater_rss(mm) here? but nobody should be looking */
    /* Use ULONG_MAX here to ensure all VMAs in the mm are unmapped */
    unmap_vmas(&tlb, &mas, vma, 0, ULONG_MAX, ULONG_MAX, false);
    mmap_read_unlock(mm);

    /*
     * Set MMF_OOM_SKIP to hide this task from the oom killer/reaper
     * because the memory has been already freed.
     */
    set_bit(MMF_OOM_SKIP, &mm->flags);
>> 3 This patch can alleviate the load of the tasks exiting process. This patch
>>    can make that the exiting tasks release its anon folios faster instead of
>>    releasing its swap memory from its anon folios swap-in in
>> shrink_inactive_list.
>>
>> 4 "mutil backed-applications" means that there are a large number of
>>     the backend applications in the system.
>>
>> Thanks
>>> And I do mean specifically of this patch, because to my eyes it
>>> shouldn't even compile.
>> Has been tested.
> That's odd. I thought GCC used to complain about
>
> long x = 0x100000000;
>
> but it does the right thing. Except on 32-bit where it'll say
> "warning: integer constant out of range".
>
> In any case, the number you chose is not going to work on 32-bit systems
> or on ARM or x86. It conflicts with protection keys on x86 and MTE on
> ARM.
You're right, I overlooked the situation with the 32-bit system.
> We can do it without any new magic numbers. I'm not sure this is a
> great approach, but this should work:
>
> +++ b/mm/rmap.c
> @@ -840,6 +840,12 @@ static bool folio_referenced_one(struct folio *folio,
> int referenced = 0;
> unsigned long start = address, ptes = 0;
>
> + /* Skip this folio if it's mapped by an exiting task */
> + if (test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) {
> + pra->referenced = -1;
> + return false;
> + }
> +
> while (page_vma_mapped_walk(&pvmw)) {
> address = pvmw.address;
>
I agree with your point of view. I think this is currently the best
solution,
  but I think it also needs to be added with:
if (unlikely(!atomic_read(&vma->vm_mm->mm_users)))

Please help review it again.

Best Regards.


\
 
 \ /
  Last update: 2024-05-27 14:37    [W:0.035 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site