linux-kernel - Re: fs/dcache.c - BUG: soft lockup - CPU#5 stuck for 22s! [systemd-udevd:1667]

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]

Message-ID: <CA+55aFyHCF1+DfxndzxkGLLoNErRF2MA0YWtdx7UOTJ=gQWhEw@mail.gmail.com>
Date:	Wed, 28 May 2014 21:21:00 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Miklos Szeredi <mszeredi@...e.cz>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: fs/dcache.c - BUG: soft lockup - CPU#5 stuck for 22s! [systemd-udevd:1667]

On Wed, May 28, 2014 at 8:11 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
>
> Here's an updated patch, hopefully slightly less vomit-inducing.

Hmm. Less vomit-inducing, except for this part:

>                 dentry = list_entry(list->prev, struct dentry, d_lru);
> +
> +               parent = NULL;
>                 spin_lock(&dentry->d_lock);
> +               if (!IS_ROOT(dentry)) {
> +                       parent = dentry->d_parent;
> +                       if (unlikely(!spin_trylock(&parent->d_lock))) {
> +                               spin_unlock(&dentry->d_lock);
> +                               parent = NULL;
> +                               read_seqlock_excl(&rename_lock);
> +                               if (!IS_ROOT(dentry)) {
> +                                       parent = dentry->d_parent;
> +                                       spin_lock(&parent->d_lock);
> +                               }
> +                               read_sequnlock_excl(&rename_lock);
> +                               spin_lock(&dentry->d_lock);
> +                       }
> +               }

Ugh, that just *screams* for a helper function. Something like

        parent = get_parent_and_lock(dentry);

or whatever, with that trylock/renamelock dance separated out. The
rule would be that it would lock the "dentry", and return the
(similarly locked) parent. Or NULL for a root dentry, of course.

Please?

             Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/