linux-kernel - Re: [PATCH 17/46] fs: Use rename lock and RCU for multi-step operations

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: <AANLkTi=pj-pLYVSrU+6+BtioCWLod2ueZecNaap619fw@mail.gmail.com>
Date:	Tue, 18 Jan 2011 14:32:27 -0800
From:	Yehuda Sadeh Weinraub <yehudasa@...il.com>
To:	Nick Piggin <npiggin@...nel.dk>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Sage Weil <sage@...dream.net>
Subject: Re: [PATCH 17/46] fs: Use rename lock and RCU for multi-step operations

On Sat, Nov 27, 2010 at 1:44 AM, Nick Piggin <npiggin@...nel.dk> wrote:
> The remaining usages for dcache_lock is to allow atomic, multi-step read-side
> operations over the directory tree by excluding modifications to the tree.
> Also, to walk in the leaf->root direction in the tree where we don't have
> a natural d_lock ordering.
>
> This could be accomplished by taking every d_lock, but this would mean a
> huge number of locks and actually gets very tricky.
>
> Solve this instead by using the rename seqlock for multi-step read-side
> operations, retry in case of a rename so we don't walk up the wrong parent.
> Concurrent dentry insertions are not serialised against.  Concurrent deletes
> are tricky when walking up the directory: our parent might have been deleted
> when dropping locks so also need to check and retry for that.
>
> We can also use the rename lock in cases where livelock is a worry (and it
> is introduced in subsequent patch).
>
> Signed-off-by: Nick Piggin <npiggin@...nel.dk>
..
> @@ -237,6 +238,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
>        __releases(dcache_inode_lock)
>        __releases(dcache_lock)
>  {
> +       dentry->d_parent = NULL;
>        list_del(&dentry->d_u.d_child);
>        if (parent)
>                spin_unlock(&parent->d_lock);

There's an issue with ceph as it references the
dentry->d_parent(->d_inode) at dentry_release(), so setting
dentry->d_parent to NULL here doesn't work with ceph. Though there is
some workaround for it, we would like to be sure that this one is
really required so that we don't exacerbate the ugliness. The
workaround is to keep a pointer to the parent inode in the private
dentry structure, which will be referenced only at the .release()
callback. This is clearly not ideal.

Yehuda
--
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/