On Mon, Jul 8, 2024 at 3:40 PM zhiguojiang <justinjiang@xxxxxxxx> wrote: > > > > 在 2024/6/30 17:35, Barry Song 写道: > > [Some people who received this message don't often get email from baohua@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > > > On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@xxxxxxxx> wrote: > > This is clearly version 3, as you previously sent version 2, correct? > Hi Barry, > > Yes, patchs are as follows: > v3: > https://lore.kernel.org/linux-mm/20240221114904.1849-1-justinjiang@xxxxxxxx/ > v2: > https://lore.kernel.org/linux-mm/20240202012251.1aa5afbfdf2f8b3a862bced3@xxxxxxxxxxxxxxxxxxxx/ > v1: > https://lore.kernel.org/linux-mm/20240124124308.461-1-justinjiang@xxxxxxxx/ > > > >> If an anon folio reclaimed by shrink_inactive_list is mapped by an > >> exiting task, this anon folio will be firstly swaped-out into > >> swapspace in shrink flow and then this swap folio is freed in task > >> exit flow. But if this folio mapped by an exiting task can skip > >> shrink and be freed directly in task exiting flow, which will save > >> swap-out time and alleviate the load of the tasks exiting process. > >> The file folio is also similar. > >> > > Could you please describe the specific impact on users, including user > > experience and power consumption? How serious is this problem? > 1.At present, I do not have a suitable method to accurately measure the > optimization benefit datas of this modifications, but I believe it > theoretically > has some benefits. > 2.Launching large memory app (for example, starting the camera) in multiple > backend scenes may result in the high cpu load of exiting processes. > > > >> And when system is low memory, it more likely to occur, because more > >> backend applidatuions will be killed. > > Applications? > Yes. > > > >> This patch can alleviate the load of the tasks exiting process. > > I'm not completely convinced this patch is correct, but it appears to be > > heading in the right direction. Therefore, I expect to see new versions > > rather than it being dead. > > > >> Signed-off-by: Zhiguo Jiang <justinjiang@xxxxxxxx> > >> --- > >> mm/rmap.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> mode change 100644 => 100755 mm/rmap.c > > You changed the file mode to 755, which is incorrect. > > > >> diff --git a/mm/rmap.c b/mm/rmap.c > >> index 3746a5531018..146e5f4ec069 > >> --- a/mm/rmap.c > >> +++ b/mm/rmap.c > >> @@ -840,6 +840,13 @@ 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 (unlikely(!atomic_read(&vma->vm_mm->mm_users)) || > >> + unlikely(test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags))) { > >> + pra->referenced = -1; > > Why use -1? Is this meant to simulate lock contention to keep the folio without > > activating it? > > > > /* rmap lock contention: rotate */ > > if (referenced_ptes == -1) > > return FOLIOREF_KEEP; > > > > Please do have some comments to explain why. > > > > I'm not convinced this change is appropriate for shared folios. It seems > > more suitable for exclusive folios used solely by the exiting process. > 1.The skiped folios are FOLIOREF_KEEP and added into inactive lru, beacase > the folios will be freed soon in the exiting process flow. This requires an explicit comment in the code. The expression referenced_ptes == -1 implies "rmap lock contention: rotate", but it appears that this is not the case here. You should either update the original comment to reflect the current logic or use a different value. > 2.Yes, the shared folios can not be simply skipped. I have made relevant > modifications in patch v4 and please help to further review. > https://lore.kernel.org/linux-mm/20240708031517.856-1-justinjiang@xxxxxxxx/ > > Thanks > Zhiguo > > > > > >> + return false; > >> + } > >> + > >> while (page_vma_mapped_walk(&pvmw)) { > >> address = pvmw.address; > >> > >> -- > >> 2.39.0 > >> > > Thanks > > Barry >