mm/page_alloc: cleanup __alloc_pages_direct_compact()
Subsystems (2)
PATCH 1/1 mm/page_alloc: cleanup __alloc_pages_direct_compact()
This patch cleans up the if(page).
No functional change.
Signed-off-by Pengfei Li <[email protected]>
--- mm/page_alloc.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)
mm/page_alloc.c +16 -12
@@ -3890,6 +3890,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, enum compact_priority prio, enum compact_result *compact_result) { struct page *page = NULL; +struct zone *zone;unsigned long pflags;unsigned int noreclaim_flag;
@@ -3911,23 +3912,26 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,*/ count_vm_event (COMPACTSTALL ); -/* Prep a captured page if available */ -if (page) +if (page) { +/* Prep a captured page if available */ prep_new_page (page, order, gfp_mask, alloc_flags); - -/* Try get a page from the freelist if available */ -if (!page) + }else { +/* Try get a page from the freelist if available */ page =get_page_from_freelist (gfp_mask, order, alloc_flags, ac); -if (page) { -struct zone *zone =page_zone (page); - - zone->compact_blockskip_flush = false; -compaction_defer_reset (zone, order, true); -count_vm_event (COMPACTSUCCESS ); -return page; +if (!page) + goto failed; } + zone =page_zone (page); + zone->compact_blockskip_flush = false; +compaction_defer_reset (zone, order, true); + +count_vm_event (COMPACTSUCCESS ); + +return page; + +failed:/* * It's bad if compaction run occurs and fails. The most likely reason * is that pages exist, but not enough to satisfy watermarks.
Signature · 1 line
2.21.0
Quoted 5 lines · Pengfei Li
This patch cleans up the if(page).
No functional change.
Signed-off-by Pengfei Li <[email protected]>
I don't see much benefit here. The indentation wasn't that bad that it had to be reduced using goto. But the patch is not incorrect so I'm not NACKing.
Quoted 56 lines
--- mm/page_alloc.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)mm/page_alloc.c +16
-12@@ -3890,6 +3890,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, enum compact_priority prio, enum compact_result *compact_result) { struct page *page = NULL; +struct zone *zone;unsigned long pflags;unsigned int noreclaim_flag;@@ -3911,23 +3912,26 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,*/ count_vm_event (COMPACTSTALL ); -/* Prep a captured page if available */ -if (page) +if (page) { +/* Prep a captured page if available */ prep_new_page (page, order, gfp_mask, alloc_flags); - -/* Try get a page from the freelist if available */ -if (!page) + }else { +/* Try get a page from the freelist if available */ page =get_page_from_freelist (gfp_mask, order, alloc_flags, ac); -if (page) { -struct zone *zone =page_zone (page); - - zone->compact_blockskip_flush = false; -compaction_defer_reset (zone, order, true); -count_vm_event (COMPACTSUCCESS ); -return page; +if (!page) + goto failed; } + zone =page_zone (page); + zone->compact_blockskip_flush = false; +compaction_defer_reset (zone, order, true); + +count_vm_event (COMPACTSUCCESS ); + +return page; + +failed:/* * It's bad if compaction run occurs and fails. The most likely reason * is that pages exist, but not enough to satisfy watermarks.
Quoted 4 lines · Vlastimil Babka
Quoted 2 lines · Pengfei Li
This patch cleans up the if(page).
Quoted 8 lines
Quoted 3 lines
No functional change.
Signed-off-by Pengfei Li <[email protected]>I don't see much benefit here. The indentation wasn't that bad that it had to be reduced using goto. But the patch is not incorrect so I'm not NACKing.
Thanks for your review and comments.
This patch reduces the number of times the if(page)
(as the compiler does), and the downside is that there is a goto.
If this improves readability, accept it. Otherwise, leave it as it is.
Thanks again.
---
Pengfei
Quoted 57 lines
Quoted 56 lines
--- mm/page_alloc.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)mm/page_alloc.c +16
-12@@ -3890,6 +3890,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, enum compact_priority prio, enum compact_result *compact_result) { struct page *page = NULL; +struct zone *zone;unsigned long pflags;unsigned int noreclaim_flag;@@ -3911,23 +3912,26 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,*/ count_vm_event (COMPACTSTALL ); -/* Prep a captured page if available */ -if (page) +if (page) { +/* Prep a captured page if available */ prep_new_page (page, order, gfp_mask, alloc_flags); - -/* Try get a page from the freelist if available */ -if (!page) + }else { +/* Try get a page from the freelist if available */ page =get_page_from_freelist (gfp_mask, order, alloc_flags, ac); -if (page) { -struct zone *zone =page_zone (page); - - zone->compact_blockskip_flush = false; -compaction_defer_reset (zone, order, true); -count_vm_event (COMPACTSUCCESS ); -return page; +if (!page) + goto failed; } + zone =page_zone (page); + zone->compact_blockskip_flush = false; +compaction_defer_reset (zone, order, true); + +count_vm_event (COMPACTSUCCESS ); + +return page; + +failed:/* * It's bad if compaction run occurs and fails. The most likely reason * is that pages exist, but not enough to satisfy watermarks.