linux-kernel - Re: write_lock_irq(&tasklist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 May 2018 23:17:24 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     psodagud@...eaurora.org, Kees Cook <keescook@...omium.org>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Rik van Riel <riel@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Eric Biggers <ebiggers@...gle.com>,
        Frederic Weisbecker <fweisbec@...il.com>, sherryy@...roid.com,
        Vegard Nossum <vegard.nossum@...cle.com>,
        Christoph Lameter <cl@...ux.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Sasha Levin <alexander.levin@...izon.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: write_lock_irq(&tasklist_lock)

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).