[CCed everyone]
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@xxxxxxxxxxxxxxxxxxxx> wrote:
On Tue, 6 Mar 2018 14:20:22 -0500 Pavel Tatashin <pasha.tatashin@xxxxxxxxxx> 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.
>
> ...
Kinda ugly, but I can see the logic behind the decisions.>
> --- 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));
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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href="" href="mailto:dont@xxxxxxxxx">dont@xxxxxxxxx"> email@xxxxxxxxx </a>
