make take_dentry_name_snapshot() lockless

Use ->d_seq instead of grabbing ->d_lock; in case of shortname dentries
that avoids any stores to shared data objects and in case of long names
we are down to (unavoidable) atomic_inc on the external_name refcount.

Makes the thing safer as well - the areas where ->d_seq is held odd are
all nested inside the areas where ->d_lock is held, and the latter are
much more numerous.

NOTE: now that there is a lockless path where we might try to grab
a reference to an already doomed external_name instance, it is no
longer possible for external_name.u.count and external_name.u.head
to share space (kudos to Linus for spotting that).

To reduce the noise this commit just make external_name.u a struct
(instead of union); the next commit will dissolve it.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/dcache.c b/fs/dcache.c
index 52662a5d08e4795944cf03978ebcf36bc1118dae..f387dc97df864d3e00d12680881c354d34063589 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -296,9 +296,9 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 }
 
 struct external_name {
-       union {
-               atomic_t count;
-               struct rcu_head head;
+       struct {
+               atomic_t count;         // ->count and ->head can't be combined
+               struct rcu_head head;   // see take_dentry_name_snapshot()
        } u;
        unsigned char name[];
 };
@@ -329,15 +329,30 @@ static inline int dname_external(const struct dentry *dentry)
 
 void take_dentry_name_snapshot(struct name_snapshot *name, struct dentry *dentry)
 {
-       spin_lock(&dentry->d_lock);
-       name->name = dentry->d_name;
-       if (unlikely(dname_external(dentry))) {
-               atomic_inc(&external_name(dentry)->u.count);
-       } else {
+       unsigned seq;
+       const unsigned char *s;
+
+       rcu_read_lock();
+retry:
+       seq = read_seqcount_begin(&dentry->d_seq);
+       s = READ_ONCE(dentry->d_name.name);
+       name->name.hash_len = dentry->d_name.hash_len;
+       name->name.name = name->inline_name.string;
+       if (likely(s == dentry->d_shortname.string)) {
                name->inline_name = dentry->d_shortname;
-               name->name.name = name->inline_name.string;
+       } else {
+               struct external_name *p;
+               p = container_of(s, struct external_name, name[0]);
+               // get a valid reference
+               if (unlikely(!atomic_inc_not_zero(&p->u.count)))
+                       goto retry;
+               name->name.name = s;
        }
-       spin_unlock(&dentry->d_lock);
+       if (read_seqcount_retry(&dentry->d_seq, seq)) {
+               release_dentry_name_snapshot(name);
+               goto retry;
+       }
+       rcu_read_unlock();
 }
 EXPORT_SYMBOL(take_dentry_name_snapshot);