From affda48410a5bbfd516def60bbc97f2683cd9f7b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 29 May 2016 18:35:12 -0400 Subject: trim fsnotify hooks a bit fsnotify_d_move()/__fsnotify_d_instantiate()/__fsnotify_update_dcache_flags() are identical to each other, regardless of the config. Signed-off-by: Al Viro --- fs/dcache.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index ad4a542e9bab..f9c63c108881 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1769,7 +1769,7 @@ static void __d_instantiate(struct dentry *dentry, struct inode *inode) raw_write_seqcount_begin(&dentry->d_seq); __d_set_inode_and_type(dentry, inode, add_flags); raw_write_seqcount_end(&dentry->d_seq); - __fsnotify_d_instantiate(dentry); + fsnotify_update_flags(dentry); spin_unlock(&dentry->d_lock); } @@ -2563,7 +2563,7 @@ static inline void __d_add(struct dentry *dentry, struct inode *inode) raw_write_seqcount_begin(&dentry->d_seq); __d_set_inode_and_type(dentry, inode, add_flags); raw_write_seqcount_end(&dentry->d_seq); - __fsnotify_d_instantiate(dentry); + fsnotify_update_flags(dentry); } _d_rehash(dentry); if (dir) @@ -2853,8 +2853,8 @@ static void __d_move(struct dentry *dentry, struct dentry *target, list_move(&target->d_child, &target->d_parent->d_subdirs); list_move(&dentry->d_child, &dentry->d_parent->d_subdirs); if (exchange) - fsnotify_d_move(target); - fsnotify_d_move(dentry); + fsnotify_update_flags(target); + fsnotify_update_flags(dentry); } write_seqcount_end(&target->d_seq); -- cgit From 550dce01dd606c88a837138aa448ccd367fb0cbb Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 29 May 2016 20:13:30 -0400 Subject: unify dentry_iput() and dentry_unlink_inode() There is a lot of duplication between dentry_unlink_inode() and dentry_iput(). The only real difference is that dentry_unlink_inode() bumps ->d_seq and dentry_iput() doesn't. The argument of the latter is known to have been unhashed, so anybody who might've found it in RCU lookup would already be doomed to a ->d_seq mismatch. And we want to avoid pointless smp_rmb() there. This patch makes dentry_unlink_inode() bump ->d_seq only for hashed dentries. It's safe (d_delete() calls that sucker only if we are holding the only reference to dentry, so rehash is not going to happen) and it allows to use dentry_unlink_inode() in __dentry_kill() and get rid of dentry_iput(). The interesting question here is profiling; it *is* a hot path, and extra conditional jumps in there might or might not be painful. Signed-off-by: Al Viro --- fs/dcache.c | 45 ++++++++++----------------------------------- 1 file changed, 10 insertions(+), 35 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index f9c63c108881..fe7cde2f2fe5 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -335,44 +335,21 @@ static inline void dentry_rcuwalk_invalidate(struct dentry *dentry) /* * Release the dentry's inode, using the filesystem - * d_iput() operation if defined. Dentry has no refcount - * and is unhashed. - */ -static void dentry_iput(struct dentry * dentry) - __releases(dentry->d_lock) - __releases(dentry->d_inode->i_lock) -{ - struct inode *inode = dentry->d_inode; - if (inode) { - __d_clear_type_and_inode(dentry); - hlist_del_init(&dentry->d_u.d_alias); - spin_unlock(&dentry->d_lock); - spin_unlock(&inode->i_lock); - if (!inode->i_nlink) - fsnotify_inoderemove(inode); - if (dentry->d_op && dentry->d_op->d_iput) - dentry->d_op->d_iput(dentry, inode); - else - iput(inode); - } else { - spin_unlock(&dentry->d_lock); - } -} - -/* - * Release the dentry's inode, using the filesystem - * d_iput() operation if defined. dentry remains in-use. + * d_iput() operation if defined. */ static void dentry_unlink_inode(struct dentry * dentry) __releases(dentry->d_lock) __releases(dentry->d_inode->i_lock) { struct inode *inode = dentry->d_inode; + bool hashed = !d_unhashed(dentry); - raw_write_seqcount_begin(&dentry->d_seq); + if (hashed) + raw_write_seqcount_begin(&dentry->d_seq); __d_clear_type_and_inode(dentry); hlist_del_init(&dentry->d_u.d_alias); - raw_write_seqcount_end(&dentry->d_seq); + if (hashed) + raw_write_seqcount_end(&dentry->d_seq); spin_unlock(&dentry->d_lock); spin_unlock(&inode->i_lock); if (!inode->i_nlink) @@ -540,12 +517,10 @@ static void __dentry_kill(struct dentry *dentry) dentry->d_flags |= DCACHE_DENTRY_KILLED; if (parent) spin_unlock(&parent->d_lock); - dentry_iput(dentry); - /* - * dentry_iput drops the locks, at which point nobody (except - * transient RCU lookups) can reach this dentry. - */ - BUG_ON(dentry->d_lockref.count > 0); + if (dentry->d_inode) + dentry_unlink_inode(dentry); + else + spin_unlock(&dentry->d_lock); this_cpu_dec(nr_dentry); if (dentry->d_op && dentry->d_op->d_release) dentry->d_op->d_release(dentry); -- cgit From 2d902671ce1cd98cdc88d78c481889a1b2996101 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Thu, 30 Jun 2016 08:53:27 +0200 Subject: vfs: merge .d_select_inode() into .d_real() The two methods essentially do the same: find the real dentry/inode belonging to an overlay dentry. The difference is in the usage: vfs_open() uses ->d_select_inode() and expects the function to perform copy-up if necessary based on the open flags argument. file_dentry() uses ->d_real() passing in the overlay dentry as well as the underlying inode. vfs_rename() uses ->d_select_inode() but passes zero flags. ->d_real() with a zero inode would have worked just as well here. This patch merges the functionality of ->d_select_inode() into ->d_real() by adding an 'open_flags' argument to the latter. [Al Viro] Make the signature of d_real() match that of ->d_real() again. And constify the inode argument, while we are at it. Signed-off-by: Miklos Szeredi --- fs/dcache.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index d6847d7b123d..5405b89fe8ec 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1729,7 +1729,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op) DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE | DCACHE_OP_DELETE | - DCACHE_OP_SELECT_INODE | DCACHE_OP_REAL)); dentry->d_op = op; if (!op) @@ -1746,8 +1745,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op) dentry->d_flags |= DCACHE_OP_DELETE; if (op->d_prune) dentry->d_flags |= DCACHE_OP_PRUNE; - if (op->d_select_inode) - dentry->d_flags |= DCACHE_OP_SELECT_INODE; if (op->d_real) dentry->d_flags |= DCACHE_OP_REAL; -- cgit From ae0a843c740b4e63684eefae96097cf62d9b7a14 Mon Sep 17 00:00:00 2001 From: He Kuang Date: Sat, 26 Mar 2016 09:12:10 +0000 Subject: dentry_cmp(): use lockless_dereference() instead of smp_read_barrier_depends() lockless_dereference() was added which can be used in place of hard-coding smp_read_barrier_depends(). Signed-off-by: He Kuang Signed-off-by: Al Viro --- fs/dcache.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index e5c8ba76d426..dc37c0238b46 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -226,10 +226,9 @@ static inline int dentry_string_cmp(const unsigned char *cs, const unsigned char static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount) { - const unsigned char *cs; /* * Be careful about RCU walk racing with rename: - * use ACCESS_ONCE to fetch the name pointer. + * use 'lockless_dereference' to fetch the name pointer. * * NOTE! Even if a rename will mean that the length * was not loaded atomically, we don't care. The @@ -243,8 +242,8 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c * early because the data cannot match (there can * be no NUL in the ct/tcount data) */ - cs = ACCESS_ONCE(dentry->d_name.name); - smp_read_barrier_depends(); + const unsigned char *cs = lockless_dereference(dentry->d_name.name); + return dentry_string_cmp(cs, ct, tcount); } -- cgit From d4c91a8f7e5514a1e9cd37b453fda0dedfa8045d Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 25 Jun 2016 23:33:49 -0400 Subject: new helper: d_same_name() Signed-off-by: Al Viro --- fs/dcache.c | 127 +++++++++++++++++------------------------------------------- 1 file changed, 36 insertions(+), 91 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index dc37c0238b46..040c2586d483 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2066,42 +2066,19 @@ struct dentry *d_add_ci(struct dentry *dentry, struct inode *inode, } EXPORT_SYMBOL(d_add_ci); -/* - * Do the slow-case of the dentry name compare. - * - * Unlike the dentry_cmp() function, we need to atomically - * load the name and length information, so that the - * filesystem can rely on them, and can use the 'name' and - * 'len' information without worrying about walking off the - * end of memory etc. - * - * Thus the read_seqcount_retry() and the "duplicate" info - * in arguments (the low-level filesystem should not look - * at the dentry inode or name contents directly, since - * rename can change them while we're in RCU mode). - */ -enum slow_d_compare { - D_COMP_OK, - D_COMP_NOMATCH, - D_COMP_SEQRETRY, -}; -static noinline enum slow_d_compare slow_dentry_cmp( - const struct dentry *parent, - struct dentry *dentry, - unsigned int seq, - const struct qstr *name) +static inline bool d_same_name(const struct dentry *dentry, + const struct dentry *parent, + const struct qstr *name) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - - if (read_seqcount_retry(&dentry->d_seq, seq)) { - cpu_relax(); - return D_COMP_SEQRETRY; + if (likely(!(parent->d_flags & DCACHE_OP_COMPARE))) { + if (dentry->d_name.len != name->len) + return false; + return dentry_cmp(dentry, name->name, name->len) == 0; } - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - return D_COMP_NOMATCH; - return D_COMP_OK; + return parent->d_op->d_compare(parent, dentry, + dentry->d_name.len, dentry->d_name.name, + name) == 0; } /** @@ -2180,6 +2157,9 @@ seqretry: * dentry compare, we will do seqretries until it is stable, * and if we end up with a successful lookup, we actually * want to exit RCU lookup anyway. + * + * Note that raw_seqcount_begin still *does* smp_rmb(), so + * we are still guaranteed NUL-termination of ->d_name.name. */ seq = raw_seqcount_begin(&dentry->d_seq); if (dentry->d_parent != parent) @@ -2188,24 +2168,28 @@ seqretry: continue; if (unlikely(parent->d_flags & DCACHE_OP_COMPARE)) { + int tlen; + const char *tname; if (dentry->d_name.hash != hashlen_hash(hashlen)) continue; - *seqp = seq; - switch (slow_dentry_cmp(parent, dentry, seq, name)) { - case D_COMP_OK: - return dentry; - case D_COMP_NOMATCH: - continue; - default: + tlen = dentry->d_name.len; + tname = dentry->d_name.name; + /* we want a consistent (name,len) pair */ + if (read_seqcount_retry(&dentry->d_seq, seq)) { + cpu_relax(); goto seqretry; } + if (parent->d_op->d_compare(parent, dentry, + tlen, tname, name) != 0) + continue; + } else { + if (dentry->d_name.hash_len != hashlen) + continue; + if (dentry_cmp(dentry, str, hashlen_len(hashlen)) != 0) + continue; } - - if (dentry->d_name.hash_len != hashlen) - continue; *seqp = seq; - if (!dentry_cmp(dentry, str, hashlen_len(hashlen))) - return dentry; + return dentry; } return NULL; } @@ -2253,9 +2237,7 @@ EXPORT_SYMBOL(d_lookup); */ struct dentry *__d_lookup(const struct dentry *parent, const struct qstr *name) { - unsigned int len = name->len; unsigned int hash = name->hash; - const unsigned char *str = name->name; struct hlist_bl_head *b = d_hash(parent, hash); struct hlist_bl_node *node; struct dentry *found = NULL; @@ -2294,21 +2276,8 @@ struct dentry *__d_lookup(const struct dentry *parent, const struct qstr *name) if (d_unhashed(dentry)) goto next; - /* - * It is safe to compare names since d_move() cannot - * change the qstr (protected by d_lock). - */ - if (parent->d_flags & DCACHE_OP_COMPARE) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - goto next; - } else { - if (dentry->d_name.len != len) - goto next; - if (dentry_cmp(dentry, str, len)) - goto next; - } + if (!d_same_name(dentry, parent, name)) + goto next; dentry->d_lockref.count++; found = dentry; @@ -2461,9 +2430,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent, const struct qstr *name, wait_queue_head_t *wq) { - unsigned int len = name->len; unsigned int hash = name->hash; - const unsigned char *str = name->name; struct hlist_bl_head *b = in_lookup_hash(parent, hash); struct hlist_bl_node *node; struct dentry *new = d_alloc(parent, name); @@ -2514,17 +2481,8 @@ retry: continue; if (dentry->d_parent != parent) continue; - if (parent->d_flags & DCACHE_OP_COMPARE) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - continue; - } else { - if (dentry->d_name.len != len) - continue; - if (dentry_cmp(dentry, str, len)) - continue; - } + if (!d_same_name(dentry, parent, name)) + continue; hlist_bl_unlock(b); /* now we can try to grab a reference */ if (!lockref_get_not_dead(&dentry->d_lockref)) { @@ -2551,17 +2509,8 @@ retry: goto mismatch; if (unlikely(d_unhashed(dentry))) goto mismatch; - if (parent->d_flags & DCACHE_OP_COMPARE) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - goto mismatch; - } else { - if (unlikely(dentry->d_name.len != len)) - goto mismatch; - if (unlikely(dentry_cmp(dentry, str, len))) - goto mismatch; - } + if (unlikely(!d_same_name(dentry, parent, name))) + goto mismatch; /* OK, it *is* a hashed match; return it */ spin_unlock(&dentry->d_lock); dput(new); @@ -2657,8 +2606,6 @@ EXPORT_SYMBOL(d_add); struct dentry *d_exact_alias(struct dentry *entry, struct inode *inode) { struct dentry *alias; - int len = entry->d_name.len; - const char *name = entry->d_name.name; unsigned int hash = entry->d_name.hash; spin_lock(&inode->i_lock); @@ -2672,9 +2619,7 @@ struct dentry *d_exact_alias(struct dentry *entry, struct inode *inode) continue; if (alias->d_parent != entry->d_parent) continue; - if (alias->d_name.len != len) - continue; - if (dentry_cmp(alias, name, len)) + if (!d_same_name(alias, entry->d_parent, &entry->d_name)) continue; spin_lock(&alias->d_lock); if (!d_unhashed(alias)) { -- cgit From 285b102d3b745f3c2c110c9c327741d87e64aacc Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 28 Jun 2016 11:47:32 +0200 Subject: vfs: new d_init method Allow filesystem to initialize dentry at allocation time. Signed-off-by: Miklos Szeredi Signed-off-by: Al Viro --- fs/dcache.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index d5beef01cdfc..6d60a764c848 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1569,6 +1569,7 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) { struct dentry *dentry; char *dname; + int err; dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); if (!dentry) @@ -1627,6 +1628,16 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) INIT_LIST_HEAD(&dentry->d_child); d_set_d_op(dentry, dentry->d_sb->s_d_op); + if (dentry->d_op && dentry->d_op->d_init) { + err = dentry->d_op->d_init(dentry); + if (err) { + if (dname_external(dentry)) + kfree(external_name(dentry)); + kmem_cache_free(dentry_cache, dentry); + return NULL; + } + } + this_cpu_inc(nr_dentry); return dentry; -- cgit From 47be61845c775643f1aa4d2a54343549f943c94c Mon Sep 17 00:00:00 2001 From: Wei Fang Date: Wed, 6 Jul 2016 11:32:20 +0800 Subject: fs/dcache.c: avoid soft-lockup in dput() We triggered soft-lockup under stress test which open/access/write/close one file concurrently on more than five different CPUs: WARN: soft lockup - CPU#0 stuck for 11s! [who:30631] ... [] dput+0x100/0x298 [] terminate_walk+0x4c/0x60 [] path_lookupat+0x5cc/0x7a8 [] filename_lookup+0x38/0xf0 [] user_path_at_empty+0x78/0xd0 [] user_path_at+0x1c/0x28 [] SyS_faccessat+0xb4/0x230 ->d_lock trylock may failed many times because of concurrently operations, and dput() may execute a long time. Fix this by replacing cpu_relax() with cond_resched(). dput() used to be sleepable, so make it sleepable again should be safe. Cc: Signed-off-by: Wei Fang Signed-off-by: Al Viro --- fs/dcache.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index 6d60a764c848..f650a4fc5b7c 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -596,7 +596,6 @@ static struct dentry *dentry_kill(struct dentry *dentry) failed: spin_unlock(&dentry->d_lock); - cpu_relax(); return dentry; /* try again with same dentry */ } @@ -770,6 +769,8 @@ void dput(struct dentry *dentry) return; repeat: + might_sleep(); + rcu_read_lock(); if (likely(fast_dput(dentry))) { rcu_read_unlock(); @@ -803,8 +804,10 @@ repeat: kill_it: dentry = dentry_kill(dentry); - if (dentry) + if (dentry) { + cond_resched(); goto repeat; + } } EXPORT_SYMBOL(dput); -- cgit