Re: [PATCH v2] mm: workingset: replace IRQ-off check with a lockdep assert.

[Matthew Wilcox <willy@infradead.org>]

On Mon, Feb 11, 2019 at 08:13:45PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-02-11 13:53:18 [-0500], Johannes Weiner wrote:
> > I'm not against checking for the lock, but if IRQs aren't disabled,
> > what ensures __mod_lruvec_state() is safe?
> 
> how do you define safe? I've been looking for dependencies of
> __mod_lruvec_state() but found only that the lock is held during the RMW
> operation with WORKINGSET_NODES idx.
> 
> >                                            I'm guessing it's because
> > preemption is disabled and irq handlers are punted to process context.
> preemption is enabled and IRQ are processed in forced-threaded mode.
> 
> > That said, it seems weird to me that
> > 
> > 	spin_lock_irqsave();
> > 	BUG_ON(!irqs_disabled());
> > 	spin_unlock_irqrestore();
> > 
> > would trigger. Wouldn't it make sense to have a raw_irqs_disabled() or
> > something and keep the irqs_disabled() abstraction layer intact?
> 
> maybe if I know why interrupts should be disabled in the first place.
> The ->i_pages lock is never acquired with disabled interrupts so it
> should be safe to proceed as-is. Should there be a spot in -RT where the
> lock is acquired with disabled interrupts then lockdep would scream. And
> then we would have to decide to either move everything raw_ locks (and
> live with the consequences) or avoid acquiring the lock with disabled
> interrupts.

I think you mean 'the i_pages lock is never acquired with interrupts
enabled".  Lockdep would scream if it were -- you'd be in a situation
where an interrupt handler which acquired the i_pages lock could deadlock
against you.