Re: [PATCH] mm/page_alloc: fix potential deadlock on zonelist_update_seq seqlock

Tetsuo Handa
2023-04-04T08:20:23Z

On 2023/04/04 16:54, Michal Hocko wrote:
>> + /*
>> + * Since __alloc_pages_slowpath() spins if zonelist_update_seq.seqcount
>> + * is odd, any memory allocation while zonelist_update_seq.seqcount is
>> + * odd have to be avoided.
>> + *
>> + * Explicitly disable local irqs in order to avoid calling
>> + * kmalloc(GFP_ATOMIC) from e.g. timer interrupt handler.
>> + * Also, explicitly prevent printk() from synchronously waiting for
>> + * port->lock because tty_insert_flip_string_and_push_buffer() might
>> + * call kmalloc(GFP_ATOMIC | __GFP_NOWARN) while holding port->lock.
>
> This explanation of local_irq_save just doesn't make any sense. You do
> not prevent any other cpu from entering the IRQ and doing the same
> thing.

There is no need to prevent other CPUs from doing the same thing.
The intent of local_irq_save() here is to avoid below sequence.

CPU0
----
__build_all_zonelists() {
write_seqlock(&zonelist_update_seq); // makes zonelist_update_seq.seqcount odd
// e.g. timer interrupt handler runs at this moment
some_timer_func() {
kmalloc(GFP_ATOMIC) {
__alloc_pages_slowpath() {
read_seqbegin(&zonelist_update_seq) {
// forever spins because zonelist_update_seq.seqcount is odd
}
}
}
}
// e.g. timer interrupt handler finishes
write_sequnlock(&zonelist_update_seq); // makes zonelist_update_seq.seqcount even
}

> If the sole purpose of local_irq_save is to conform
> printk_deferred_enter then state that instead.

As described above, this local_irq_save() is not intended for conforming
printk_deferred_enter(). This is the same with set_mems_allowed() calling
local_irq_save() before write_seqcount_begin(&current->mems_allowed_seq).

> Although it seems that
> Petr believes that preempt_disable should be sufficient and then it
> would be preferred as well. This would require update to the comment for
> printk_deferred_enter though.

This patch is supposed to be backported to stable kernels where commit
3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists
and page allocation") was backported.