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(¤t->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.
