linux-kernel - Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20180222054044.GN30522@ZenIV.linux.org.uk>
Date:   Thu, 22 Feb 2018 05:40:44 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     John Ogness <john.ogness@...utronix.de>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry_kill()

On Fri, Feb 16, 2018 at 10:03:15AM -0800, Linus Torvalds wrote:
> On Fri, Feb 16, 2018 at 7:09 AM, John Ogness <john.ogness@...utronix.de> wrote:
> > dentry_kill() holds dentry->d_lock and needs to acquire both
> > dentry->d_inode->i_lock and dentry->d_parent->d_lock. This cannot be
> > done with spin_lock() operations because it's the reverse of the
> > regular lock order. To avoid ABBA deadlocks it is done with two
> > trylock loops.
> >
> > Trylock loops are problematic in two scenarios:
> 
> I don't mind this patch series per se (although I would really like Al
> to ack it), but this particular patch I hate.

I'm not happy about the previous patch either.  Why do we need the users
of that thing to deal with retries?  And I'm not even sure we want to
bother with retries on inode change inside dentry_kill() itself - just
unlock, return dentry and let the caller handle that.  Callers *must*
handle "need to drop another one" anyway, so...