* [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
@ 2023-04-11 18:24 wenyang.linux
2023-04-11 20:48 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: wenyang.linux @ 2023-04-11 18:24 UTC (permalink / raw)
To: Andrew Morton
Cc: Wen Yang, Mel Gorman, Oscar Salvador, William Lam, Fu Wei,
linux-mm, linux-kernel
From: Wen Yang <wenyang.linux@foxmail.com>
For the /proc/sys/vm/compact_memory file, the admin-guide states:
When 1 is written to the file, all zones are compacted such that free
memory is available in contiguous blocks where possible. This can be
important for example in the allocation of huge pages although processes
will also directly compact memory as required
But it was not strictly followed, writing any value would cause all
zones to be compacted. In some critical scenarios, some applications
operating it, such as echo 0, have caused serious problems.
It has been slightly optimized to comply with the admin-guide.
Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: William Lam <william.lam@bytedance.com>
Cc: Fu Wei <wefu@redhat.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
mm/compaction.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/mm/compaction.c b/mm/compaction.c
index c8bcdea15f5f..3c4aa533d61c 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2780,6 +2780,17 @@ static int compaction_proactiveness_sysctl_handler(struct ctl_table *table, int
static int sysctl_compaction_handler(struct ctl_table *table, int write,
void *buffer, size_t *length, loff_t *ppos)
{
+ struct ctl_table t;
+ int compact;
+ int ret;
+
+ t = *table;
+ t.data = &compact;
+
+ ret = proc_dointvec_minmax(&t, write, buffer, length, ppos);
+ if (ret)
+ return ret;
+
if (write)
compact_nodes();
@@ -3099,6 +3110,8 @@ static struct ctl_table vm_compaction[] = {
.maxlen = sizeof(int),
.mode = 0200,
.proc_handler = sysctl_compaction_handler,
+ .extra1 = SYSCTL_ONE,
+ .extra2 = SYSCTL_ONE,
},
{
.procname = "compaction_proactiveness",
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
2023-04-11 18:24 [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide wenyang.linux
@ 2023-04-11 20:48 ` Andrew Morton
2023-04-12 16:54 ` Wen Yang
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2023-04-11 20:48 UTC (permalink / raw)
To: wenyang.linux
Cc: Mel Gorman, Oscar Salvador, William Lam, Fu Wei, linux-mm, linux-kernel
On Wed, 12 Apr 2023 02:24:26 +0800 wenyang.linux@foxmail.com wrote:
> For the /proc/sys/vm/compact_memory file, the admin-guide states:
> When 1 is written to the file, all zones are compacted such that free
> memory is available in contiguous blocks where possible. This can be
> important for example in the allocation of huge pages although processes
> will also directly compact memory as required
>
> But it was not strictly followed, writing any value would cause all
> zones to be compacted. In some critical scenarios, some applications
> operating it, such as echo 0, have caused serious problems.
Really? You mean someone actually did this and didn't observe the
effect during their testing?
> It has been slightly optimized to comply with the admin-guide.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
2023-04-11 20:48 ` Andrew Morton
@ 2023-04-12 16:54 ` Wen Yang
2023-04-15 17:42 ` Wen Yang
0 siblings, 1 reply; 6+ messages in thread
From: Wen Yang @ 2023-04-12 16:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Mel Gorman, Oscar Salvador, William Lam, Fu Wei, linux-mm, linux-kernel
在 2023/4/12 04:48, Andrew Morton 写道:
> On Wed, 12 Apr 2023 02:24:26 +0800 wenyang.linux@foxmail.com wrote:
>
>> For the /proc/sys/vm/compact_memory file, the admin-guide states:
>> When 1 is written to the file, all zones are compacted such that free
>> memory is available in contiguous blocks where possible. This can be
>> important for example in the allocation of huge pages although processes
>> will also directly compact memory as required
>>
>> But it was not strictly followed, writing any value would cause all
>> zones to be compacted. In some critical scenarios, some applications
>> operating it, such as echo 0, have caused serious problems.
> Really? You mean someone actually did this and didn't observe the
> effect during their testing?
Thanks for your reply.
Since /proc/sys/vm/compact_memory has been well documented for over a
decade:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/admin-guide/sysctl/vm.rst#n109
it is believed that only writing 1 will trigger trigger all zones to be
compacted.
Especially for those who write applications, they may only focus on
documentation and generally do not read kernel code. Moreover, such
problems are not easily detected through testing on low pressure machines.
Writing any meaningful or meaningless values will trigger it and affect
the entire server:
# echo 1 > /proc/sys/vm/compact_memory
# echo 0 > /proc/sys/vm/compact_memory
# echo dead > /proc/sys/vm/compact_memory
# echo "hello world" > /proc/sys/vm/compact_memory
The implementation of this high-risk operation may require following the
admin-guides.
--
Best wishes,
Wen
>> It has been slightly optimized to comply with the admin-guide.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
2023-04-12 16:54 ` Wen Yang
@ 2023-04-15 17:42 ` Wen Yang
2023-04-17 11:13 ` Mel Gorman
0 siblings, 1 reply; 6+ messages in thread
From: Wen Yang @ 2023-04-15 17:42 UTC (permalink / raw)
To: Andrew Morton
Cc: Mel Gorman, Oscar Salvador, William Lam, Fu Wei, linux-mm, linux-kernel
在 2023/4/13 00:54, Wen Yang 写道:
>
> 在 2023/4/12 04:48, Andrew Morton 写道:
>> On Wed, 12 Apr 2023 02:24:26 +0800 wenyang.linux@foxmail.com wrote:
>>
>>> For the /proc/sys/vm/compact_memory file, the admin-guide states:
>>> When 1 is written to the file, all zones are compacted such that free
>>> memory is available in contiguous blocks where possible. This can be
>>> important for example in the allocation of huge pages although
>>> processes
>>> will also directly compact memory as required
>>>
>>> But it was not strictly followed, writing any value would cause all
>>> zones to be compacted. In some critical scenarios, some applications
>>> operating it, such as echo 0, have caused serious problems.
>> Really? You mean someone actually did this and didn't observe the
>> effect during their testing?
>
> Thanks for your reply.
>
> Since /proc/sys/vm/compact_memory has been well documented for over a
> decade:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/admin-guide/sysctl/vm.rst#n109
>
>
> it is believed that only writing 1 will trigger trigger all zones to
> be compacted.
>
> Especially for those who write applications, they may only focus on
> documentation and generally do not read kernel code. Moreover, such
> problems are not easily detected through testing on low pressure
> machines.
>
> Writing any meaningful or meaningless values will trigger it and
> affect the entire server:
>
> # echo 1 > /proc/sys/vm/compact_memory
> # echo 0 > /proc/sys/vm/compact_memory
> # echo dead > /proc/sys/vm/compact_memory
> # echo "hello world" > /proc/sys/vm/compact_memory
>
> The implementation of this high-risk operation may require following
> the admin-guides.
>
> --
>
> Best wishes,
>
> Wen
>
>
Hello, do you think it's better to optimize the
sysctl_compaction_handler code or update the admin-guide document?
--
Best wishes,
Wen
>>> It has been slightly optimized to comply with the admin-guide.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
2023-04-15 17:42 ` Wen Yang
@ 2023-04-17 11:13 ` Mel Gorman
2023-04-18 14:10 ` Wen Yang
0 siblings, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2023-04-17 11:13 UTC (permalink / raw)
To: Wen Yang
Cc: Andrew Morton, Oscar Salvador, William Lam, Fu Wei, linux-mm,
linux-kernel
On Sun, Apr 16, 2023 at 01:42:44AM +0800, Wen Yang wrote:
>
> ??? 2023/4/13 00:54, Wen Yang ??????:
> >
> > ??? 2023/4/12 04:48, Andrew Morton ??????:
> > > On Wed, 12 Apr 2023 02:24:26 +0800 wenyang.linux@foxmail.com wrote:
> > >
> > > > For the /proc/sys/vm/compact_memory file, the admin-guide states:
> > > > When 1 is written to the file, all zones are compacted such that free
> > > > memory is available in contiguous blocks where possible. This can be
> > > > important for example in the allocation of huge pages although
> > > > processes
> > > > will also directly compact memory as required
> > > >
> > > > But it was not strictly followed, writing any value would cause all
> > > > zones to be compacted. In some critical scenarios, some applications
> > > > operating it, such as echo 0, have caused serious problems.
> > > Really? You mean someone actually did this and didn't observe the
> > > effect during their testing?
> >
> > Thanks for your reply.
> >
> > Since /proc/sys/vm/compact_memory has been well documented for over a
> > decade:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/admin-guide/sysctl/vm.rst#n109
> >
> >
> > it is believed that only writing 1 will trigger trigger all zones to be
> > compacted.
> >
> > Especially for those who write applications, they may only focus on
> > documentation and generally do not read kernel code. Moreover, such
> > problems are not easily detected through testing on low pressure
> > machines.
> >
> > Writing any meaningful or meaningless values will trigger it and affect
> > the entire server:
> >
> > # echo 1 > /proc/sys/vm/compact_memory
> > # echo 0 > /proc/sys/vm/compact_memory
> > # echo dead > /proc/sys/vm/compact_memory
> > # echo "hello world" > /proc/sys/vm/compact_memory
> >
> > The implementation of this high-risk operation may require following the
> > admin-guides.
> >
> > --
> >
> > Best wishes,
> >
> > Wen
> >
> >
> Hello, do you think it's better to optimize the sysctl_compaction_handler
> code or update the admin-guide document?
>
Enforce the 1 on the unlikely chance that the sysctl handler is ever
extended to do something different and expects a bitmask. The original
intent intent of the sysctl was debugging -- demonstrating a contiguous
allocation failure when aggressive compaction should have succeeded. Later
some machines dedicated to batch jobs used the compaction sysctl to compact
memory before a new job started to reduce startup latencies.
Drop the justification "In some critical scenarios, some applications
operating it, such as echo 0, have caused serious problems." from the
changelog. I cannot imagine a sane "critical scenario" where an application
running as root is writing expected garbage to proc or sysfs files and
then surprised when something unexpected happens.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide
2023-04-17 11:13 ` Mel Gorman
@ 2023-04-18 14:10 ` Wen Yang
0 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2023-04-18 14:10 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Oscar Salvador, William Lam, Fu Wei, linux-mm,
linux-kernel
在 2023/4/17 19:13, Mel Gorman 写道:
> On Sun, Apr 16, 2023 at 01:42:44AM +0800, Wen Yang wrote:
>> ??? 2023/4/13 00:54, Wen Yang ??????:
>>> ??? 2023/4/12 04:48, Andrew Morton ??????:
>>>> On Wed, 12 Apr 2023 02:24:26 +0800 wenyang.linux@foxmail.com wrote:
>>>>
>>>>> For the /proc/sys/vm/compact_memory file, the admin-guide states:
>>>>> When 1 is written to the file, all zones are compacted such that free
>>>>> memory is available in contiguous blocks where possible. This can be
>>>>> important for example in the allocation of huge pages although
>>>>> processes
>>>>> will also directly compact memory as required
>>>>>
>>>>> But it was not strictly followed, writing any value would cause all
>>>>> zones to be compacted. In some critical scenarios, some applications
>>>>> operating it, such as echo 0, have caused serious problems.
>>>> Really? You mean someone actually did this and didn't observe the
>>>> effect during their testing?
>>> Thanks for your reply.
>>>
>>> Since /proc/sys/vm/compact_memory has been well documented for over a
>>> decade:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/admin-guide/sysctl/vm.rst#n109
>>>
>>>
>>> it is believed that only writing 1 will trigger trigger all zones to be
>>> compacted.
>>>
>>> Especially for those who write applications, they may only focus on
>>> documentation and generally do not read kernel code. Moreover, such
>>> problems are not easily detected through testing on low pressure
>>> machines.
>>>
>>> Writing any meaningful or meaningless values will trigger it and affect
>>> the entire server:
>>>
>>> # echo 1 > /proc/sys/vm/compact_memory
>>> # echo 0 > /proc/sys/vm/compact_memory
>>> # echo dead > /proc/sys/vm/compact_memory
>>> # echo "hello world" > /proc/sys/vm/compact_memory
>>>
>>> The implementation of this high-risk operation may require following the
>>> admin-guides.
>>>
>>> --
>>>
>>> Best wishes,
>>>
>>> Wen
>>>
>>>
>> Hello, do you think it's better to optimize the sysctl_compaction_handler
>> code or update the admin-guide document?
>>
> Enforce the 1 on the unlikely chance that the sysctl handler is ever
> extended to do something different and expects a bitmask. The original
> intent intent of the sysctl was debugging -- demonstrating a contiguous
> allocation failure when aggressive compaction should have succeeded. Later
> some machines dedicated to batch jobs used the compaction sysctl to compact
> memory before a new job started to reduce startup latencies.
>
> Drop the justification "In some critical scenarios, some applications
> operating it, such as echo 0, have caused serious problems." from the
> changelog. I cannot imagine a sane "critical scenario" where an application
> running as root is writing expected garbage to proc or sysfs files and
> then surprised when something unexpected happens.
>
Thanks for your comments.
We will modify it according to your suggestion and then send v2.
--
Best wishes,
Wen
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-18 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 18:24 [PATCH] mm: compaction: optimize compact_memory to comply with the admin-guide wenyang.linux
2023-04-11 20:48 ` Andrew Morton
2023-04-12 16:54 ` Wen Yang
2023-04-15 17:42 ` Wen Yang
2023-04-17 11:13 ` Mel Gorman
2023-04-18 14:10 ` Wen Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).