Currently we have no way to determine if compaction was triggered
by sysctl write, but this is an interesting information to have,
specially in systems with high uptime that presents lots of
fragmented memory. There's no statistic indicating if compaction
was triggered manually or ran by Linux itself, the vmstat numbers
cannot tell the user this information.
This patch adds a very simple message to kernel log when compaction
is requested through a write to sysctl file, and also it accumulates
the number of previously manual compaction executions. It follows
the approach used by drop_caches.
Signed-off-by: Guilherme G. Piccoli <[email protected]>
---
This patch was based on linux-next/akpm branch, at d0f3f6070c3a.
Thanks in advance for reviews!
Cheers,
Guilherme
mm/compaction.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index fb43e731ac31..80e748b0bbb6 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2465,9 +2465,13 @@ int sysctl_compact_memory;
int sysctl_compaction_handler(struct ctl_table *table, int write,
void *buffer, size_t *length, loff_t *ppos)
{
- if (write)
+ static unsigned compaction_acct;
+
+ if (write) {
+ pr_info("compact_memory triggered - it already previously ran %u times\n",
+ compaction_acct++);
compact_nodes();
-
+ }
return 0;
}
--
2.25.2
On Thu, 7 May 2020 18:59:46 -0300 "Guilherme G. Piccoli" <[email protected]> wrote:
> Currently we have no way to determine if compaction was triggered
> by sysctl write, but this is an interesting information to have,
> specially in systems with high uptime that presents lots of
> fragmented memory. There's no statistic indicating if compaction
> was triggered manually or ran by Linux itself, the vmstat numbers
> cannot tell the user this information.
Could add it to vmstat?
> This patch adds a very simple message to kernel log when compaction
> is requested through a write to sysctl file, and also it accumulates
> the number of previously manual compaction executions. It follows
> the approach used by drop_caches.
Userspace could write to /dev/kmsg when it decides to trigger
compaction? Although using the kernel log seems a fairly lame way for
userspace to record its own actions...
On Thu, May 7, 2020 at 8:04 PM Andrew Morton <[email protected]> wrote:
>
> Could add it to vmstat?
Hi Andrew, thanks for your suggestion! I thought the same, as a second
potential solution for this..was planning to add as a comment below
the "---" but forgot heheh
I agree that would be great in vmstat, do you have a name suggestion
for it? "compaction_triggered" maybe?
> Userspace could write to /dev/kmsg when it decides to trigger
> compaction? Although using the kernel log seems a fairly lame way for
> userspace to record its own actions...
Well...you can think that the problem we are trying to solve was more
like...admin forgot if they triggered or not the compaction hehe
So, counting on the user to keep track of it is what I'd like to
avoid. And thinking about drop_caches (that have an indication when
triggered) makes me think this indeed does make sense for compaction
too.
Cheers,
Guilherme
On Thu, 7 May 2020, Guilherme G. Piccoli wrote:
> Well...you can think that the problem we are trying to solve was more
> like...admin forgot if they triggered or not the compaction hehe
> So, counting on the user to keep track of it is what I'd like to
> avoid. And thinking about drop_caches (that have an indication when
> triggered) makes me think this indeed does make sense for compaction
> too.
> Cheers,
>
It doesn't make sense because it's only being done here for the entire
system, there are also per-node sysfs triggers so you could do something
like iterate over the nodemask of all nodes with memory and trigger
compaction manually and then nothing is emitted to the kernel log.
There is new statsfs support that Red Hat is proposing that can be used
for things like this. It currently only supports KVM statistics but
adding MM statistics is something that would be a natural extension and
avoids polluting both the kernel log and /proc/vmstat.
On Fri, May 8, 2020 at 3:31 PM David Rientjes <[email protected]> wrote:
> It doesn't make sense because it's only being done here for the entire
> system, there are also per-node sysfs triggers so you could do something
> like iterate over the nodemask of all nodes with memory and trigger
> compaction manually and then nothing is emitted to the kernel log.
>
> There is new statsfs support that Red Hat is proposing that can be used
> for things like this. It currently only supports KVM statistics but
> adding MM statistics is something that would be a natural extension and
> avoids polluting both the kernel log and /proc/vmstat.
Thanks for the review. Is this what you're talking about [0] ? Very interesting!
Also, I agree about the per-node compaction, it's a good point. But at
the same time, having the information on the number of manual
compaction triggered is interesting, at least for some users. What if
we add that as a per-node stat in zoneinfo?
Cheers,
Guilherme
[0] lore.kernel.org/kvm/[email protected]
On Fri, 8 May 2020, Guilherme Piccoli wrote:
> On Fri, May 8, 2020 at 3:31 PM David Rientjes <[email protected]> wrote:
> > It doesn't make sense because it's only being done here for the entire
> > system, there are also per-node sysfs triggers so you could do something
> > like iterate over the nodemask of all nodes with memory and trigger
> > compaction manually and then nothing is emitted to the kernel log.
> >
> > There is new statsfs support that Red Hat is proposing that can be used
> > for things like this. It currently only supports KVM statistics but
> > adding MM statistics is something that would be a natural extension and
> > avoids polluting both the kernel log and /proc/vmstat.
>
> Thanks for the review. Is this what you're talking about [0] ? Very interesting!
>
Exactly.
> Also, I agree about the per-node compaction, it's a good point. But at
> the same time, having the information on the number of manual
> compaction triggered is interesting, at least for some users. What if
> we add that as a per-node stat in zoneinfo?
>
The kernel log is not preferred for this (or drop_caches, really) because
the amount of info can causing important information to be lost. We don't
really gain anything by printing that someone manually triggered
compaction; they could just write to the kernel log themselves if they
really wanted to. The reverse is not true: we can't suppress your kernel
message with this patch.
Instead, a statsfs-like approach could be used to indicate when this has
happened and there is no chance of losing events because it got scrolled
off the kernel log. It has the added benefit of not requiring the entire
log to be parsed for such events.
On Sun, May 10, 2020 at 10:25 PM David Rientjes <[email protected]> wrote:
> [...]
> The kernel log is not preferred for this (or drop_caches, really) because
> the amount of info can causing important information to be lost. We don't
> really gain anything by printing that someone manually triggered
> compaction; they could just write to the kernel log themselves if they
> really wanted to. The reverse is not true: we can't suppress your kernel
> message with this patch.
>
> Instead, a statsfs-like approach could be used to indicate when this has
> happened and there is no chance of losing events because it got scrolled
> off the kernel log. It has the added benefit of not requiring the entire
> log to be parsed for such events.
OK, agreed! Let's forget the kernel log. So, do you think the way to
go is the statsfs, not a zoneinfo stat, a per-node thing? I'm saying
that because kernel mm subsystem statistics seem pretty.."comfortable"
the way they are, in files like vmstat, zoneinfo, etc. Let me know
your thoughts on this, if I could work on that or should wait statsfs.
Thanks,
Guilherme
On 5/11/20 1:26 PM, Guilherme Piccoli wrote:
> On Sun, May 10, 2020 at 10:25 PM David Rientjes <[email protected]> wrote:
>> [...]
>> The kernel log is not preferred for this (or drop_caches, really) because
>> the amount of info can causing important information to be lost. We don't
>> really gain anything by printing that someone manually triggered
>> compaction; they could just write to the kernel log themselves if they
>> really wanted to. The reverse is not true: we can't suppress your kernel
>> message with this patch.
>>
>> Instead, a statsfs-like approach could be used to indicate when this has
>> happened and there is no chance of losing events because it got scrolled
>> off the kernel log. It has the added benefit of not requiring the entire
>> log to be parsed for such events.
> OK, agreed! Let's forget the kernel log. So, do you think the way to
> go is the statsfs, not a zoneinfo stat, a per-node thing? I'm saying
> that because kernel mm subsystem statistics seem pretty.."comfortable"
> the way they are, in files like vmstat, zoneinfo, etc. Let me know
> your thoughts on this, if I could work on that or should wait statsfs.
> Thanks,
>
>
> Guilherme
I think a trace notification in compaction like kcompad_wake would be good.
Hi Peter, thanks for the feedback. What do you mean by "trace
notification" ? We seem to have a trace event in that function you
mentioned. Also, accounting for that function is enough to
differentiate when the compaction is triggered by the kernel itself or
by the user (which is our use case here) ?
Cheers,
Guilherme
On 5/18/20 2:14 PM, Guilherme Piccoli wrote:
> Hi Peter, thanks for the feedback. What do you mean by "trace
> notification" ? We seem to have a trace event in that function you
> mentioned. Also, accounting for that function is enough to
> differentiate when the compaction is triggered by the kernel itself or
> by the user (which is our use case here) ?
Usually change existing causes confusion. It should not be a problem but it happen.
> Cheers,
>
>
> Guilherme
On Mon, May 18, 2020 at 9:54 AM Enderborg, Peter
<[email protected]> wrote:
> Usually change existing causes confusion. It should not be a problem but it happen.
>
I am sorry, but I really didn't understand your statement, can you be
more specific?
Thanks again!