Re: write_lock_irq(&tasklist_lock)

[Peter Zijlstra <peterz@infradead.org>]

On Tue, May 22, 2018 at 01:27:17PM -0700, Linus Torvalds wrote:

> It's usually not a huge problem because there are so few writers, but what
> you're seeing is the writer starvation issue because readers can be
> plentiful.

qrwlock is a fair lock and should not exhibit writer starvation. Write
acquire time is of course proportional to the number of CPUs in the
system because each can be holding a reader, but once there is a pending
writer there should not be new readers.

> > Do we need write_lock_irq(&tasklist_lock) in below portion of code ? Can
> > I use write_unlock instead of write_lock_irq in portion of code?
> 
> You absolutely need write_lock_irq(), because taking the tasklist_lock
> without disabling interrupts will deadlock very quickly due to an interrupt
> taking the tasklist_lock for reading.
> 
> That said, the write_lock_irq(&tasklist_lock) could possibly be replaced
> with something like
> 
>    static void tasklist_write_lock(void)
>    {
>          unsigned long flags;
>          local_irq_save(flags);
>          while (!write_trylock(&tasklist_lock)) {
>                  local_irq_restore(flags);
>                  do { cpu_relax(); } while (write_islocked(&tasklist_lock));
>                  local_irq_disable();
>          }
>    }
> 
> but we don't have that "write_islocked()" function.

You basically want to spin-wait with interrupts enabled, right? I think
there were problems with that, but I can't remember, my brain is
entirely shot for the day. But given how qrwlock is constructed, you'd
have to look at the arch spinlock implementation for that (qspinlock for
x86).

The obvious problem of course is:


	spin_lock_irq(&L)
	// contended
	local_irq_enable();
	while(...)
	  cpu_relax();
	   <IRQ>
	     spin_lock(&L);

Which because the spinlock is fair, is an instant deadlock.


You can do the IRQ enabled spin-wait for unfair locks, but by now all
our locks are fair (because we kept hitting starvation).