* [PATCH] mm/compaction: Remove some useless code in compact_zone() @ 2020-10-14 7:23 yanfei.xu 2020-10-14 12:28 ` David Hildenbrand 0 siblings, 1 reply; 4+ messages in thread From: yanfei.xu @ 2020-10-14 7:23 UTC (permalink / raw) To: akpm; +Cc: linux-mm, linux-kernel From: Yanfei Xu <yanfei.xu@windriver.com> start_pfn has been declared at the begin of compact_zone(), it's no need to declare it again. And remove an useless semicolon. Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> --- mm/compaction.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mm/compaction.c b/mm/compaction.c index 176dcded298e..5e69c1f94d96 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -2272,7 +2272,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) while ((ret = compact_finished(cc)) == COMPACT_CONTINUE) { int err; - unsigned long start_pfn = cc->migrate_pfn; + start_pfn = cc->migrate_pfn; /* * Avoid multiple rescans which can happen if a page cannot be @@ -2309,7 +2309,6 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) case ISOLATE_SUCCESS: update_cached = false; last_migrated_pfn = start_pfn; - ; } err = migrate_pages(&cc->migratepages, compaction_alloc, -- 2.18.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mm/compaction: Remove some useless code in compact_zone() 2020-10-14 7:23 [PATCH] mm/compaction: Remove some useless code in compact_zone() yanfei.xu @ 2020-10-14 12:28 ` David Hildenbrand 2020-10-16 15:05 ` Vlastimil Babka 0 siblings, 1 reply; 4+ messages in thread From: David Hildenbrand @ 2020-10-14 12:28 UTC (permalink / raw) To: yanfei.xu, akpm; +Cc: linux-mm, linux-kernel On 14.10.20 09:23, yanfei.xu@windriver.com wrote: > From: Yanfei Xu <yanfei.xu@windriver.com> > > start_pfn has been declared at the begin of compact_zone(), it's > no need to declare it again. And remove an useless semicolon. > > Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> > --- > mm/compaction.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 176dcded298e..5e69c1f94d96 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -2272,7 +2272,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) > > while ((ret = compact_finished(cc)) == COMPACT_CONTINUE) { > int err; > - unsigned long start_pfn = cc->migrate_pfn; > + start_pfn = cc->migrate_pfn; There is a user in trace_mm_compaction_end(start_pfn, cc->migrate_pfn, cc->free_pfn, end_pfn, sync, ret); we would now trace a different value, no? > > /* > * Avoid multiple rescans which can happen if a page cannot be > @@ -2309,7 +2309,6 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) > case ISOLATE_SUCCESS: > update_cached = false; > last_migrated_pfn = start_pfn; > - ; Huh, how does something like that happen :) -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm/compaction: Remove some useless code in compact_zone() 2020-10-14 12:28 ` David Hildenbrand @ 2020-10-16 15:05 ` Vlastimil Babka 2020-10-19 7:32 ` Xu, Yanfei 0 siblings, 1 reply; 4+ messages in thread From: Vlastimil Babka @ 2020-10-16 15:05 UTC (permalink / raw) To: David Hildenbrand, yanfei.xu, akpm; +Cc: linux-mm, linux-kernel On 10/14/20 2:28 PM, David Hildenbrand wrote: > On 14.10.20 09:23, yanfei.xu@windriver.com wrote: >> From: Yanfei Xu <yanfei.xu@windriver.com> >> >> start_pfn has been declared at the begin of compact_zone(), it's >> no need to declare it again. And remove an useless semicolon. >> >> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> >> --- >> mm/compaction.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/mm/compaction.c b/mm/compaction.c >> index 176dcded298e..5e69c1f94d96 100644 >> --- a/mm/compaction.c >> +++ b/mm/compaction.c >> @@ -2272,7 +2272,7 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) >> >> while ((ret = compact_finished(cc)) == COMPACT_CONTINUE) { >> int err; >> - unsigned long start_pfn = cc->migrate_pfn; >> + start_pfn = cc->migrate_pfn; > > There is a user in > > trace_mm_compaction_end(start_pfn, cc->migrate_pfn, cc->free_pfn, > end_pfn, sync, ret); > > we would now trace a different value, no? Agreed. We should rather rename the while-local one to avoid confusion. Something like "iteration_start_pfn" ? >> >> /* >> * Avoid multiple rescans which can happen if a page cannot be >> @@ -2309,7 +2309,6 @@ compact_zone(struct compact_control *cc, struct capture_control *capc) >> case ISOLATE_SUCCESS: >> update_cached = false; >> last_migrated_pfn = start_pfn; >> - ; > > Huh, how does something like that happen :) Looks like "case ISOLATE_SUCCESS:" used to be an empty implementation, then statements got added, but semicolon not removed. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm/compaction: Remove some useless code in compact_zone() 2020-10-16 15:05 ` Vlastimil Babka @ 2020-10-19 7:32 ` Xu, Yanfei 0 siblings, 0 replies; 4+ messages in thread From: Xu, Yanfei @ 2020-10-19 7:32 UTC (permalink / raw) To: Vlastimil Babka, David Hildenbrand, akpm; +Cc: linux-mm, linux-kernel On 10/16/20 11:05 PM, Vlastimil Babka wrote: > On 10/14/20 2:28 PM, David Hildenbrand wrote: >> On 14.10.20 09:23, yanfei.xu@windriver.com wrote: >>> From: Yanfei Xu <yanfei.xu@windriver.com> >>> >>> start_pfn has been declared at the begin of compact_zone(), it's >>> no need to declare it again. And remove an useless semicolon. >>> >>> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> >>> --- >>> mm/compaction.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/mm/compaction.c b/mm/compaction.c >>> index 176dcded298e..5e69c1f94d96 100644 >>> --- a/mm/compaction.c >>> +++ b/mm/compaction.c >>> @@ -2272,7 +2272,7 @@ compact_zone(struct compact_control *cc, struct >>> capture_control *capc) >>> >>> while ((ret = compact_finished(cc)) == COMPACT_CONTINUE) { >>> int err; >>> - unsigned long start_pfn = cc->migrate_pfn; >>> + start_pfn = cc->migrate_pfn; >> >> There is a user in >> >> trace_mm_compaction_end(start_pfn, cc->migrate_pfn, cc->free_pfn, >> end_pfn, sync, ret); >> >> we would now trace a different value, no? > > Agreed. We should rather rename the while-local one to avoid confusion. > Something like "iteration_start_pfn" ? > Thank you, David and Vlastimil, for pointing out the impact to the tracepoint. I think "iteration_start_pfn" is appropriate and will send V2. >>> >>> /* >>> * Avoid multiple rescans which can happen if a page cannot be >>> @@ -2309,7 +2309,6 @@ compact_zone(struct compact_control *cc, struct >>> capture_control *capc) >>> case ISOLATE_SUCCESS: >>> update_cached = false; >>> last_migrated_pfn = start_pfn; >>> - ; >> >> Huh, how does something like that happen :) > > Looks like "case ISOLATE_SUCCESS:" used to be an empty implementation, > then statements got added, but semicolon not removed. Yup, this case used to be an empty. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-19 7:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-14 7:23 [PATCH] mm/compaction: Remove some useless code in compact_zone() yanfei.xu 2020-10-14 12:28 ` David Hildenbrand 2020-10-16 15:05 ` Vlastimil Babka 2020-10-19 7:32 ` Xu, Yanfei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).