[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d7433859-6217-483c-9d48-bb6de97b09f7@gmail.com>
Date: Thu, 15 Aug 2024 18:10:12 +0100
From: Usama Arif <usamaarif642@...il.com>
To: David Hildenbrand <david@...hat.com>, akpm@...ux-foundation.org,
linux-mm@...ck.org
Cc: hannes@...xchg.org, riel@...riel.com, shakeel.butt@...ux.dev,
roman.gushchin@...ux.dev, yuzhao@...gle.com, baohua@...nel.org,
ryan.roberts@....com, rppt@...nel.org, willy@...radead.org,
cerasuolodomenico@...il.com, corbet@....net, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH v3 4/6] mm: Introduce a pageflag for partially mapped
folios
On 15/08/2024 17:33, David Hildenbrand wrote:
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 6df0e9f4f56c..c024ab0f745c 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3397,6 +3397,7 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
>> * page_deferred_list.
>> */
>> list_del_init(&folio->_deferred_list);
>> + folio_clear_partially_mapped(folio);
>> }
>> spin_unlock(&ds_queue->split_queue_lock);
>> if (mapping) {
>> @@ -3453,11 +3454,12 @@ void __folio_undo_large_rmappable(struct folio *folio)
>> if (!list_empty(&folio->_deferred_list)) {
>> ds_queue->split_queue_len--;
>> list_del_init(&folio->_deferred_list);
>> + folio_clear_partially_mapped(folio);
>> }
>> spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>> }
>> -void deferred_split_folio(struct folio *folio)
>> +void deferred_split_folio(struct folio *folio, bool partially_mapped)
>> {
> /* We lost race with folio_put() */> list_del_init(&folio->_deferred_list);
Was there some comment here? I just see ">" remove from the start of /* We lost race with folio_put() */
>> + folio_clear_partially_mapped(folio);
>> ds_queue->split_queue_len--;
>> }
>> if (!--sc->nr_to_scan)
>> @@ -3558,7 +3564,6 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>> next:
>> folio_put(folio);
>> }
>> -
>> spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>> list_splice_tail(&list, &ds_queue->split_queue);
>> spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 1fdd9eab240c..2ae2d9a18e40 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1758,6 +1758,7 @@ static void __update_and_free_hugetlb_folio(struct hstate *h,
>> free_gigantic_folio(folio, huge_page_order(h));
>> } else {
>> INIT_LIST_HEAD(&folio->_deferred_list);
>> + folio_clear_partially_mapped(folio);
>> folio_put(folio);
>> }
>> }
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 52f7fc4e8ac3..d64546b8d377 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -662,8 +662,10 @@ static inline void prep_compound_head(struct page *page, unsigned int order)
>> atomic_set(&folio->_entire_mapcount, -1);
>> atomic_set(&folio->_nr_pages_mapped, 0);
>> atomic_set(&folio->_pincount, 0);
>> - if (order > 1)
>> + if (order > 1) {
>> INIT_LIST_HEAD(&folio->_deferred_list);
>> + folio_clear_partially_mapped(folio);
>
> Can we use the non-atomic version here?
>
I believe we can use the non-atomic version in all places where set/clear is done as all set/clear are protected by ds_queue->split_queue_lock. So basically could replace all folio_set/clear_partially_mapped with __folio_set/clear_partially_mapped.
But I guess its likely not going to make much difference? I will do it anyways in the next revision, rather than sending a fix patch. There haven't been any reviews for patch 5 so will wait a few days for any comments on that.
Thanks
Powered by blists - more mailing lists