Re: [PATCH v1 1/3] mm: Allow deferred splitting of arbitrary large anon folios

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

 



On 18.07.23 10:58, Ryan Roberts wrote:
On 17/07/2023 17:48, David Hildenbrand wrote:
On 17.07.23 18:01, Ryan Roberts wrote:
On 17/07/2023 16:42, David Hildenbrand wrote:
On 17.07.23 16:31, Ryan Roberts wrote:
In preparation for the introduction of large folios for anonymous
memory, we would like to be able to split them when they have unmapped
subpages, in order to free those unused pages under memory pressure. So
remove the artificial requirement that the large folio needed to be at
least PMD-sized.

Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx>
Reviewed-by: Yu Zhao <yuzhao@xxxxxxxxxx>
Reviewed-by: Yin Fengwei <fengwei.yin@xxxxxxxxx>
---
    mm/rmap.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 0c0d8857dfce..2baf57d65c23 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1430,7 +1430,7 @@ void page_remove_rmap(struct page *page, struct
vm_area_struct *vma,
             * page of the folio is unmapped and at least one page
             * is still mapped.
             */
-        if (folio_test_pmd_mappable(folio) && folio_test_anon(folio))
+        if (folio_test_large(folio) && folio_test_anon(folio))
                if (!compound || nr < nr_pmdmapped)
                    deferred_split_folio(folio);

!compound will always be true I guess, so nr_pmdmapped == 0 (which will always
be the case) will be ignored.

I don't follow why !compound will always be true. This function is
page_remove_rmap() (not folio_remove_rmap_range() which I add in a later patch).
page_remove_rmap() can work on pmd-mapped pages where compound=true is passed in.

I was talking about the folio_test_pmd_mappable() -> folio_test_large() change.
For folio_test_large() && !folio_test_pmd_mappable() I expect that we'll never
pass in "compound=true".


Sorry David, I've been staring at the code and your comment, and I still don't
understand your point. I assumed you were trying to say that compound is always
false and therefore "if (!compound || nr < nr_pmdmapped)" can be removed? But
its not the case that compound is always false; it will be true when called to
remove a pmd-mapped compound page.

Let me try again:

Assume, as I wrote, that we are given a folio that is "folio_test_large() && !folio_test_pmd_mappable()". That is, a folio that is *not* pmd mappable.

If it's not pmd-mappable, certainly, nr_pmdmapped == 0, and therefore, "nr < nr_pmdmapped" will never ever trigger.

The only way to have it added to the deferred split queue is, therefore "if (!compound)".

So *for these folios*, we will always pass "compound == false" to make that "if (!compound)" succeed.


Does that make sense?

What change are you suggesting, exactly?

Oh, I never suggested a change (I even gave you my RB). I was just thinking out loud.

--
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