| Message ID | 1553848599-6124-1-git-send-email-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/compaction: fix missed direct_compaction setting for non-direct compaction | expand |
On 3/29/19 9:36 AM, Yafang Shao wrote: > direct_compaction is not initialized for kcompactd or manually triggered > compaction (via /proc or /sys). It doesn't need to, this style of initialization does guarantee that any field not explicitly mentioned is initialized to 0/NULL/false... and this pattern is used all over the kernel. > That may cause unexpected behavior in __compact_finished(), so we should > set direct_compaction to false explicitly for these compactions. It's not necessary. > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Cc: Vlastimil Babka <vbabka@suse.cz> > --- > mm/compaction.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 98f99f4..ba2b711 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -2400,13 +2400,12 @@ static void compact_node(int nid) > .total_free_scanned = 0, > .mode = MIGRATE_SYNC, > .ignore_skip_hint = true, > + .direct_compaction = false, > .whole_zone = true, > .gfp_mask = GFP_KERNEL, > }; > > - > for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) { > - > zone = &pgdat->node_zones[zoneid]; > if (!populated_zone(zone)) > continue; > @@ -2522,8 +2521,10 @@ static void kcompactd_do_work(pg_data_t *pgdat) > .classzone_idx = pgdat->kcompactd_classzone_idx, > .mode = MIGRATE_SYNC_LIGHT, > .ignore_skip_hint = false, > + .direct_compaction = false, > .gfp_mask = GFP_KERNEL, > }; > + > trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order, > cc.classzone_idx); > count_compact_event(KCOMPACTD_WAKE); >
On Fri, Mar 29, 2019 at 4:45 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 3/29/19 9:36 AM, Yafang Shao wrote: > > direct_compaction is not initialized for kcompactd or manually triggered > > compaction (via /proc or /sys). > > It doesn't need to, this style of initialization does guarantee that any > field not explicitly mentioned is initialized to 0/NULL/false... and > this pattern is used all over the kernel. > Hmm. You mean the gcc will set the local variable to 0 ? Are there any reference to this behavior ? Thanks Yafang > > That may cause unexpected behavior in __compact_finished(), so we should > > set direct_compaction to false explicitly for these compactions. > > It's not necessary. > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Cc: Vlastimil Babka <vbabka@suse.cz> > > --- > > mm/compaction.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/mm/compaction.c b/mm/compaction.c > > index 98f99f4..ba2b711 100644 > > --- a/mm/compaction.c > > +++ b/mm/compaction.c > > @@ -2400,13 +2400,12 @@ static void compact_node(int nid) > > .total_free_scanned = 0, > > .mode = MIGRATE_SYNC, > > .ignore_skip_hint = true, > > + .direct_compaction = false, > > .whole_zone = true, > > .gfp_mask = GFP_KERNEL, > > }; > > > > - > > for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) { > > - > > zone = &pgdat->node_zones[zoneid]; > > if (!populated_zone(zone)) > > continue; > > @@ -2522,8 +2521,10 @@ static void kcompactd_do_work(pg_data_t *pgdat) > > .classzone_idx = pgdat->kcompactd_classzone_idx, > > .mode = MIGRATE_SYNC_LIGHT, > > .ignore_skip_hint = false, > > + .direct_compaction = false, > > .gfp_mask = GFP_KERNEL, > > }; > > + > > trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order, > > cc.classzone_idx); > > count_compact_event(KCOMPACTD_WAKE); > > >
On 3/29/19 9:48 AM, Yafang Shao wrote: > On Fri, Mar 29, 2019 at 4:45 PM Vlastimil Babka <vbabka@suse.cz> wrote: >> >> On 3/29/19 9:36 AM, Yafang Shao wrote: >>> direct_compaction is not initialized for kcompactd or manually triggered >>> compaction (via /proc or /sys). >> >> It doesn't need to, this style of initialization does guarantee that any >> field not explicitly mentioned is initialized to 0/NULL/false... and >> this pattern is used all over the kernel. >> > > Hmm. > You mean the gcc will set the local variable to 0 ? Not local variable, but fields omitted in this "designated initializers" scenario. > Are there any reference to this behavior ? https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html "Omitted fields are implicitly initialized the same as for objects that have static storage duration. " and static objects are implicitly 0
On Fri, Mar 29, 2019 at 4:54 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 3/29/19 9:48 AM, Yafang Shao wrote: > > On Fri, Mar 29, 2019 at 4:45 PM Vlastimil Babka <vbabka@suse.cz> wrote: > >> > >> On 3/29/19 9:36 AM, Yafang Shao wrote: > >>> direct_compaction is not initialized for kcompactd or manually triggered > >>> compaction (via /proc or /sys). > >> > >> It doesn't need to, this style of initialization does guarantee that any > >> field not explicitly mentioned is initialized to 0/NULL/false... and > >> this pattern is used all over the kernel. > >> > > > > Hmm. > > You mean the gcc will set the local variable to 0 ? > > Not local variable, but fields omitted in this "designated initializers" > scenario. > > > Are there any reference to this behavior ? > > https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html > > "Omitted fields are implicitly initialized the same as for objects that > have static storage duration. " > and static objects are implicitly 0 > Got it! Many thanks for your help. Thanks Yafang
diff --git a/mm/compaction.c b/mm/compaction.c index 98f99f4..ba2b711 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -2400,13 +2400,12 @@ static void compact_node(int nid) .total_free_scanned = 0, .mode = MIGRATE_SYNC, .ignore_skip_hint = true, + .direct_compaction = false, .whole_zone = true, .gfp_mask = GFP_KERNEL, }; - for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) { - zone = &pgdat->node_zones[zoneid]; if (!populated_zone(zone)) continue; @@ -2522,8 +2521,10 @@ static void kcompactd_do_work(pg_data_t *pgdat) .classzone_idx = pgdat->kcompactd_classzone_idx, .mode = MIGRATE_SYNC_LIGHT, .ignore_skip_hint = false, + .direct_compaction = false, .gfp_mask = GFP_KERNEL, }; + trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order, cc.classzone_idx); count_compact_event(KCOMPACTD_WAKE);
direct_compaction is not initialized for kcompactd or manually triggered compaction (via /proc or /sys). That may cause unexpected behavior in __compact_finished(), so we should set direct_compaction to false explicitly for these compactions. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Vlastimil Babka <vbabka@suse.cz> --- mm/compaction.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)