Skip to contentSkip to conversation

mm/swap_state.c: use the same way to count page in [add_to|delete_from]_swap_cache

PATCH Superseded Wei Yang 2 participants 3 messages Mar 14, 2020 – Mar 15, 2020

Subsystems (2)
Memory management MM swap

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Wei Yang

author
#0 lore raw

PATCH 1/1 mm/swap_state.c: use the same way to count page in [add_to|delete_from]_swap_cache

Function add_to_swap_cache() and delete_from_swap_cache() are counter parts, while currently they use different way to count page.

It doesn't break any thing because we only have two size for PageAnon, but this is confusing and not a good practice.

This patch corrects it by both using compound_nr().

Signed-off-by Wei Yang <[email protected]>
---
 mm/swap_state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
mm/swap_state.c +2 -2
index 8e7ce9a9bc5e..51d8884a693a 100644
@@ -158,7 +158,7 @@ int add_to_swap_cache(struct page *page, swp_entry_t entry, gfp_t gfp)
 void __delete_from_swap_cache(struct page *page, swp_entry_t entry)
 {
 	struct address_space *address_space = swap_address_space(entry);
-	int i, nr = hpage_nr_pages(page);
+	int i, nr = compound_nr(page);
 	pgoff_t idx = swp_offset(entry);
 	XA_STATE(xas, &address_space->i_pages, idx);
 

@@ -251,7 +251,7 @@ void delete_from_swap_cache(struct page *page)
 	xa_unlock_irq(&address_space->i_pages);
 
 	put_swap_page(page, entry);
-	page_ref_sub(page, hpage_nr_pages(page));
+	page_ref_sub(page, compound_nr(page));
 }
 
 /*
Signature · 1 line
2.23.0
#1 lore raw
Quoted 7 lines · Wei Yang

Function add_to_swap_cache() and delete_from_swap_cache() are counter parts, while currently they use different way to count page.

It doesn't break any thing because we only have two size for PageAnon, but this is confusing and not a good practice.

This patch corrects it by both using compound_nr().

You're converting in the wrong direction. hpage_nr_pages() is optimised away when CONFIG_TRANSPARENT_HUGEPAGE is undefined, whereas compound_nr() is not.

I also have this patch pending:
http://git.infradead.org/users/willy/linux-dax.git/commitdiff/192b635b428ae74f680574cdcc3d5e9d213fcb64

author
#2 lore raw
Quoted 4 lines · Matthew Wilcox
Quoted 3 lines · Wei Yang

Function add_to_swap_cache() and delete_from_swap_cache() are counter parts, while currently they use different way to count page.

Quoted 8 lines
Quoted 4 lines

It doesn't break any thing because we only have two size for PageAnon, but this is confusing and not a good practice.

This patch corrects it by both using compound_nr().

You're converting in the wrong direction. hpage_nr_pages() is optimised away when CONFIG_TRANSPARENT_HUGEPAGE is undefined, whereas compound_nr() is not.

Ok, I didn't think about the optimisation.

Will use hpage_nr_pages() in v2.

Quoted 3 lines

I also have this patch pending:
http://git.infradead.org/users/willy/linux-dax.git/commitdiff/192b635b428ae74f680574cdcc3d5e9d213fcb64

Signature · 2 lines
Wei Yang
Help you, Help me