2020-09-17 01:23:29

by Vijay Balakrishna

[permalink] [raw]
Subject: [v3 0/2] recalculate min_free_kbytes post memory hotplug

v2 -> v3
--------
[v2 1/2]
- removed symptoms references from changelog

[v2 2/2]
- addressed following issues Michal Hocko raised:
. nr_free_buffer_pages can oveflow in int on very large machines
. min_free_kbytes can decrease the size theoretically

v1 -> v2
--------
- addressed issue Kirill A. Shutemov raised:
. changes would override min_free_kbytes set by user

Vijay Balakrishna (2):
mm: khugepaged: recalculate min_free_kbytes after memory hotplug as
expected by khugepaged
mm: khugepaged: avoid overriding min_free_kbytes set by user

include/linux/khugepaged.h | 5 +++++
mm/khugepaged.c | 16 +++++++++++++---
mm/memory_hotplug.c | 3 +++
mm/page_alloc.c | 2 +-
4 files changed, 22 insertions(+), 4 deletions(-)

--
2.28.0


2020-09-17 01:23:50

by Vijay Balakrishna

[permalink] [raw]
Subject: [v3 2/2] mm: khugepaged: avoid overriding min_free_kbytes set by user

set_recommended_min_free_kbytes need to honor min_free_kbytes set by the
user. Post start-of-day THP enable or memory hotplug operations can
lose user specified min_free_kbytes, in particular when it is higher than
calculated recommended value. user_min_free_kbytes initialized to 0
to avoid undesired result when comparing with "unsigned long" type.

Signed-off-by: Vijay Balakrishna <[email protected]>
Cc: [email protected]
Reviewed-by: Pavel Tatashin <[email protected]>
---
mm/khugepaged.c | 3 ++-
mm/page_alloc.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 4f7107476a6f..3c1147816d12 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2283,7 +2283,8 @@ static void set_recommended_min_free_kbytes(void)
(unsigned long) nr_free_buffer_pages() / 20);
recommended_min <<= (PAGE_SHIFT-10);

- if (recommended_min > min_free_kbytes) {
+ if (recommended_min > min_free_kbytes ||
+ recommended_min > user_min_free_kbytes) {
if (user_min_free_kbytes >= 0)
pr_info("raising min_free_kbytes from %d to %lu to help transparent hugepage allocations\n",
min_free_kbytes, recommended_min);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..7b81fb139034 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -315,7 +315,7 @@ compound_page_dtor * const compound_page_dtors[NR_COMPOUND_DTORS] = {
};

int min_free_kbytes = 1024;
-int user_min_free_kbytes = -1;
+int user_min_free_kbytes = 0;
#ifdef CONFIG_DISCONTIGMEM
/*
* DiscontigMem defines memory ranges as separate pg_data_t even if the ranges
--
2.28.0

2020-09-17 01:24:12

by Vijay Balakrishna

[permalink] [raw]
Subject: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

When memory is hotplug added or removed the min_free_kbytes must be
recalculated based on what is expected by khugepaged. Currently
after hotplug, min_free_kbytes will be set to a lower default and higher
default set when THP enabled is lost. This change restores min_free_kbytes
as expected for THP consumers.

Fixes: f000565adb77 ("thp: set recommended min free kbytes")

Signed-off-by: Vijay Balakrishna <[email protected]>
Cc: [email protected]
Reviewed-by: Pavel Tatashin <[email protected]>
---
include/linux/khugepaged.h | 5 +++++
mm/khugepaged.c | 13 +++++++++++--
mm/memory_hotplug.c | 3 +++
3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/linux/khugepaged.h b/include/linux/khugepaged.h
index bc45ea1efbf7..c941b7377321 100644
--- a/include/linux/khugepaged.h
+++ b/include/linux/khugepaged.h
@@ -15,6 +15,7 @@ extern int __khugepaged_enter(struct mm_struct *mm);
extern void __khugepaged_exit(struct mm_struct *mm);
extern int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
unsigned long vm_flags);
+extern void khugepaged_min_free_kbytes_update(void);
#ifdef CONFIG_SHMEM
extern void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr);
#else
@@ -85,6 +86,10 @@ static inline void collapse_pte_mapped_thp(struct mm_struct *mm,
unsigned long addr)
{
}
+
+static inline void khugepaged_min_free_kbytes_update(void)
+{
+}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */

#endif /* _LINUX_KHUGEPAGED_H */
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index cfa0dba5fd3b..4f7107476a6f 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -56,6 +56,9 @@ enum scan_result {
#define CREATE_TRACE_POINTS
#include <trace/events/huge_memory.h>

+static struct task_struct *khugepaged_thread __read_mostly;
+static DEFINE_MUTEX(khugepaged_mutex);
+
/* default scan 8*512 pte (or vmas) every 30 second */
static unsigned int khugepaged_pages_to_scan __read_mostly;
static unsigned int khugepaged_pages_collapsed;
@@ -2292,8 +2295,6 @@ static void set_recommended_min_free_kbytes(void)

int start_stop_khugepaged(void)
{
- static struct task_struct *khugepaged_thread __read_mostly;
- static DEFINE_MUTEX(khugepaged_mutex);
int err = 0;

mutex_lock(&khugepaged_mutex);
@@ -2320,3 +2321,11 @@ int start_stop_khugepaged(void)
mutex_unlock(&khugepaged_mutex);
return err;
}
+
+void khugepaged_min_free_kbytes_update(void)
+{
+ mutex_lock(&khugepaged_mutex);
+ if (khugepaged_enabled() && khugepaged_thread)
+ set_recommended_min_free_kbytes();
+ mutex_unlock(&khugepaged_mutex);
+}
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e9d5ab5d3ca0..3e19272c1fad 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -36,6 +36,7 @@
#include <linux/memblock.h>
#include <linux/compaction.h>
#include <linux/rmap.h>
+#include <linux/khugepaged.h>

#include <asm/tlbflush.h>

@@ -857,6 +858,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
zone_pcp_update(zone);

init_per_zone_wmark_min();
+ khugepaged_min_free_kbytes_update();

kswapd_run(nid);
kcompactd_run(nid);
@@ -1600,6 +1602,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
pgdat_resize_unlock(zone->zone_pgdat, &flags);

init_per_zone_wmark_min();
+ khugepaged_min_free_kbytes_update();

if (!populated_zone(zone)) {
zone_pcp_reset(zone);
--
2.28.0

2020-09-17 04:23:55

by Vijay Balakrishna

[permalink] [raw]
Subject: Re: [v3 2/2] mm: khugepaged: avoid overriding min_free_kbytes set by user

Please ignore this patch. I forgot to run scripts/checkpatch.pl and see

CHECK: Alignment should match open parenthesis
#30: FILE: mm/khugepaged.c:2287:
+ if (recommended_min > min_free_kbytes ||
+ recommended_min > user_min_free_kbytes) {

ERROR: do not initialise globals to 0
#43: FILE: mm/page_alloc.c:318:
+int user_min_free_kbytes = 0;

Sorry for trouble, I will send a new version.

Vijay

On 9/16/2020 6:21 PM, Vijay Balakrishna wrote:
> set_recommended_min_free_kbytes need to honor min_free_kbytes set by the
> user. Post start-of-day THP enable or memory hotplug operations can
> lose user specified min_free_kbytes, in particular when it is higher than
> calculated recommended value. user_min_free_kbytes initialized to 0
> to avoid undesired result when comparing with "unsigned long" type.
>
> Signed-off-by: Vijay Balakrishna <[email protected]>
> Cc: [email protected]
> Reviewed-by: Pavel Tatashin <[email protected]>
> ---
> mm/khugepaged.c | 3 ++-
> mm/page_alloc.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 4f7107476a6f..3c1147816d12 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2283,7 +2283,8 @@ static void set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes) {
> + if (recommended_min > min_free_kbytes ||
> + recommended_min > user_min_free_kbytes) {
> if (user_min_free_kbytes >= 0)
> pr_info("raising min_free_kbytes from %d to %lu to help transparent hugepage allocations\n",
> min_free_kbytes, recommended_min);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..7b81fb139034 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -315,7 +315,7 @@ compound_page_dtor * const compound_page_dtors[NR_COMPOUND_DTORS] = {
> };
>
> int min_free_kbytes = 1024;
> -int user_min_free_kbytes = -1;
> +int user_min_free_kbytes = 0;
> #ifdef CONFIG_DISCONTIGMEM
> /*
> * DiscontigMem defines memory ranges as separate pg_data_t even if the ranges
>

2020-09-21 12:56:29

by Sasha Levin

[permalink] [raw]
Subject: Re: [v3 2/2] mm: khugepaged: avoid overriding min_free_kbytes set by user

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.8.10, v5.4.66, v4.19.146, v4.14.198, v4.9.236, v4.4.236.

v5.8.10: Build OK!
v5.4.66: Build OK!
v4.19.146: Failed to apply! Possible dependencies:
1c30844d2dfe ("mm: reclaim small amounts of memory when an external fragmentation event occurs")
24512228b7a3 ("mm: do not boost watermarks to avoid fragmentation for the DISCONTIG memory model")
426dcd4b600f ("hexagon: switch to NO_BOOTMEM")
6471f52af786 ("alpha: switch to NO_BOOTMEM")
6bb154504f8b ("mm, page_alloc: spread allocations across zones before introducing fragmentation")
9705bea5f833 ("mm: convert zone->managed_pages to atomic variable")
a921444382b4 ("mm: move zone watermark accesses behind an accessor")
b4a991ec584b ("mm: remove CONFIG_NO_BOOTMEM")
bc3ec75de545 ("dma-mapping: merge direct and noncoherent ops")
bda49a81164a ("mm: remove nobootmem")
c32e64e852f3 ("csky: Build infrastructure")
e0a9317d9004 ("hexagon: use generic dma_noncoherent_ops")
f406f222d4b2 ("hexagon: implement the sync_sg_for_device DMA operation")

v4.14.198: Failed to apply! Possible dependencies:
1c30844d2dfe ("mm: reclaim small amounts of memory when an external fragmentation event occurs")
1d47a3ec09b5 ("mm/cma: remove ALLOC_CMA")
24512228b7a3 ("mm: do not boost watermarks to avoid fragmentation for the DISCONTIG memory model")
3d2054ad8c2d ("ARM: CMA: avoid double mapping to the CMA area if CONFIG_HIGHMEM=y")
453f85d43fa9 ("mm: remove __GFP_COLD")
6bb154504f8b ("mm, page_alloc: spread allocations across zones before introducing fragmentation")
85ccc8fa81af ("mm/page_alloc: make sure __rmqueue() etc are always inline")
a921444382b4 ("mm: move zone watermark accesses behind an accessor")
bad8c6c0b114 ("mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE")

v4.9.236: Failed to apply! Possible dependencies:
14b468791fa9 ("mm: workingset: move shadow entry tracking to radix tree exceptional tracking")
1c30844d2dfe ("mm: reclaim small amounts of memory when an external fragmentation event occurs")
24512228b7a3 ("mm: do not boost watermarks to avoid fragmentation for the DISCONTIG memory model")
2a2e48854d70 ("mm: vmscan: fix IO/refault regression in cache workingset transition")
31176c781508 ("mm: memcontrol: clean up memory.events counting function")
6bb154504f8b ("mm, page_alloc: spread allocations across zones before introducing fragmentation")
8e675f7af507 ("mm/oom_kill: count global and memory cgroup oom kills")
9d998b4f1e39 ("mm, vmscan: add active list aging tracepoint")
a921444382b4 ("mm: move zone watermark accesses behind an accessor")
cd04ae1e2dc8 ("mm, oom: do not rely on TIF_MEMDIE for memory reserves access")
d6622f6365db ("mm/vmscan: more restrictive condition for retry in do_try_to_free_pages")
dcec0b60a821 ("mm, vmscan: add mm_vmscan_inactive_list_is_low tracepoint")
df0e53d0619e ("mm: memcontrol: re-use global VM event enum")
f7942430e40f ("lib: radix-tree: native accounting of exceptional entries")

v4.4.236: Failed to apply! Possible dependencies:
0b57d6ba0bd1 ("mm/mmap.c: remove redundant local variables for may_expand_vm()")
1170532bb49f ("mm: convert printk(KERN_<LEVEL> to pr_<level>")
5a6e75f8110c ("shmem: prepare huge= mount option and sysfs knob")
756a025f0009 ("mm: coalesce split strings")
84638335900f ("mm: rework virtual memory accounting")
8cee852ec53f ("mm, procfs: breakdown RSS for anon, shmem and file in /proc/pid/status")
b46e756f5e47 ("thp: extract khugepaged from mm/huge_memory.c")
d07e22597d1d ("mm: mmap: add new /proc tunable for mmap_base ASLR")
d977d56ce5b3 ("mm: warn about VmData over RLIMIT_DATA")
d9fe4fab1197 ("x86/mm/pat: Add untrack_pfn_moved for mremap")
eca56ff906bd ("mm, shmem: add internal shmem resident memory accounting")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks
Sasha

2020-09-21 12:56:58

by Sasha Levin

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: f000565adb77 ("thp: set recommended min free kbytes").

The bot has tested the following trees: v5.8.10, v5.4.66, v4.19.146, v4.14.198, v4.9.236, v4.4.236.

v5.8.10: Build OK!
v5.4.66: Build OK!
v4.19.146: Failed to apply! Possible dependencies:
013de2d6671d ("csky: MMU and page table management")
27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")
426dcd4b600f ("hexagon: switch to NO_BOOTMEM")
6471f52af786 ("alpha: switch to NO_BOOTMEM")
9143a9359d05 ("csky: Kernel booting")
aca52c398389 ("mm: remove CONFIG_HAVE_MEMBLOCK")
bc3ec75de545 ("dma-mapping: merge direct and noncoherent ops")
c32e64e852f3 ("csky: Build infrastructure")
ca9a46f8a4f0 ("mm/memory_hotplug: online_pages cannot be 0 in online_pages()")
de1193f0be66 ("mm, memory_hotplug: update pcp lists everytime onlining a memory block")
e0a9317d9004 ("hexagon: use generic dma_noncoherent_ops")
e900a918b098 ("mm: shuffle initial free memory to improve memory-side-cache utilization")
f3ecc0ff0457 ("dma-mapping: move the dma_coherent flag to struct device")
f406f222d4b2 ("hexagon: implement the sync_sg_for_device DMA operation")

v4.14.198: Failed to apply! Possible dependencies:
008ef0969dd9 ("btrfs: drop lock parameter from update_ioctl_balance_args and rename")
17ef445f9bef ("Documentation/filesystems: update documentation of file_operations")
27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")
3d8f7615319b ("vfs: implement readahead(2) using POSIX_FADV_WILLNEED")
45cd0faae371 ("vfs: add the fadvise() file operation")
5740c99e9d30 ("vfs: dedupe: return int")
6e8b704df584 ("fs: update documentation to mention __poll_t and match the code")
87eb5eb24232 ("vfs: dedupe: rationalize args")
b4e98d9ac775 ("mm: account pud page tables")
c4812909f5d5 ("mm: introduce wrappers to access mm->nr_ptes")
ca9a46f8a4f0 ("mm/memory_hotplug: online_pages cannot be 0 in online_pages()")
de1193f0be66 ("mm, memory_hotplug: update pcp lists everytime onlining a memory block")
e900a918b098 ("mm: shuffle initial free memory to improve memory-side-cache utilization")
f51d2b59120f ("btrfs: allow to set compression level for zlib")
f5c29bd9dbd3 ("Btrfs: add __init macro to btrfs init functions")

v4.9.236: Failed to apply! Possible dependencies:
0b89ede62963 ("s390/mm: fork vs. 5 level page tabel")
27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")
505a60e22560 ("asm-generic: introduce 5level-fixup.h")
a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent hugepages")
b4e98d9ac775 ("mm: account pud page tables")
c2febafc6773 ("mm: convert generic code to 5-level paging")
c4812909f5d5 ("mm: introduce wrappers to access mm->nr_ptes")
c763ea2650df ("x86/kconfig: Sort the 'config X86' selects alphabetically")
d94e068573f2 ("x86/kconfig: Move 64-bit only arch Kconfig selects to 'config X86_64'")

v4.4.236: Failed to apply! Possible dependencies:
0b57d6ba0bd1 ("mm/mmap.c: remove redundant local variables for may_expand_vm()")
1170532bb49f ("mm: convert printk(KERN_<LEVEL> to pr_<level>")
27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")
5a6e75f8110c ("shmem: prepare huge= mount option and sysfs knob")
756a025f0009 ("mm: coalesce split strings")
84638335900f ("mm: rework virtual memory accounting")
8cee852ec53f ("mm, procfs: breakdown RSS for anon, shmem and file in /proc/pid/status")
b46e756f5e47 ("thp: extract khugepaged from mm/huge_memory.c")
d07e22597d1d ("mm: mmap: add new /proc tunable for mmap_base ASLR")
d977d56ce5b3 ("mm: warn about VmData over RLIMIT_DATA")
d9fe4fab1197 ("x86/mm/pat: Add untrack_pfn_moved for mremap")
eca56ff906bd ("mm, shmem: add internal shmem resident memory accounting")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks
Sasha

2020-09-23 21:29:18

by Vijay Balakrishna

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

Can this patch be included? As Kirill is ok with patch now.

Thanks,
Vijay

On 9/16/2020 6:21 PM, Vijay Balakrishna wrote:
> When memory is hotplug added or removed the min_free_kbytes must be
> recalculated based on what is expected by khugepaged. Currently
> after hotplug, min_free_kbytes will be set to a lower default and higher
> default set when THP enabled is lost. This change restores min_free_kbytes
> as expected for THP consumers.
>
> Fixes: f000565adb77 ("thp: set recommended min free kbytes")
>
> Signed-off-by: Vijay Balakrishna <[email protected]>
> Cc: [email protected]
> Reviewed-by: Pavel Tatashin <[email protected]>
> ---
> include/linux/khugepaged.h | 5 +++++
> mm/khugepaged.c | 13 +++++++++++--
> mm/memory_hotplug.c | 3 +++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/khugepaged.h b/include/linux/khugepaged.h
> index bc45ea1efbf7..c941b7377321 100644
> --- a/include/linux/khugepaged.h
> +++ b/include/linux/khugepaged.h
> @@ -15,6 +15,7 @@ extern int __khugepaged_enter(struct mm_struct *mm);
> extern void __khugepaged_exit(struct mm_struct *mm);
> extern int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
> unsigned long vm_flags);
> +extern void khugepaged_min_free_kbytes_update(void);
> #ifdef CONFIG_SHMEM
> extern void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr);
> #else
> @@ -85,6 +86,10 @@ static inline void collapse_pte_mapped_thp(struct mm_struct *mm,
> unsigned long addr)
> {
> }
> +
> +static inline void khugepaged_min_free_kbytes_update(void)
> +{
> +}
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> #endif /* _LINUX_KHUGEPAGED_H */
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index cfa0dba5fd3b..4f7107476a6f 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -56,6 +56,9 @@ enum scan_result {
> #define CREATE_TRACE_POINTS
> #include <trace/events/huge_memory.h>
>
> +static struct task_struct *khugepaged_thread __read_mostly;
> +static DEFINE_MUTEX(khugepaged_mutex);
> +
> /* default scan 8*512 pte (or vmas) every 30 second */
> static unsigned int khugepaged_pages_to_scan __read_mostly;
> static unsigned int khugepaged_pages_collapsed;
> @@ -2292,8 +2295,6 @@ static void set_recommended_min_free_kbytes(void)
>
> int start_stop_khugepaged(void)
> {
> - static struct task_struct *khugepaged_thread __read_mostly;
> - static DEFINE_MUTEX(khugepaged_mutex);
> int err = 0;
>
> mutex_lock(&khugepaged_mutex);
> @@ -2320,3 +2321,11 @@ int start_stop_khugepaged(void)
> mutex_unlock(&khugepaged_mutex);
> return err;
> }
> +
> +void khugepaged_min_free_kbytes_update(void)
> +{
> + mutex_lock(&khugepaged_mutex);
> + if (khugepaged_enabled() && khugepaged_thread)
> + set_recommended_min_free_kbytes();
> + mutex_unlock(&khugepaged_mutex);
> +}
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..3e19272c1fad 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -36,6 +36,7 @@
> #include <linux/memblock.h>
> #include <linux/compaction.h>
> #include <linux/rmap.h>
> +#include <linux/khugepaged.h>
>
> #include <asm/tlbflush.h>
>
> @@ -857,6 +858,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
> zone_pcp_update(zone);
>
> init_per_zone_wmark_min();
> + khugepaged_min_free_kbytes_update();
>
> kswapd_run(nid);
> kcompactd_run(nid);
> @@ -1600,6 +1602,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
> pgdat_resize_unlock(zone->zone_pgdat, &flags);
>
> init_per_zone_wmark_min();
> + khugepaged_min_free_kbytes_update();
>
> if (!populated_zone(zone)) {
> zone_pcp_reset(zone);
>

2020-09-25 02:53:05

by Andrew Morton

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

On Wed, 23 Sep 2020 14:27:30 -0700 Vijay Balakrishna <[email protected]> wrote:

> Can this patch be included? As Kirill is ok with patch now.

He is? I can't immediately find that email.

Do we have an acked-by?

2020-09-25 07:46:25

by Michal Hocko

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

On Wed 16-09-20 18:21:48, Vijay Balakrishna wrote:
> When memory is hotplug added or removed the min_free_kbytes must be
> recalculated based on what is expected by khugepaged. Currently
> after hotplug, min_free_kbytes will be set to a lower default and higher
> default set when THP enabled is lost. This change restores min_free_kbytes
> as expected for THP consumers.
>
> Fixes: f000565adb77 ("thp: set recommended min free kbytes")
>
> Signed-off-by: Vijay Balakrishna <[email protected]>
> Cc: [email protected]
> Reviewed-by: Pavel Tatashin <[email protected]>

I am ok with this patch. I am not sure this is worth backporting to
stable trees becasuse this is not a functional bug. Surprising behavior,
yes, but not much more than that.

Acked-by: Michal Hocko <[email protected]>

One minor comment below
[...]
> @@ -857,6 +858,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
> zone_pcp_update(zone);
>
> init_per_zone_wmark_min();
> + khugepaged_min_free_kbytes_update();
>
> kswapd_run(nid);
> kcompactd_run(nid);
> @@ -1600,6 +1602,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
> pgdat_resize_unlock(zone->zone_pgdat, &flags);
>
> init_per_zone_wmark_min();
> + khugepaged_min_free_kbytes_update();
>
> if (!populated_zone(zone)) {
> zone_pcp_reset(zone);

Can we move khugepaged_min_free_kbytes_update into
init_per_zone_wmark_min? If it stays external we might hit the same
problem when somebody else needs to modify min_free_kbytes. Early init
call will be likely too early for khugepaged but that shouldn't matter
AFAICS because it will call khugepaged_min_free_kbytes_update on its
own.
--
Michal Hocko
SUSE Labs

2020-09-25 07:49:22

by Michal Hocko

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged

On Thu 24-09-20 19:51:03, Andrew Morton wrote:
> On Wed, 23 Sep 2020 14:27:30 -0700 Vijay Balakrishna <[email protected]> wrote:
>
> > Can this patch be included? As Kirill is ok with patch now.
>
> He is? I can't immediately find that email.

http://lkml.kernel.org/r/[email protected]
--
Michal Hocko
SUSE Labs

2020-09-25 16:34:21

by Vijay Balakrishna

[permalink] [raw]
Subject: Re: [v3 1/2] mm: khugepaged: recalculate min_free_kbytes after memory hotplug as expected by khugepaged



On 9/25/2020 12:42 AM, Michal Hocko wrote:
> On Wed 16-09-20 18:21:48, Vijay Balakrishna wrote:
>> When memory is hotplug added or removed the min_free_kbytes must be
>> recalculated based on what is expected by khugepaged. Currently
>> after hotplug, min_free_kbytes will be set to a lower default and higher
>> default set when THP enabled is lost. This change restores min_free_kbytes
>> as expected for THP consumers.
>>
>> Fixes: f000565adb77 ("thp: set recommended min free kbytes")
>>
>> Signed-off-by: Vijay Balakrishna <[email protected]>
>> Cc: [email protected]
>> Reviewed-by: Pavel Tatashin <[email protected]>
>
> I am ok with this patch. I am not sure this is worth backporting to
> stable trees becasuse this is not a functional bug. Surprising behavior,
> yes, but not much more than that.
>
> Acked-by: Michal Hocko <[email protected]>

Thanks Michal.

>
> One minor comment below
> [...]
>> @@ -857,6 +858,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
>> zone_pcp_update(zone);
>>
>> init_per_zone_wmark_min();
>> + khugepaged_min_free_kbytes_update();
>>
>> kswapd_run(nid);
>> kcompactd_run(nid);
>> @@ -1600,6 +1602,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
>> pgdat_resize_unlock(zone->zone_pgdat, &flags);
>>
>> init_per_zone_wmark_min();
>> + khugepaged_min_free_kbytes_update();
>>
>> if (!populated_zone(zone)) {
>> zone_pcp_reset(zone);
>
> Can we move khugepaged_min_free_kbytes_update into
> init_per_zone_wmark_min? If it stays external we might hit the same
> problem when somebody else needs to modify min_free_kbytes. Early init
> call will be likely too early for khugepaged but that shouldn't matter
> AFAICS because it will call khugepaged_min_free_kbytes_update on its
> own.

Sure, let me take a look and post v4 next week.

Thanks,
Vijay

>