[PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[Yafang Shao]

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(-)

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);
-- 
1.8.3.1

Re: [PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[Vlastimil Babka]

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);
> 

Re: [PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[Yafang Shao]

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);
> >
>

Re: [PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[Vlastimil Babka]

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

Re: [PATCH] mm/compaction: fix missed direct_compaction setting for non-direct compaction

[Yafang Shao]

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