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

[Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>]

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.