diff for duplicates of <20171025175424.GA14039@cmpxchg.org> diff --git a/a/2.txt b/N1/2.txt index 8b13789..2f4c9e8 100644 --- a/a/2.txt +++ b/N1/2.txt @@ -1 +1,73 @@ +>From 7318c963a582833d4556c51fc2e1658e00c14e3e Mon Sep 17 00:00:00 2001 +From: Johannes Weiner <hannes@cmpxchg.org> +Date: Thu, 5 Oct 2017 12:32:47 -0400 +Subject: [PATCH 1/3] mm: memdelay: fix task flags race condition +WARNING: CPU: 35 PID: 2263 at ../include/linux/memcontrol.h:466 + +This is memcg warning that current->memcg_may_oom is set when it +doesn't expect it to. + +The warning came in new with the memdelay patches. They add another +task flag in the same int as memcg_may_oom, but modify it from +try_to_wake_up, from a task that isn't current. This isn't safe. + +Move the flag to the other int holding task flags, whose modifications +are serialized through the scheduler locks. + +Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> +--- + include/linux/sched.h | 2 +- + kernel/sched/core.c | 8 ++++---- + 2 files changed, 5 insertions(+), 5 deletions(-) + +diff --git a/include/linux/sched.h b/include/linux/sched.h +index de15e3c8c43a..d1aa8f4c19ab 100644 +--- a/include/linux/sched.h ++++ b/include/linux/sched.h +@@ -627,6 +627,7 @@ struct task_struct { + unsigned sched_contributes_to_load:1; + unsigned sched_migrated:1; + unsigned sched_remote_wakeup:1; ++ unsigned sched_memdelay_requeue:1; + /* Force alignment to the next boundary: */ + unsigned :0; + +@@ -651,7 +652,6 @@ struct task_struct { + /* disallow userland-initiated cgroup migration */ + unsigned no_cgroup_migration:1; + #endif +- unsigned memdelay_migrate_enqueue:1; + + unsigned long atomic_flags; /* Flags requiring atomic access. */ + +diff --git a/kernel/sched/core.c b/kernel/sched/core.c +index bf105c870da6..b4fa806bf153 100644 +--- a/kernel/sched/core.c ++++ b/kernel/sched/core.c +@@ -760,10 +760,10 @@ static inline void enqueue_task(struct rq *rq, struct task_struct *p, int flags) + if (!(flags & ENQUEUE_RESTORE)) + sched_info_queued(rq, p); + +- WARN_ON_ONCE(!(flags & ENQUEUE_WAKEUP) && p->memdelay_migrate_enqueue); +- if (!(flags & ENQUEUE_WAKEUP) || p->memdelay_migrate_enqueue) { ++ WARN_ON_ONCE(!(flags & ENQUEUE_WAKEUP) && p->sched_memdelay_requeue); ++ if (!(flags & ENQUEUE_WAKEUP) || p->sched_memdelay_requeue) { + memdelay_add_runnable(p); +- p->memdelay_migrate_enqueue = 0; ++ p->sched_memdelay_requeue = 0; + } else { + memdelay_wakeup(p); + } +@@ -2065,8 +2065,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) + + rq = __task_rq_lock(p, &rf); + memdelay_del_sleeping(p); ++ p->sched_memdelay_requeue = 1; + __task_rq_unlock(rq, &rf); +- p->memdelay_migrate_enqueue = 1; + + set_task_cpu(p, cpu); + } +-- +2.14.2 diff --git a/N1/3.hdr b/N1/3.hdr new file mode 100644 index 0000000..9d039ab --- /dev/null +++ b/N1/3.hdr @@ -0,0 +1,2 @@ +Content-Type: text/x-diff; charset=us-ascii +Content-Disposition: attachment; filename="0002-mm-memdelay-idle-time-is-not-productive-time.patch" diff --git a/N1/3.txt b/N1/3.txt new file mode 100644 index 0000000..ba7cb43 --- /dev/null +++ b/N1/3.txt @@ -0,0 +1,51 @@ +>From 7157c70aed93990f59942d39d1c0d8948164cfe2 Mon Sep 17 00:00:00 2001 +From: Johannes Weiner <hannes@cmpxchg.org> +Date: Thu, 5 Oct 2017 12:34:49 -0400 +Subject: [PATCH 2/3] mm: memdelay: idle time is not productive time + +There is an error in the multi-core logic, where memory delay numbers +drop as the number of CPU cores increases. Idle CPUs, even though they +don't host delayed processes, shouldn't contribute to the "not +delayed" bucket. Because that's the baseline for productivity, and +idle CPUs aren't productive. + +Do not consider idle CPU time in the productivity baseline. + +Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> +--- + include/linux/memdelay.h | 3 ++- + mm/memdelay.c | 4 +++- + 2 files changed, 5 insertions(+), 2 deletions(-) + +diff --git a/include/linux/memdelay.h b/include/linux/memdelay.h +index d85d01692610..c49f65338c1f 100644 +--- a/include/linux/memdelay.h ++++ b/include/linux/memdelay.h +@@ -24,7 +24,8 @@ enum memdelay_task_state { + * productivity states of all tasks inside the domain. + */ + enum memdelay_domain_state { +- MDS_NONE, /* No delayed tasks */ ++ MDS_IDLE, /* No tasks */ ++ MDS_NONE, /* Working tasks */ + MDS_PART, /* Delayed tasks, working tasks */ + MDS_FULL, /* Delayed tasks, no working tasks */ + NR_MEMDELAY_DOMAIN_STATES, +diff --git a/mm/memdelay.c b/mm/memdelay.c +index c7c32dbb67ac..ea5ede79f044 100644 +--- a/mm/memdelay.c ++++ b/mm/memdelay.c +@@ -118,8 +118,10 @@ static void domain_cpu_update(struct memdelay_domain *md, int cpu, + else if (mdc->tasks[MTS_DELAYED]) + state = (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) ? + MDS_PART : MDS_FULL; +- else ++ else if (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) + state = MDS_NONE; ++ else ++ state = MDS_IDLE; + + if (mdc->state == state) + return; +-- +2.14.2 diff --git a/N1/4.hdr b/N1/4.hdr new file mode 100644 index 0000000..299c5ff --- /dev/null +++ b/N1/4.hdr @@ -0,0 +1,2 @@ +Content-Type: text/x-diff; charset=us-ascii +Content-Disposition: attachment; filename="0003-mm-memdelay-drop-IO-as-productive-time.patch" diff --git a/N1/4.txt b/N1/4.txt new file mode 100644 index 0000000..2080425 --- /dev/null +++ b/N1/4.txt @@ -0,0 +1,39 @@ +>From ea663e42b24871d370b6ddbfbf47c1775a2f9f09 Mon Sep 17 00:00:00 2001 +From: Johannes Weiner <jweiner@fb.com> +Date: Thu, 28 Sep 2017 10:36:39 -0700 +Subject: [PATCH 3/3] mm: memdelay: drop IO as productive time + +Counting IO as productive time distorts the sense of pressure with +workloads that are naturally IO-bound. Memory pressure can cause IO, +and thus cause "productive" IO to slow down - yet we don't attribute +this slowdown properly to a shortage of memory. + +Disregard IO time altogether, and use CPU time alone as the baseline. + +Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> +--- + mm/memdelay.c | 7 +++---- + 1 file changed, 3 insertions(+), 4 deletions(-) + +diff --git a/mm/memdelay.c b/mm/memdelay.c +index ea5ede79f044..fbce1d4ba142 100644 +--- a/mm/memdelay.c ++++ b/mm/memdelay.c +@@ -113,12 +113,11 @@ static void domain_cpu_update(struct memdelay_domain *md, int cpu, + * one running the workload, the domain is considered fully + * blocked 50% of the time. + */ +- if (mdc->tasks[MTS_DELAYED_ACTIVE] && !mdc->tasks[MTS_IOWAIT]) ++ if (mdc->tasks[MTS_DELAYED_ACTIVE]) + state = MDS_FULL; + else if (mdc->tasks[MTS_DELAYED]) +- state = (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) ? +- MDS_PART : MDS_FULL; +- else if (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) ++ state = mdc->tasks[MTS_RUNNABLE] ? MDS_PART : MDS_FULL; ++ else if (mdc->tasks[MTS_RUNNABLE]) + state = MDS_NONE; + else + state = MDS_IDLE; +-- +2.14.2 diff --git a/a/content_digest b/N1/content_digest index 6eb09cf..3f9d8c3 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -76,5 +76,174 @@ "\01:2\0" "fn\00001-mm-memdelay-fix-task-flags-race-condition.patch\0" "b\0" + ">From 7318c963a582833d4556c51fc2e1658e00c14e3e Mon Sep 17 00:00:00 2001\n" + "From: Johannes Weiner <hannes@cmpxchg.org>\n" + "Date: Thu, 5 Oct 2017 12:32:47 -0400\n" + "Subject: [PATCH 1/3] mm: memdelay: fix task flags race condition\n" + "\n" + "WARNING: CPU: 35 PID: 2263 at ../include/linux/memcontrol.h:466\n" + "\n" + "This is memcg warning that current->memcg_may_oom is set when it\n" + "doesn't expect it to.\n" + "\n" + "The warning came in new with the memdelay patches. They add another\n" + "task flag in the same int as memcg_may_oom, but modify it from\n" + "try_to_wake_up, from a task that isn't current. This isn't safe.\n" + "\n" + "Move the flag to the other int holding task flags, whose modifications\n" + "are serialized through the scheduler locks.\n" + "\n" + "Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>\n" + "---\n" + " include/linux/sched.h | 2 +-\n" + " kernel/sched/core.c | 8 ++++----\n" + " 2 files changed, 5 insertions(+), 5 deletions(-)\n" + "\n" + "diff --git a/include/linux/sched.h b/include/linux/sched.h\n" + "index de15e3c8c43a..d1aa8f4c19ab 100644\n" + "--- a/include/linux/sched.h\n" + "+++ b/include/linux/sched.h\n" + "@@ -627,6 +627,7 @@ struct task_struct {\n" + " \tunsigned\t\t\tsched_contributes_to_load:1;\n" + " \tunsigned\t\t\tsched_migrated:1;\n" + " \tunsigned\t\t\tsched_remote_wakeup:1;\n" + "+\tunsigned\t\t\tsched_memdelay_requeue:1;\n" + " \t/* Force alignment to the next boundary: */\n" + " \tunsigned\t\t\t:0;\n" + " \n" + "@@ -651,7 +652,6 @@ struct task_struct {\n" + " \t/* disallow userland-initiated cgroup migration */\n" + " \tunsigned\t\t\tno_cgroup_migration:1;\n" + " #endif\n" + "-\tunsigned\t\t\tmemdelay_migrate_enqueue:1;\n" + " \n" + " \tunsigned long\t\t\tatomic_flags; /* Flags requiring atomic access. */\n" + " \n" + "diff --git a/kernel/sched/core.c b/kernel/sched/core.c\n" + "index bf105c870da6..b4fa806bf153 100644\n" + "--- a/kernel/sched/core.c\n" + "+++ b/kernel/sched/core.c\n" + "@@ -760,10 +760,10 @@ static inline void enqueue_task(struct rq *rq, struct task_struct *p, int flags)\n" + " \tif (!(flags & ENQUEUE_RESTORE))\n" + " \t\tsched_info_queued(rq, p);\n" + " \n" + "-\tWARN_ON_ONCE(!(flags & ENQUEUE_WAKEUP) && p->memdelay_migrate_enqueue);\n" + "-\tif (!(flags & ENQUEUE_WAKEUP) || p->memdelay_migrate_enqueue) {\n" + "+\tWARN_ON_ONCE(!(flags & ENQUEUE_WAKEUP) && p->sched_memdelay_requeue);\n" + "+\tif (!(flags & ENQUEUE_WAKEUP) || p->sched_memdelay_requeue) {\n" + " \t\tmemdelay_add_runnable(p);\n" + "-\t\tp->memdelay_migrate_enqueue = 0;\n" + "+\t\tp->sched_memdelay_requeue = 0;\n" + " \t} else {\n" + " \t\tmemdelay_wakeup(p);\n" + " \t}\n" + "@@ -2065,8 +2065,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)\n" + " \n" + " \t\trq = __task_rq_lock(p, &rf);\n" + " \t\tmemdelay_del_sleeping(p);\n" + "+\t\tp->sched_memdelay_requeue = 1;\n" + " \t\t__task_rq_unlock(rq, &rf);\n" + "-\t\tp->memdelay_migrate_enqueue = 1;\n" + " \n" + " \t\tset_task_cpu(p, cpu);\n" + " \t}\n" + "-- \n" + 2.14.2 + "\01:3\0" + "fn\00002-mm-memdelay-idle-time-is-not-productive-time.patch\0" + "b\0" + ">From 7157c70aed93990f59942d39d1c0d8948164cfe2 Mon Sep 17 00:00:00 2001\n" + "From: Johannes Weiner <hannes@cmpxchg.org>\n" + "Date: Thu, 5 Oct 2017 12:34:49 -0400\n" + "Subject: [PATCH 2/3] mm: memdelay: idle time is not productive time\n" + "\n" + "There is an error in the multi-core logic, where memory delay numbers\n" + "drop as the number of CPU cores increases. Idle CPUs, even though they\n" + "don't host delayed processes, shouldn't contribute to the \"not\n" + "delayed\" bucket. Because that's the baseline for productivity, and\n" + "idle CPUs aren't productive.\n" + "\n" + "Do not consider idle CPU time in the productivity baseline.\n" + "\n" + "Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>\n" + "---\n" + " include/linux/memdelay.h | 3 ++-\n" + " mm/memdelay.c | 4 +++-\n" + " 2 files changed, 5 insertions(+), 2 deletions(-)\n" + "\n" + "diff --git a/include/linux/memdelay.h b/include/linux/memdelay.h\n" + "index d85d01692610..c49f65338c1f 100644\n" + "--- a/include/linux/memdelay.h\n" + "+++ b/include/linux/memdelay.h\n" + "@@ -24,7 +24,8 @@ enum memdelay_task_state {\n" + " * productivity states of all tasks inside the domain.\n" + " */\n" + " enum memdelay_domain_state {\n" + "-\tMDS_NONE,\t\t/* No delayed tasks */\n" + "+\tMDS_IDLE,\t\t/* No tasks */\n" + "+\tMDS_NONE,\t\t/* Working tasks */\n" + " \tMDS_PART,\t\t/* Delayed tasks, working tasks */\n" + " \tMDS_FULL,\t\t/* Delayed tasks, no working tasks */\n" + " \tNR_MEMDELAY_DOMAIN_STATES,\n" + "diff --git a/mm/memdelay.c b/mm/memdelay.c\n" + "index c7c32dbb67ac..ea5ede79f044 100644\n" + "--- a/mm/memdelay.c\n" + "+++ b/mm/memdelay.c\n" + "@@ -118,8 +118,10 @@ static void domain_cpu_update(struct memdelay_domain *md, int cpu,\n" + " \telse if (mdc->tasks[MTS_DELAYED])\n" + " \t\tstate = (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) ?\n" + " \t\t\tMDS_PART : MDS_FULL;\n" + "-\telse\n" + "+\telse if (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT])\n" + " \t\tstate = MDS_NONE;\n" + "+\telse\n" + "+\t\tstate = MDS_IDLE;\n" + " \n" + " \tif (mdc->state == state)\n" + " \t\treturn;\n" + "-- \n" + 2.14.2 + "\01:4\0" + "fn\00003-mm-memdelay-drop-IO-as-productive-time.patch\0" + "b\0" + ">From ea663e42b24871d370b6ddbfbf47c1775a2f9f09 Mon Sep 17 00:00:00 2001\n" + "From: Johannes Weiner <jweiner@fb.com>\n" + "Date: Thu, 28 Sep 2017 10:36:39 -0700\n" + "Subject: [PATCH 3/3] mm: memdelay: drop IO as productive time\n" + "\n" + "Counting IO as productive time distorts the sense of pressure with\n" + "workloads that are naturally IO-bound. Memory pressure can cause IO,\n" + "and thus cause \"productive\" IO to slow down - yet we don't attribute\n" + "this slowdown properly to a shortage of memory.\n" + "\n" + "Disregard IO time altogether, and use CPU time alone as the baseline.\n" + "\n" + "Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>\n" + "---\n" + " mm/memdelay.c | 7 +++----\n" + " 1 file changed, 3 insertions(+), 4 deletions(-)\n" + "\n" + "diff --git a/mm/memdelay.c b/mm/memdelay.c\n" + "index ea5ede79f044..fbce1d4ba142 100644\n" + "--- a/mm/memdelay.c\n" + "+++ b/mm/memdelay.c\n" + "@@ -113,12 +113,11 @@ static void domain_cpu_update(struct memdelay_domain *md, int cpu,\n" + " \t * one running the workload, the domain is considered fully\n" + " \t * blocked 50% of the time.\n" + " \t */\n" + "-\tif (mdc->tasks[MTS_DELAYED_ACTIVE] && !mdc->tasks[MTS_IOWAIT])\n" + "+\tif (mdc->tasks[MTS_DELAYED_ACTIVE])\n" + " \t\tstate = MDS_FULL;\n" + " \telse if (mdc->tasks[MTS_DELAYED])\n" + "-\t\tstate = (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT]) ?\n" + "-\t\t\tMDS_PART : MDS_FULL;\n" + "-\telse if (mdc->tasks[MTS_RUNNABLE] || mdc->tasks[MTS_IOWAIT])\n" + "+\t\tstate = mdc->tasks[MTS_RUNNABLE] ? MDS_PART : MDS_FULL;\n" + "+\telse if (mdc->tasks[MTS_RUNNABLE])\n" + " \t\tstate = MDS_NONE;\n" + " \telse\n" + " \t\tstate = MDS_IDLE;\n" + "-- \n" + 2.14.2 -05777cca4b3ef0adfbb24612442d7732f72224e380482ee0f5177fcfe6272449 +79f49e1ef4a377e1fe48609af664ea080767f512971402c409c7516789a55f00
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.