Re: [RFC PATCH 1/1] mm: Skip folio with private data during isolation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26.08.24 13:10, Zhaoyang Huang wrote:
On Mon, Aug 26, 2024 at 6:36 PM David Hildenbrand <david@xxxxxxxxxx> wrote:

On 26.08.24 10:50, zhaoyang.huang wrote:
From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>

Since clean target folio with private data will be given up finally in
__remove_mapping as it has extra refcnt, it is better to skip it during
isolation to save the slot for more qualified folio. Current one could
be the candidate for next round of scanning after the private data gone.

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
---
   mm/vmscan.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index cfa839284b92..755bf3a387f3 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1685,6 +1685,8 @@ static unsigned long isolate_lru_folios(unsigned long nr_to_scan,
                */
               scan += nr_pages;

+             if (folio_test_private(folio) && !folio_test_dirty(folio))
+                     goto move;
               if (!folio_test_lru(folio))
                       goto move;
               if (!sc->may_unmap && folio_mapped(folio))

An earlier filemap_release_folio() would have failed if the private data
(buffers) cannot get freed, and we went into the activate_locked path.


if (folio_needs_release(folio)) {
         if (!filemap_release_folio(folio, sc->gfp_mask)
                 goto activate_locked;
...

if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) {
         ...
} else if (!mapping || !__remove_mapping(mapping, folio, true,
}

At least on the shrink_folio_list() path, I'm not sure the code you are
adding could even trigger. We should not reach __remove_mapping() with
folio_test_private().
Thanks for heads up. You are right, the bh is judged if existing
before __remove_mapping. ASAIU, the metadata associated with the bh
has risk to be freed such as journal data etc or it introduces extra
IO. Actually, this patch is inspired by a practical problem we just
run across which the bh remains on LRU for a long time since it is
attached to a journal_head that can not be freed by jbd2.

Okay, but I assume (as stated) this patch does not have an affect on that, or am I missing something?

I assume you have some way to test before/after, if you run into similar problems in practice.

--
Cheers,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux