* [patch] mm, compaction: properly initialize alloc_flags in compact_control
@ 2017-10-16 22:03 David Rientjes
2017-10-16 22:12 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: David Rientjes @ 2017-10-16 22:03 UTC (permalink / raw)
To: Andrew Morton; +Cc: Vlastimil Babka, Joonsoo Kim, linux-kernel, linux-mm
compaction_suitable() requires a useful cc->alloc_flags, otherwise the
results of compact_zone() can be indeterminate. Kcompactd currently
checks compaction_suitable() itself with alloc_flags == 0, but passes an
uninitialized value from the stack to compact_zone(), which does its own
check.
The same is true for compact_node() when explicitly triggering full node
compaction.
Properly initialize cc.alloc_flags on the stack.
Signed-off-by: David Rientjes <rientjes@google.com>
---
mm/compaction.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1792,9 +1792,9 @@ static void compact_node(int nid)
{
pg_data_t *pgdat = NODE_DATA(nid);
int zoneid;
- struct zone *zone;
struct compact_control cc = {
.order = -1,
+ .alloc_flags = 0,
.total_migrate_scanned = 0,
.total_free_scanned = 0,
.mode = MIGRATE_SYNC,
@@ -1805,6 +1805,7 @@ static void compact_node(int nid)
for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) {
+ struct zone *zone;
zone = &pgdat->node_zones[zoneid];
if (!populated_zone(zone))
@@ -1923,6 +1924,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)
struct zone *zone;
struct compact_control cc = {
.order = pgdat->kcompactd_max_order,
+ .alloc_flags = 0,
.total_migrate_scanned = 0,
.total_free_scanned = 0,
.classzone_idx = pgdat->kcompactd_classzone_idx,
@@ -1945,8 +1947,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
if (compaction_deferred(zone, cc.order))
continue;
- if (compaction_suitable(zone, cc.order, 0, zoneid) !=
- COMPACT_CONTINUE)
+ if (compaction_suitable(zone, cc.order, cc.alloc_flags,
+ zoneid) != COMPACT_CONTINUE)
continue;
cc.nr_freepages = 0;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch] mm, compaction: properly initialize alloc_flags in compact_control
2017-10-16 22:03 [patch] mm, compaction: properly initialize alloc_flags in compact_control David Rientjes
@ 2017-10-16 22:12 ` Andrew Morton
2017-10-17 6:51 ` Vlastimil Babka
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2017-10-16 22:12 UTC (permalink / raw)
To: David Rientjes; +Cc: Vlastimil Babka, Joonsoo Kim, linux-kernel, linux-mm
On Mon, 16 Oct 2017 15:03:37 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
>
> compaction_suitable() requires a useful cc->alloc_flags, otherwise the
> results of compact_zone() can be indeterminate. Kcompactd currently
> checks compaction_suitable() itself with alloc_flags == 0, but passes an
> uninitialized value from the stack to compact_zone(), which does its own
> check.
>
> The same is true for compact_node() when explicitly triggering full node
> compaction.
>
> Properly initialize cc.alloc_flags on the stack.
>
The compiler will zero any not-explicitly-initialized fields in these
initializers.
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1792,9 +1792,9 @@ static void compact_node(int nid)
> {
> pg_data_t *pgdat = NODE_DATA(nid);
> int zoneid;
> - struct zone *zone;
> struct compact_control cc = {
> .order = -1,
> + .alloc_flags = 0,
> .total_migrate_scanned = 0,
> .total_free_scanned = 0,
> .mode = MIGRATE_SYNC,
> @@ -1805,6 +1805,7 @@ static void compact_node(int nid)
>
>
> for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) {
> + struct zone *zone;
>
> zone = &pgdat->node_zones[zoneid];
> if (!populated_zone(zone))
> @@ -1923,6 +1924,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)
> struct zone *zone;
> struct compact_control cc = {
> .order = pgdat->kcompactd_max_order,
> + .alloc_flags = 0,
> .total_migrate_scanned = 0,
> .total_free_scanned = 0,
> .classzone_idx = pgdat->kcompactd_classzone_idx,
> @@ -1945,8 +1947,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
> if (compaction_deferred(zone, cc.order))
> continue;
>
> - if (compaction_suitable(zone, cc.order, 0, zoneid) !=
> - COMPACT_CONTINUE)
> + if (compaction_suitable(zone, cc.order, cc.alloc_flags,
> + zoneid) != COMPACT_CONTINUE)
> continue;
So afaict the above hunk is the only functional change here. It will
propagate any of compact_zone()'s modifications to cc->alloc_flags into
succeeding calls to compaction_suitable(). I suspect this is a
no-op (didn't look), and it wasn't changelogged.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch] mm, compaction: properly initialize alloc_flags in compact_control
2017-10-16 22:12 ` Andrew Morton
@ 2017-10-17 6:51 ` Vlastimil Babka
0 siblings, 0 replies; 3+ messages in thread
From: Vlastimil Babka @ 2017-10-17 6:51 UTC (permalink / raw)
To: Andrew Morton, David Rientjes; +Cc: Joonsoo Kim, linux-kernel, linux-mm
On 10/17/2017 12:12 AM, Andrew Morton wrote:
> On Mon, 16 Oct 2017 15:03:37 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
>
>>
>> compaction_suitable() requires a useful cc->alloc_flags, otherwise the
>> results of compact_zone() can be indeterminate. Kcompactd currently
>> checks compaction_suitable() itself with alloc_flags == 0, but passes an
>> uninitialized value from the stack to compact_zone(), which does its own
>> check.
>>
>> The same is true for compact_node() when explicitly triggering full node
>> compaction.
>>
>> Properly initialize cc.alloc_flags on the stack.
>>
>
> The compiler will zero any not-explicitly-initialized fields in these
> initializers.
Right.
>> @@ -1945,8 +1947,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>> if (compaction_deferred(zone, cc.order))
>> continue;
>>
>> - if (compaction_suitable(zone, cc.order, 0, zoneid) !=
>> - COMPACT_CONTINUE)
>> + if (compaction_suitable(zone, cc.order, cc.alloc_flags,
>> + zoneid) != COMPACT_CONTINUE)
>> continue;
>
> So afaict the above hunk is the only functional change here. It will
> propagate any of compact_zone()'s modifications to cc->alloc_flags into
> succeeding calls to compaction_suitable(). I suspect this is a
> no-op (didn't look), and it wasn't changelogged.
compact_zone() shouldn't modify cc->alloc_flags. Actually, it's even
declared as "const" in struct compact_control.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-17 6:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 22:03 [patch] mm, compaction: properly initialize alloc_flags in compact_control David Rientjes
2017-10-16 22:12 ` Andrew Morton
2017-10-17 6:51 ` Vlastimil Babka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox