| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <99ba4e0d-ef36-4516-a275-014cf5eb22fd@linux.alibaba.com>
Date: Tue, 11 Jun 2024 11:31:39 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: Daniel Gomez <da.gomez@...sung.com>
Cc: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"hughd@...gle.com" <hughd@...gle.com>,
"willy@...radead.org" <willy@...radead.org>,
"david@...hat.com" <david@...hat.com>,
"wangkefeng.wang@...wei.com" <wangkefeng.wang@...wei.com>,
"chrisl@...nel.org" <chrisl@...nel.org>,
"ying.huang@...el.com" <ying.huang@...el.com>,
"21cnbao@...il.com" <21cnbao@...il.com>,
"ryan.roberts@....com" <ryan.roberts@....com>,
"shy828301@...il.com" <shy828301@...il.com>, "ziy@...dia.com"
<ziy@...dia.com>, "ioworker0@...il.com" <ioworker0@...il.com>,
Pankaj Raghav <p.raghav@...sung.com>, "linux-mm@...ck.org"
<linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/7] mm: add new 'orders' parameter for find_get_entries()
and find_lock_entries()
On 2024/6/10 23:23, Daniel Gomez wrote:
> Hi Baolin,
>
> On Thu, Jun 06, 2024 at 07:58:55PM +0800, Baolin Wang wrote:
>> In the following patches, shmem will support the swap out of large folios,
>> which means the shmem mappings may contain large order swap entries, so an
>> 'orders' array is added for find_get_entries() and find_lock_entries() to
>> obtain the order size of shmem swap entries, which will help in the release
>> of shmem large folio swap entries.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>> ---
>> mm/filemap.c | 27 +++++++++++++++++++++++++--
>> mm/internal.h | 4 ++--
>> mm/shmem.c | 17 +++++++++--------
>> mm/truncate.c | 8 ++++----
>> 4 files changed, 40 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index 37061aafd191..47fcd9ee6012 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -2036,14 +2036,24 @@ static inline struct folio *find_get_entry(struct xa_state *xas, pgoff_t max,
>> * Return: The number of entries which were found.
>> */
>> unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>> - pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices)
>> + pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices,
>> + int *orders)
>> {
>> XA_STATE(xas, &mapping->i_pages, *start);
>> struct folio *folio;
>> + int order;
>>
>> rcu_read_lock();
>> while ((folio = find_get_entry(&xas, end, XA_PRESENT)) != NULL) {
>> indices[fbatch->nr] = xas.xa_index;
>> + if (orders) {
>> + if (!xa_is_value(folio))
>> + order = folio_order(folio);
>> + else
>> + order = xa_get_order(xas.xa, xas.xa_index);
>> +
>> + orders[fbatch->nr] = order;
>> + }
>> if (!folio_batch_add(fbatch, folio))
>> break;
>> }
>> @@ -2056,6 +2066,8 @@ unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>> folio = fbatch->folios[idx];
>> if (!xa_is_value(folio))
>> nr = folio_nr_pages(folio);
>> + else if (orders)
>> + nr = 1 << orders[idx];
>> *start = indices[idx] + nr;
>> }
>> return folio_batch_count(fbatch);
>> @@ -2082,10 +2094,12 @@ unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>> * Return: The number of entries which were found.
>> */
>> unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>> - pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices)
>> + pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices,
>> + int *orders)
>> {
>> XA_STATE(xas, &mapping->i_pages, *start);
>> struct folio *folio;
>> + int order;
>>
>> rcu_read_lock();
>> while ((folio = find_get_entry(&xas, end, XA_PRESENT))) {
>> @@ -2099,9 +2113,16 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>> if (folio->mapping != mapping ||
>> folio_test_writeback(folio))
>> goto unlock;
>> + if (orders)
>> + order = folio_order(folio);
>> VM_BUG_ON_FOLIO(!folio_contains(folio, xas.xa_index),
>> folio);
>> + } else if (orders) {
>> + order = xa_get_order(xas.xa, xas.xa_index);
>> }
>> +
>> + if (orders)
>> + orders[fbatch->nr] = order;
>> indices[fbatch->nr] = xas.xa_index;
>> if (!folio_batch_add(fbatch, folio))
>> break;
>> @@ -2120,6 +2141,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>> folio = fbatch->folios[idx];
>> if (!xa_is_value(folio))
>> nr = folio_nr_pages(folio);
>> + else if (orders)
>> + nr = 1 << orders[idx];
>> *start = indices[idx] + nr;
>> }
>> return folio_batch_count(fbatch);
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 3419c329b3bc..0b5adb6c33cc 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -339,9 +339,9 @@ static inline void force_page_cache_readahead(struct address_space *mapping,
>> }
>>
>> unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>> - pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
>> + pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices, int *orders);
>> unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>> - pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
>> + pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices, int *orders);
>> void filemap_free_folio(struct address_space *mapping, struct folio *folio);
>> int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
>> bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 0ac71580decb..28ba603d87b8 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -840,14 +840,14 @@ static void shmem_delete_from_page_cache(struct folio *folio, void *radswap)
>> * Remove swap entry from page cache, free the swap and its page cache.
>> */
>> static int shmem_free_swap(struct address_space *mapping,
>> - pgoff_t index, void *radswap)
>> + pgoff_t index, void *radswap, int order)
>> {
>> void *old;
>
> Matthew Wilcox suggested [1] returning the number of pages freed in shmem_free_swap().
>
> [1] https://lore.kernel.org/all/ZQRf2pGWurrE0uO+@casper.infradead.org/
>
> Which I submitted here:
> https://lore.kernel.org/all/20231028211518.3424020-5-da.gomez@samsung.com/
>
> Do you agree with the suggestion? If so, could we update my patch to use
> free_swap_and_cache_nr() or include that here?
Yes, this looks good to me. But we still need some modification for
find_lock_entries() and find_get_entries() to update the '*start'
correctly. I will include your changes into this patch in next version.
Thanks.
Powered by blists - more mailing lists