Re: [PATCH] mm: might_sleep warning

[Pavel Tatashin <pasha.tatashin@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

[CCed everyone]

Hi Andrew,

I afraid we cannot change this spinlock to mutex
because deferred_grow_zone() might be called from an interrupt context if
interrupt thread needs to allocate memory.

Thank you,
Pavel


On Tue, Mar 6, 2018 at 3:36 PM, Andrew Morton <akpm@linux-foundation.org>
wrote:

> On Tue,  6 Mar 2018 14:20:22 -0500 Pavel Tatashin <
> pasha.tatashin@oracle.com> wrote:
>
> > Robot reported this issue:
> > https://lkml.org/lkml/2018/2/27/851
> >
> > That is introduced by:
> > mm: initialize pages on demand during boot
> >
> > The problem is caused by changing static branch value within spin lock.
> > Spin lock disables preemption, and changing static branch value takes
> > mutex lock in its path, and thus may sleep.
> >
> > The fix is to add another boolean variable to avoid the need to change
> > static branch within spinlock.
> >
> > ...
> >
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1579,6 +1579,7 @@ static int __init deferred_init_memmap(void *data)
> >   * page_alloc_init_late() soon after smp_init() is complete.
> >   */
> >  static __initdata DEFINE_SPINLOCK(deferred_zone_grow_lock);
> > +static bool deferred_zone_grow __initdata = true;
> >  static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> >
> >  /*
> > @@ -1616,7 +1617,7 @@ deferred_grow_zone(struct zone *zone, unsigned int
> order)
> >        * Bail if we raced with another thread that disabled on demand
> >        * initialization.
> >        */
> > -     if (!static_branch_unlikely(&deferred_pages)) {
> > +     if (!static_branch_unlikely(&deferred_pages) ||
> !deferred_zone_grow) {
> >               spin_unlock_irqrestore(&deferred_zone_grow_lock, flags);
> >               return false;
> >       }
> > @@ -1683,10 +1684,15 @@ void __init page_alloc_init_late(void)
> >       /*
> >        * We are about to initialize the rest of deferred pages,
> permanently
> >        * disable on-demand struct page initialization.
> > +      *
> > +      * Note: it is prohibited to modify static branches in
> non-preemptible
> > +      * context. Since, spin_lock() disables preemption, we must use an
> > +      * extra boolean deferred_zone_grow.
> >        */
> >       spin_lock(&deferred_zone_grow_lock);
> > -     static_branch_disable(&deferred_pages);
> > +     deferred_zone_grow = false;
> >       spin_unlock(&deferred_zone_grow_lock);
> > +     static_branch_disable(&deferred_pages);
> >
> >       /* There will be num_node_state(N_MEMORY) threads */
> >       atomic_set(&pgdat_init_n_undone, num_node_state(N_MEMORY));
>
> Kinda ugly, but I can see the logic behind the decisions.
>
> Can we instead turn deferred_zone_grow_lock into a mutex?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

[-- Attachment #2: Type: text/html, Size: 7336 bytes --]