On 2023/06/21 22:06, Sebastian Andrzej Siewior wrote:
> On 2023-06-21 20:33:35 [+0900], Tetsuo Handa wrote:
>> On 2023/06/21 19:40, Sebastian Andrzej Siewior wrote:
>>> printk_deferred_enter() has to be invoked in non-migrate-able context to
>>> ensure that deferred printing is enabled and disabled on the same CPU.
>>
>> I can't catch. local_irq_save(flags); makes non-migrate-able context
>> because sleeping is not allowed while IRQ is disabled, doesn't it?
>
> That is correct. The problem is the local_irq_save() which remains on
> PREEMPT_RT and this is problematic during write_seqlock() which acquires
> spinlock_t which becomes a sleeping lock. write_seqlock_irqsave()
> substitutes everything properly so on RT there is no local_irq_save().
include/linux/seqlock.h says
static inline unsigned long __write_seqlock_irqsave(seqlock_t *sl)
{
unsigned long flags;
spin_lock_irqsave(&sl->lock, flags);
do_write_seqcount_begin(&sl->seqcount.seqcount);
return flags;
}
/**
* write_seqlock_irqsave() - start a non-interruptible seqlock_t write
* section
* @lock: Pointer to seqlock_t
* @flags: Stack-allocated storage for saving caller's local interrupt
* state, to be passed to write_sequnlock_irqrestore().
*
* _irqsave variant of write_seqlock(). Use it only if the read side
* section, or other write sections, can be invoked from hardirq context.
*/
#define write_seqlock_irqsave(lock, flags) \
do { flags = __write_seqlock_irqsave(lock); } while (0)
/**
* write_seqlock() - start a seqlock_t write side critical section
* @sl: Pointer to seqlock_t
*
* write_seqlock opens a write side critical section for the given
* seqlock_t. It also implicitly acquires the spinlock_t embedded inside
* that sequential lock. All seqlock_t write side sections are thus
* automatically serialized and non-preemptible.
*
* Context: if the seqlock_t read section, or other write side critical
* sections, can be invoked from hardirq or softirq contexts, use the
* _irqsave or _bh variants of this function instead.
*/
static inline void write_seqlock(seqlock_t *sl)
{
spin_lock(&sl->lock);
do_write_seqcount_begin(&sl->seqcount.seqcount);
}
and regarding include/linux/spinlock.h , since spin_lock_irqsave(lock, flags) is
equivalent with local_irq_save(flags) + spin_lock(lock), there is no difference
between
local_irq_save(flags);
printk_deferred_enter();
write_seqlock(&zonelist_update_seq);
and
write_seqlock_irqsave(&zonelist_update_seq, flags);
printk_deferred_enter();
(except potential lockdep warning).
However, regarding include/linux/spinlock_rt.h , since spin_lock_irqsave(lock, flags)
is equivalent with spin_lock(lock), there is difference between
local_irq_save(flags);
printk_deferred_enter();
write_seqlock(&zonelist_update_seq);
and
write_seqlock_irqsave(&zonelist_update_seq, flags);
printk_deferred_enter();
.
Is above understanding correct?
And you are trying to replace
local_irq_save(flags);
printk_deferred_enter();
write_seqlock(&zonelist_update_seq);
with
write_seqlock_irqsave(&zonelist_update_seq, flags);
printk_deferred_enter();
, aren't you?
But include/linux/printk.h says
/*
* The printk_deferred_enter/exit macros are available only as a hack for
* some code paths that need to defer all printk console printing. Interrupts
* must be disabled for the deferred duration.
*/
#define printk_deferred_enter __printk_safe_enter
#define printk_deferred_exit __printk_safe_exit
which means that local_irq_save() is _required_ before printk_deferred_enter().
If local_irq_save() is hidden by your patch, what guarantees that
printk_deferred_enter() and printk_deferred_exit() run on the same CPU?
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?
Disabling IRQ before incrementing zonelist_update_seq is _required_ for both
making printk_deferred_enter() safe
and
making sure that printk_deferred_enter() takes effect
.
