Re: [PATCH] mm/page_alloc: Use write_seqlock_irqsave() instead write_seqlock() + local_irq_save().

[Michal Hocko <mhocko@suse.com>]

On Thu 22-06-23 08:24:30, Tetsuo Handa wrote:
> On 2023/06/21 23:50, Tetsuo Handa wrote:
> > On 2023/06/21 23:34, Sebastian Andrzej Siewior wrote:
> >>> Also, if local_irq_save() is hidden due to RT, what guarantees that
> >>>
> >>>   write_seqlock_irqsave(&zonelist_update_seq, flags);
> >>>   <<IRQ>>
> >>>     some_timer_function() {
> >>>       printk();
> >>>     }
> >>>   <<IRQ>>
> >>>   printk_deferred_enter();
> >>>
> >>> does not happen because write_seqlock_irqsave() does not disable IRQ?
> >>
> >> I don't see how zonelist_update_seq and printk here are connected
> >> without the port lock/ or memory allocation. But there are two things
> >> that are different on RT which probably answer your question:
> > 
> > It is explained as the first deadlock scenario in commit 1007843a9190
> > ("mm/page_alloc: fix potential deadlock on zonelist_update_seq seqlock").
> > We have to disable IRQ before making zonelist_update_seq.seqcount odd.
> > 
> 
> Since we must replace local_irq_save() + write_seqlock() with write_seqlock_irqsave() for
> CONFIG_PREEMPT_RT=y case but we must not replace local_irq_save() + write_seqlock() with
> write_seqlock_irqsave() for CONFIG_PREEMPT_RT=n case, the proper fix is something like below?

Now, I am confused. Why write_seqlock_irqsave is not allowed for !RT?
Let me quote the changelog and he scenario 1:
        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) {
                  // spins forever because zonelist_update_seq.seqcount is odd
                }
              }
            }
          }
        // e.g. timer interrupt handler finishes
        write_sequnlock(&zonelist_update_seq); // makes zonelist_update_seq.seqcount even

This is clearly impossible with write_seqlock_irqsave as interrupts are
disabled before the lock is taken.
-- 
Michal Hocko
SUSE Labs