| Message ID | 20230418095852.RFC.1.I53bf7f0c7d48fe7af13c5dd3ad581d3bcfd9d1bd@changeid (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] mm, compaction: kcompactd work shouldn't count towards memory PSI | expand |
On Tue, Apr 18, 2023 at 09:58:54AM -0700, Douglas Anderson wrote: > When the main kcompactd thread is doing compaction then it's always > proactive compaction. This is a little confusing because kcompactd has > two phases and one of them is called the "proactive" phase. > Specifically: > * Phase 1 (the "non-proactive" phase): we've been told by someone else > that it would be a good idea to try to compact memory. > * Phase 2 (the "proactive" phase): we analyze memory fragmentation > ourselves and compact if it looks fragmented. > > From the context of kcompactd, the above naming makes sense. However, > from the context of the kernel as a whole both phases are "proactive" > because in both cases we're trying compact memory ahead of time and > we're not actually blocking (stalling) any task who is trying to use > memory. > > Specifically, if any task is actually blocked needing memory to be > compacted then it will be in direct reclaim. That won't block waiting > on kcompactd task but instead call try_to_compact_pages() directly. > The caller of that direct compaction, __alloc_pages_direct_compact(), > already marks itself as counting towards PSI. > > Sanity checking by looking at this from another perspective, we can > look at all the people who explicitly ask kcompactd to do a reclaim by > calling wakeup_kcompactd(). That leads us to 3 places in vmscan.c. > Those are all requests from kswapd, which is also a "proactive" > mechanism in the kernel (tasks aren't blocked waiting for it). There is a reason behind annotating kswapd/kcompactd like this, it's in the longish comment in psi.c: * The time in which a task can execute on a CPU is our baseline for * productivity. Pressure expresses the amount of time in which this * potential cannot be realized due to resource contention. * * This concept of productivity has two components: the workload and * the CPU. To measure the impact of pressure on both, we define two * contention states for a resource: SOME and FULL. * * In the SOME state of a given resource, one or more tasks are * delayed on that resource. This affects the workload's ability to * perform work, but the CPU may still be executing other tasks. * * In the FULL state of a given resource, all non-idle tasks are * delayed on that resource such that nobody is advancing and the CPU * goes idle. This leaves both workload and CPU unproductive. * * SOME = nr_delayed_tasks != 0 * FULL = nr_delayed_tasks != 0 && nr_productive_tasks == 0 * * What it means for a task to be productive is defined differently * for each resource. For IO, productive means a running task. For * memory, productive means a running task that isn't a reclaimer. For * CPU, productive means an oncpu task. So when you have a CPU that's running reclaim/compaction work, that CPU isn't available to execute the workload. Say you only have one CPU shared between an allocating thread and kswapd. Even if the allocating thread never has to do reclaim on its own, if it has to wait for the CPU behind kswapd 50% of the time, that workload is positively under memory pressure. I don't think the distinction between proactive and reactive is all that meaningful. It's generally assumed that all the work done by these background threads is work that later doesn't have to be done by an allocating thread. It might matter from a latency perspective, but otherwise the work is fungible as it relates to memory pressure. HTH
Hi, On Tue, Apr 18, 2023 at 12:53 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Tue, Apr 18, 2023 at 09:58:54AM -0700, Douglas Anderson wrote: > > When the main kcompactd thread is doing compaction then it's always > > proactive compaction. This is a little confusing because kcompactd has > > two phases and one of them is called the "proactive" phase. > > Specifically: > > * Phase 1 (the "non-proactive" phase): we've been told by someone else > > that it would be a good idea to try to compact memory. > > * Phase 2 (the "proactive" phase): we analyze memory fragmentation > > ourselves and compact if it looks fragmented. > > > > From the context of kcompactd, the above naming makes sense. However, > > from the context of the kernel as a whole both phases are "proactive" > > because in both cases we're trying compact memory ahead of time and > > we're not actually blocking (stalling) any task who is trying to use > > memory. > > > > Specifically, if any task is actually blocked needing memory to be > > compacted then it will be in direct reclaim. That won't block waiting > > on kcompactd task but instead call try_to_compact_pages() directly. > > The caller of that direct compaction, __alloc_pages_direct_compact(), > > already marks itself as counting towards PSI. > > > > Sanity checking by looking at this from another perspective, we can > > look at all the people who explicitly ask kcompactd to do a reclaim by > > calling wakeup_kcompactd(). That leads us to 3 places in vmscan.c. > > Those are all requests from kswapd, which is also a "proactive" > > mechanism in the kernel (tasks aren't blocked waiting for it). > > There is a reason behind annotating kswapd/kcompactd like this, it's > in the longish comment in psi.c: > > * The time in which a task can execute on a CPU is our baseline for > * productivity. Pressure expresses the amount of time in which this > * potential cannot be realized due to resource contention. > * > * This concept of productivity has two components: the workload and > * the CPU. To measure the impact of pressure on both, we define two > * contention states for a resource: SOME and FULL. > * > * In the SOME state of a given resource, one or more tasks are > * delayed on that resource. This affects the workload's ability to > * perform work, but the CPU may still be executing other tasks. > * > * In the FULL state of a given resource, all non-idle tasks are > * delayed on that resource such that nobody is advancing and the CPU > * goes idle. This leaves both workload and CPU unproductive. > * > * SOME = nr_delayed_tasks != 0 > * FULL = nr_delayed_tasks != 0 && nr_productive_tasks == 0 > * > * What it means for a task to be productive is defined differently > * for each resource. For IO, productive means a running task. For > * memory, productive means a running task that isn't a reclaimer. For > * CPU, productive means an oncpu task. Ah, thanks for the pointer! > So when you have a CPU that's running reclaim/compaction work, that > CPU isn't available to execute the workload. > > Say you only have one CPU shared between an allocating thread and > kswapd. Even if the allocating thread never has to do reclaim on its > own, if it has to wait for the CPU behind kswapd 50% of the time, that > workload is positively under memory pressure. I guess I'm so much in the mindset of having 2-8 CPUs that I didn't think as much about the single CPU case. What you say makes a lot of sense for the single CPU case or for very parallel workloads that take up all available CPUs, but what about when you've got extra CPUs sitting there idling? In that case we're really not taking any CPU cycles away from someone by having one of those CPUs doing compaction in the background. I'm sure this has been discussed before somewhere, but I'd also wonder if there's ever a reason why we should prioritize kswapd/kcompactd over user programs. AKA: why don't kswapd/kcompactd run with a big "sched_nice" value? If kswapd/kcompactd were low enough priority then even in the single CPU case (or the multiple CPU case with a very parallel workload) kswapd/kcompactd would never be taking away significant time from a real program. > I don't think the distinction between proactive and reactive is all > that meaningful. It's generally assumed that all the work done by > these background threads is work that later doesn't have to be done by > an allocating thread. It might matter from a latency perspective, but > otherwise the work is fungible as it relates to memory pressure. If all compaction should count towards PSI then it feels like we have a different bug. The proactive_compact_node() function should also be marked as counting towards PSI, shouldn't it? -Doug
diff --git a/mm/compaction.c b/mm/compaction.c index 5a9501e0ae01..5a8d78b506e4 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -22,7 +22,6 @@ #include <linux/kthread.h> #include <linux/freezer.h> #include <linux/page_owner.h> -#include <linux/psi.h> #include "internal.h" #ifdef CONFIG_COMPACTION @@ -2954,8 +2953,6 @@ static int kcompactd(void *p) pgdat->kcompactd_highest_zoneidx = pgdat->nr_zones - 1; while (!kthread_should_stop()) { - unsigned long pflags; - /* * Avoid the unnecessary wakeup for proactive compaction * when it is disabled. @@ -2967,9 +2964,8 @@ static int kcompactd(void *p) kcompactd_work_requested(pgdat), timeout) && !pgdat->proactive_compact_trigger) { - psi_memstall_enter(&pflags); kcompactd_do_work(pgdat); - psi_memstall_leave(&pflags); + /* * Reset the timeout value. The defer timeout from * proactive compaction is lost here but that is fine
When the main kcompactd thread is doing compaction then it's always proactive compaction. This is a little confusing because kcompactd has two phases and one of them is called the "proactive" phase. Specifically: * Phase 1 (the "non-proactive" phase): we've been told by someone else that it would be a good idea to try to compact memory. * Phase 2 (the "proactive" phase): we analyze memory fragmentation ourselves and compact if it looks fragmented. From the context of kcompactd, the above naming makes sense. However, from the context of the kernel as a whole both phases are "proactive" because in both cases we're trying compact memory ahead of time and we're not actually blocking (stalling) any task who is trying to use memory. Specifically, if any task is actually blocked needing memory to be compacted then it will be in direct reclaim. That won't block waiting on kcompactd task but instead call try_to_compact_pages() directly. The caller of that direct compaction, __alloc_pages_direct_compact(), already marks itself as counting towards PSI. Sanity checking by looking at this from another perspective, we can look at all the people who explicitly ask kcompactd to do a reclaim by calling wakeup_kcompactd(). That leads us to 3 places in vmscan.c. Those are all requests from kswapd, which is also a "proactive" mechanism in the kernel (tasks aren't blocked waiting for it). Fixes: eb414681d5a0 ("psi: pressure stall information for CPU, memory, and IO") Signed-off-by: Douglas Anderson <dianders@chromium.org> --- I stumbled upon this while researching for a different patch [1]. Although both the other patch and this one affect kcompactd, they are otherwise unrelated. It can be noted that ${SUBJECT} patch was created solely by code inspection. I don't have any specific test cases that are made better by it, the code just didn't seem quite right to me. My knowledge of the memory subsystem is shaky at best, so please take this patch with a grain of salt. If you're a memory system expert and this patch looks totally misguided to you then it probably is. ;-) [1] https://lore.kernel.org/r/20230413182313.RFC.1.Ia86ccac02a303154a0b8bc60567e7a95d34c96d3@changeid mm/compaction.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)