[PATCH v1 11/12] Call cond_reshed() as needed in d_for_each_positive_child().
NeilBrown <[email protected]> Mon, 3 Aug 2026 11:21:20 +1000
| Newsgroups | org.kernel.vger.ceph-devel,org.kernel.vger.autofs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs |
|---|---|
| Message-ID | <[email protected]> |
From: NeilBrown <[email protected]> There are reports that large numbers of negative dentries can lead to soft lockups when walking the d_children/d_sib list, particularly in fsnotify_set_children_dentry_flags(). (See link below) This can be addressed by allowing a schedule during the walk in the same way that scan_positives() (in libfs.c) already does. As we now walk the list in common code, we can add that schedule call to all places that walk the list using d_for_each_positive_child iterators. Doing this involves using an on-stack dentry (about 330 bytes) which is not standard practice but should be safe since Commit 3df5153c5f12 ("make cursors NORCU") made cursors NORCU, confirming that they can safely be released without waiting. Also importantly nothing ever takes a reference on a cursor that it doesn't own. To facilitate this, a new __d_init() is split out of __d_alloc() and used in d_init_cursor() which can be called on an on-stack dentry as is done in the new d_next_sibling_sched(). d_for_each_positive_child*() now use d_next_sibling_sched() if appropriate and they now assert they might_sleep()). Note that d_walk() also walks this list and does not use the new helper. Some instances of d_walk() do abort when a resched is needed, others do not. This patch does not help with those. Link https://lwn.net/Articles/1079407/ Signed-off-by: NeilBrown <[email protected]> --- fs/dcache.c | 109 ++++++++++++++++++++++++++++++++++------- include/linux/dcache.h | 23 ++++++--- 2 files changed, 108 insertions(+), 24 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index 50fbbcceca01..3678f86fe693 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1885,6 +1885,24 @@ void d_invalidate(struct dentry *dentry) } EXPORT_SYMBOL(d_invalidate); +static void __d_init(struct dentry *dentry, struct super_block *sb) +{ + dentry->d_flags = 0; + lockref_init(&dentry->d_lockref); + seqcount_spinlock_init(&dentry->d_seq, &dentry->d_lock); + dentry->d_inode = NULL; + dentry->d_parent = dentry; + dentry->d_sb = sb; + dentry->d_op = sb->__s_d_op; + dentry->d_flags = sb->s_d_flags; + dentry->d_fsdata = NULL; + INIT_HLIST_BL_NODE(&dentry->d_hash); + INIT_LIST_HEAD(&dentry->d_lru); + INIT_HLIST_HEAD(&dentry->d_children); + dentry->waiters = NULL; + INIT_HLIST_NODE(&dentry->d_sib); +} + /** * __d_alloc - allocate a dcache entry * @sb: filesystem it will belong to @@ -1894,7 +1912,6 @@ EXPORT_SYMBOL(d_invalidate); * available. On a success the dentry is returned. The name passed in is * copied and the copy passed in may be reused after this call. */ - static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) { struct dentry *dentry; @@ -1922,14 +1939,14 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) p = kmalloc_flex(*p, name, name->len + 1, GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE); if (!p) { - kmem_cache_free(dentry_cache, dentry); + kmem_cache_free(dentry_cache, dentry); return NULL; } atomic_set(&p->count, 1); dname = p->name; } else { dname = dentry->d_shortname.string; - } + } dentry->__d_name.len = name->len; dentry->__d_name.hash = name->hash; @@ -1939,20 +1956,7 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) /* Make sure we always see the terminating NUL character */ smp_store_release(&dentry->__d_name.name, dname); /* ^^^ */ - dentry->d_flags = 0; - lockref_init(&dentry->d_lockref); - seqcount_spinlock_init(&dentry->d_seq, &dentry->d_lock); - dentry->d_inode = NULL; - dentry->d_parent = dentry; - dentry->d_sb = sb; - dentry->d_op = sb->__s_d_op; - dentry->d_flags = sb->s_d_flags; - dentry->d_fsdata = NULL; - INIT_HLIST_BL_NODE(&dentry->d_hash); - INIT_LIST_HEAD(&dentry->d_lru); - INIT_HLIST_HEAD(&dentry->d_children); - dentry->waiters = NULL; - INIT_HLIST_NODE(&dentry->d_sib); + __d_init(dentry, sb); if (dentry->d_op && dentry->d_op->d_init) { err = dentry->d_op->d_init(dentry); @@ -1969,6 +1973,77 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) return dentry; } +/** + * d_init_cursor - initialise an on-stack dentry cursor + * @dentry: the dentry to be initialised + * @parent: dentry of parent where cursor will be used. + * + * A DCACHE_DENTRY_CURSOR is initalised for use in marking + * a place in the d_children/d_sib list. While sb-related + * fields are filled in, they should never be used. + * The filesystems d_init is not called and a final d_put() + * should not be called, else d_release would be called. + */ +static void d_init_cursor(struct dentry *dentry, struct dentry *parent) +{ + const struct qstr *name = &slash_name; + char *dname; + + /* + * We guarantee that the inline name is always NUL-terminated. + * This way the memcpy() done by the name switching in rename + * will still always have a NUL at the end, even if we might + * be overwriting an internal NUL character + */ + dentry->d_shortname.string[DNAME_INLINE_LEN-1] = 0; + name = &slash_name; + dname = dentry->d_shortname.string; + + dentry->__d_name.len = name->len; + dentry->__d_name.hash = name->hash; + memcpy(dname, name->name, name->len); + dname[name->len] = 0; + + /* Make sure we always see the terminating NUL character */ + smp_store_release(&dentry->__d_name.name, dname); /* ^^^ */ + + __d_init(dentry, parent->d_sb); + dentry->d_parent = parent; + dentry->d_flags |= DCACHE_DENTRY_CURSOR | DCACHE_NORCU; +} + +/** + * d_next_sibling_sched - return next sibling, but schedule() first + * @child: current child + * + * This behaves like d_next_sibling(), but drops the parent d_lock + * and calls cond_resched() to avoid any soft-lockup. A cursor + * is inserted as place holder. + * + * Caller must hold the parents d_lock, and must hold a counted + * reference on the parent. + * + * Returns: the next sibling, or NULL if there is none. + */ +struct dentry *d_next_sibling_sched(struct dentry *child) +__must_hold(&child->d_parent->d_lock) +{ + struct dentry cursor; + + d_init_cursor(&cursor, child->d_parent); + hlist_add_behind(&cursor.d_sib, &child->d_sib); + spin_unlock(&cursor.d_parent->d_lock); + cond_resched(); + spin_lock(&cursor.d_parent->d_lock); + child = d_next_sibling(&cursor); + __hlist_del(&cursor.d_sib); + if (hlist_empty(&cursor.d_parent->d_children)) + dput_dlock(cursor.d_parent); + WARN_ON(d_count(&cursor) != 1); + return child; +} +EXPORT_SYMBOL(d_next_sibling_sched); + /** * d_alloc - allocate a dcache entry * @parent: parent of entry to allocate diff --git a/include/linux/dcache.h b/include/linux/dcache.h index cabf05e74b1f..f7d5954c58da 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -656,10 +656,15 @@ static inline struct dentry *d_next_sibling(const struct dentry *dentry) return hlist_entry_safe(dentry->d_sib.next, struct dentry, d_sib); } +struct dentry *d_next_sibling_sched(struct dentry *child); + static inline struct dentry *d_next_positive(struct dentry *child) { do { - child = d_next_sibling(child); + if (need_resched()) + child = d_next_sibling_sched(child); + else + child = d_next_sibling(child); } while (child && !d_really_is_positive(child)); return child; } @@ -691,9 +696,11 @@ static inline struct dentry *d_first_positive(const struct dentry *parent, * which have at some point in the past been positive. */ #define d_for_each_positive_child(child, parent) \ - scoped_guard(spinlock, &parent->d_lock) \ - for (child = d_first_positive(parent, NULL); child; \ - child = d_next_positive(child)) + if (({might_sleep();0;})) ; else \ + scoped_guard(spinlock, &parent->d_lock) \ + for (child = d_first_positive(parent, NULL); \ + child; \ + child = d_next_positive(child)) /** * d_for_each_positive_child_continue - iterate over remaining positive children @@ -710,9 +717,11 @@ static inline struct dentry *d_first_positive(const struct dentry *parent, * */ #define d_for_each_positive_child_continue(child, parent) \ - scoped_guard(spinlock, &parent->d_lock) \ - for (child = d_first_positive(parent, child); child; \ - child = d_next_positive(child)) + if (({might_sleep();0;})) ; else \ + scoped_guard(spinlock, &parent->d_lock) \ + for (child = d_first_positive(parent, child); \ + child; \ + child = d_next_positive(child)) void set_default_d_op(struct super_block *, const struct dentry_operations *); struct dentry *d_make_persistent(struct dentry *, struct inode *); -- 2.50.0.107.gf914562f5916.dirty