On Tue, Dec 24, 2024 at 7:45 PM Chen Ridong <chenridong@xxxxxxxxxxxxxxx> wrote:
>
>
>
> On 2024/12/24 12:19, Yu Zhao wrote:
> > On Mon, Dec 23, 2024 at 1:30 AM Chen Ridong <chenridong@xxxxxxxxxxxxxxx> wrote:
> >>
> >> From: Chen Ridong <chenridong@xxxxxxxxxx>
> >>
> >> The page reclaim isolates a batch of folios from the tail of one of the
> >> LRU lists and works on those folios one by one. For a suitable
> >> swap-backed folio, if the swap device is async, it queues that folio for
> >> writeback. After the page reclaim finishes an entire batch, it puts back
> >> the folios it queued for writeback to the head of the original LRU list.
> >>
> >> In the meantime, the page writeback flushes the queued folios also by
> >> batches. Its batching logic is independent from that of the page reclaim.
> >> For each of the folios it writes back, the page writeback calls
> >> folio_rotate_reclaimable() which tries to rotate a folio to the tail.
> >>
> >> folio_rotate_reclaimable() only works for a folio after the page reclaim
> >> has put it back. If an async swap device is fast enough, the page
> >> writeback can finish with that folio while the page reclaim is still
> >> working on the rest of the batch containing it. In this case, that folio
> >> will remain at the head and the page reclaim will not retry it before
> >> reaching there.
> >
> > For starters, copying & pasting others' commit messages as your own is
> > plagiarism. You need to quote them.
>
> Hi, Yu, Thank you for reminding me, I did not mean any plagiarism. I am
> a beginner, and I do not know much about that.
I don't think Ridong's intention was plagiarism. It seems this was likely an
oversight in mentioning that these commit messages are quoted from Yu's
commit addressing the same issue in mglru. It was also my carelessness for
not reminding Ridong to include something like "quoted from Yu's commit..."
and adding quotation marks.
>
> I wrote the message in my v1 and v2 to describe what issue I was fixing,
> which is wordy. What you wrote is much clearer in the commit
> 359a5e1416ca, so I pasted it. I am sorry. Should resend a new patch to
> modify the message?
>
> I have sent all patch versions to you, and I don't know whether you have
> noticed. How I wish you could point it out at the first I pasted it, so
> I wouldn't have made this mistake again and again.
>
> >> The commit 359a5e1416ca ("mm: multi-gen LRU: retry folios written back
> >> while isolated") only fixed the issue for mglru. However, this issue
> >> also exists in the traditional active/inactive LRU.
> >
> > You need to prove it with some numbers.
>
> Do you mean I should prove it with some info in my message? Actually, I
> encountered this in the traditional active/inactive LRU, I did not know
> you had fixed for mglru until Barry told me. I offered how to reproduce
> this with Link.
>
> >> This issue will be
> >> worse if THP is split, which makes the list longer and needs longer time
> >> to finish a batch of folios reclaim.
> >>
> >> This issue should be fixed in the same way for the traditional LRU.
> >> Therefore, the common logic was extracted to the 'find_folios_written_back'
> >> function firstly, which is then reused in the 'shrink_inactive_list'
> >> function. Finally, retry reclaiming those folios that may have missed the
> >> rotation for traditional LRU.
> >>
> >> Link: https://lore.kernel.org/linux-kernel/20241010081802.290893-1-chenridong@xxxxxxxxxxxxxxx/
> >> Link: https://lore.kernel.org/linux-kernel/CAGsJ_4zqL8ZHNRZ44o_CC69kE7DBVXvbZfvmQxMGiFqRxqHQdA@xxxxxxxxxxxxxx/
> >> Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
> >> Reviewed-by: Barry Song <baohua@xxxxxxxxxx>
> >> ---
> >>
> >> v5->v6:
> >> - fix compile error(implicit declaration of function 'lru_gen_distance')
> >> when CONFIG_LRU_GEN is disable.
> >
> > Did you build-test it this time? I don't think LRU_REFS_FLAGS is
> > defined when CONFIG_LRU_GEN=y.
> >
>
> Yes, I tested. I didn't test when CONFIG_LRU_GEN=n in patch v5.
> I tested with CONFIG_LRU_GEN=n and CONFIG_LRU_GEN=y in patch v6. I am
> using the next.
>
> Best regards,
> Ridong
>
> >
> >
> >> - rename 'is_retried' to is_retrying suggested by Barry Song.
> >>
> >> v5: https://lore.kernel.org/linux-kernel/CAGsJ_4x3Aj7wieK1FQKQC4Vbz5N+1dExs=Q70KQt-whS1dMxpw@xxxxxxxxxxxxxx/
> >>
> >> include/linux/mm_inline.h | 5 ++
> >> mm/vmscan.c | 108 ++++++++++++++++++++++++--------------
> >> 2 files changed, 75 insertions(+), 38 deletions(-)
>
Thanks
Barry
