On Fri, Mar 27, 2015 at 02:47:08PM -0700, Andrew Morton wrote:
> On Fri, 27 Mar 2015 13:39:38 +0200 "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
>
> > set_recommended_min_free_kbytes() adjusts zone water marks to be suitable
> > for khugepaged. We avoid doing this if khugepaged is disabled, but don't
> > catch the case when khugepaged is failed to start.
> >
> > Let's address this by checking khugepaged_thread instead of
> > khugepaged_enabled() in set_recommended_min_free_kbytes().
> > It's NULL if the kernel thread is stopped or failed to start.
> >
>
> hm, why didn't khugepaged start up? Is this a theoretical
> by-code-inspection thing or has the problem been observed in real life?
David mentioned this scenario in comment to my previous patch.
>
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -110,7 +110,8 @@ static int set_recommended_min_free_kbytes(void)
> > int nr_zones = 0;
> > unsigned long recommended_min;
> >
> > - if (!khugepaged_enabled())
> > + /* khugepaged thread has stopped to failed to start */
> > + if (!khugepaged_thread)
> > return 0;
> >
> > for_each_populated_zone(zone)
>
> Fair enough, but take a look at start_khugepaged():
>
> : static int start_khugepaged(void)
> : {
> : int err = 0;
> : if (khugepaged_enabled()) {
> : if (!khugepaged_thread)
> : khugepaged_thread = kthread_run(khugepaged, NULL,
> : "khugepaged");
> : if (unlikely(IS_ERR(khugepaged_thread))) {
> : pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> : err = PTR_ERR(khugepaged_thread);
> : khugepaged_thread = NULL;
>
> -->> stop here
Right, but set_recommended_min_free_kbytes() is also registered to
late_initcall() and will get called anyway.
It's not obvious why would we need it registered there. Call from
start_khugepaged() should be enough.
Andrea?
> : }
> :
> : if (!list_empty(&khugepaged_scan.mm_head))
> : wake_up_interruptible(&khugepaged_wait);
> :
> : set_recommended_min_free_kbytes();
> : } else if (khugepaged_thread) {
> : kthread_stop(khugepaged_thread);
> : khugepaged_thread = NULL;
> : }
> :
> : return err;
> : }
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>