Re: + mm-khugepaged-recalculate-min_free_kbytes-after-memory-hotplug-as-expected-by-khugepaged.patch added to -mm tree

[Vijay Balakrishna <vijayb@xxxxxxxxxxxxxxxxxxx>]

On 9/16/2020 12:33 AM, Michal Hocko wrote:
> On Tue 15-09-20 17:09:48, Andrew Morton wrote:
> > From: Vijay Balakrishna <vijayb@xxxxxxxxxxxxxxxxxxx>
> > Subject: 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 leaves the system with small
> > min_free_kbytes which isn't suitable for systems especially with network
> > intensive loads.  Typical failure symptoms include HW WATCHDOG reset, soft
> > lockup hang notices, NETDEVICE WATCHDOG timeouts, and OOM process kills.
> >
> > Link: https://lkml.kernel.org/r/1600204258-13683-1-git-send-email-vijayb@xxxxxxxxxxxxxxxxxxx
> > Fixes: f000565adb77 ("thp: set recommended min free kbytes")
> > Signed-off-by: Vijay Balakrishna <vijayb@xxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
> > Cc: Allen Pais <apais@xxxxxxxxxxxxx>
> > Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> > Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> > Cc: Michal Hocko <mhocko@xxxxxxxx>
> > Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
> > Cc: Song Liu <songliubraving@xxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>
> The patch has been explicitly nacked by Kirill IIRC. I am also not happy
> about it because the changelog doesn't really explain the problem and
> the follow up discussion didn't drill down to the underlying problem
> either.
>
> Maybe we want to make the min_free_kbytes udpate consistent with the
> boot but the current changelog is incomplete and this shouldn't have
> been added yet.

Let me modify changelog to remove references symptoms mentioned.

Thanks,
Vijay

> > ---
> >
> >   include/linux/khugepaged.h |    5 +++++
> >   mm/khugepaged.c            |   13 +++++++++++--
> >   mm/memory_hotplug.c        |    3 +++
> >   3 files changed, 19 insertions(+), 2 deletions(-)
> >
> > --- a/include/linux/khugepaged.h~mm-khugepaged-recalculate-min_free_kbytes-after-memory-hotplug-as-expected-by-khugepaged
> > +++ a/include/linux/khugepaged.h
> > @@ -15,6 +15,7 @@ extern int __khugepaged_enter(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_t
> >   					   unsigned long addr)
> >   {
> >   }
> > +
> > +static inline void khugepaged_min_free_kbytes_update(void)
> > +{
> > +}
> >   #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> >   
> >   #endif /* _LINUX_KHUGEPAGED_H */
> > --- a/mm/khugepaged.c~mm-khugepaged-recalculate-min_free_kbytes-after-memory-hotplug-as-expected-by-khugepaged
> > +++ a/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_kby
> >   
> >   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 @@ fail:
> >   	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);
> > +}
> > --- a/mm/memory_hotplug.c~mm-khugepaged-recalculate-min_free_kbytes-after-memory-hotplug-as-expected-by-khugepaged
> > +++ a/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
> >   	zone_pcp_update(zone);
> >   
> >   	init_per_zone_wmark_min();
> > +	khugepaged_min_free_kbytes_update();
> >   
> >   	kswapd_run(nid);
> >   	kcompactd_run(nid);
> > @@ -1614,6 +1616,7 @@ static int __ref __offline_pages(unsigne
> >   	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);
> > _
> >
> > Patches currently in -mm which might be from vijayb@xxxxxxxxxxxxxxxxxxx are
> >
> > mm-khugepaged-recalculate-min_free_kbytes-after-memory-hotplug-as-expected-by-khugepaged.patch
> > mm-khugepaged-avoid-overriding-min_free_kbytes-set-by-user.patch