Skip to content

Commit e3977e0

Browse files
htejungregkh
authored andcommitted
Revert "kernfs: convert kernfs_idr_lock to an irq safe raw spinlock"
This reverts commit dad3fb6. The commit converted kernfs_idr_lock to an IRQ-safe raw_spinlock because it could be acquired while holding an rq lock through bpf_cgroup_from_id(). However, kernfs_idr_lock is held while doing GPF_NOWAIT allocations which involves acquiring an non-IRQ-safe and non-raw lock leading to the following lockdep warning: ============================= [ BUG: Invalid wait context ] 6.7.0-rc5-kzm9g-00251-g655022a45b1c #578 Not tainted ----------------------------- swapper/0/0 is trying to lock: dfbcd488 (&c->lock){....}-{3:3}, at: local_lock_acquire+0x0/0xa4 other info that might help us debug this: context-{5:5} 2 locks held by swapper/0/0: #0: dfbc9c6 (lock){+.+.}-{3:3}, at: local_lock_acquire+0x0/0xa4 #1: c0c012a8 (kernfs_idr_lock){....}-{2:2}, at: __kernfs_new_node.constprop.0+0x68/0x258 stack backtrace: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.7.0-rc5-kzm9g-00251-g655022a45b1c #578 Hardware name: Generic SH73A0 (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x68/0x90 dump_stack_lvl from __lock_acquire+0x3cc/0x168c __lock_acquire from lock_acquire+0x274/0x30c lock_acquire from local_lock_acquire+0x28/0xa4 local_lock_acquire from ___slab_alloc+0x234/0x8a8 ___slab_alloc from __slab_alloc.constprop.0+0x30/0x44 __slab_alloc.constprop.0 from kmem_cache_alloc+0x7c/0x148 kmem_cache_alloc from radix_tree_node_alloc.constprop.0+0x44/0xdc radix_tree_node_alloc.constprop.0 from idr_get_free+0x110/0x2b8 idr_get_free from idr_alloc_u32+0x9c/0x108 idr_alloc_u32 from idr_alloc_cyclic+0x50/0xb8 idr_alloc_cyclic from __kernfs_new_node.constprop.0+0x88/0x258 __kernfs_new_node.constprop.0 from kernfs_create_root+0xbc/0x154 kernfs_create_root from sysfs_init+0x18/0x5c sysfs_init from mnt_init+0xc4/0x220 mnt_init from vfs_caches_init+0x6c/0x88 vfs_caches_init from start_kernel+0x474/0x528 start_kernel from 0x0 Let's rever the commit. It's undesirable to spread out raw spinlock usage anyway and the problem can be solved by protecting the lookup path with RCU instead. Signed-off-by: Tejun Heo <[email protected]> Cc: Andrea Righi <[email protected]> Reported-by: Geert Uytterhoeven <[email protected]> Link: http://lkml.kernel.org/r/CAMuHMdV=AKt+mwY7svEq5gFPx41LoSQZ_USME5_MEdWQze13ww@mail.gmail.com Link: https://lore.kernel.org/r/[email protected] Tested-by: Andrea Righi <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent c312828 commit e3977e0

File tree

1 file changed

+10
-13
lines changed

1 file changed

+10
-13
lines changed

fs/kernfs/dir.c

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ static DEFINE_RWLOCK(kernfs_rename_lock); /* kn->parent and ->name */
2727
*/
2828
static DEFINE_SPINLOCK(kernfs_pr_cont_lock);
2929
static char kernfs_pr_cont_buf[PATH_MAX]; /* protected by pr_cont_lock */
30-
static DEFINE_RAW_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */
30+
static DEFINE_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */
3131

3232
#define rb_to_kn(X) rb_entry((X), struct kernfs_node, rb)
3333

@@ -539,7 +539,6 @@ void kernfs_put(struct kernfs_node *kn)
539539
{
540540
struct kernfs_node *parent;
541541
struct kernfs_root *root;
542-
unsigned long flags;
543542

544543
if (!kn || !atomic_dec_and_test(&kn->count))
545544
return;
@@ -564,9 +563,9 @@ void kernfs_put(struct kernfs_node *kn)
564563
simple_xattrs_free(&kn->iattr->xattrs, NULL);
565564
kmem_cache_free(kernfs_iattrs_cache, kn->iattr);
566565
}
567-
raw_spin_lock_irqsave(&kernfs_idr_lock, flags);
566+
spin_lock(&kernfs_idr_lock);
568567
idr_remove(&root->ino_idr, (u32)kernfs_ino(kn));
569-
raw_spin_unlock_irqrestore(&kernfs_idr_lock, flags);
568+
spin_unlock(&kernfs_idr_lock);
570569
kmem_cache_free(kernfs_node_cache, kn);
571570

572571
kn = parent;
@@ -608,7 +607,6 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
608607
struct kernfs_node *kn;
609608
u32 id_highbits;
610609
int ret;
611-
unsigned long irqflags;
612610

613611
name = kstrdup_const(name, GFP_KERNEL);
614612
if (!name)
@@ -619,13 +617,13 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
619617
goto err_out1;
620618

621619
idr_preload(GFP_KERNEL);
622-
raw_spin_lock_irqsave(&kernfs_idr_lock, irqflags);
620+
spin_lock(&kernfs_idr_lock);
623621
ret = idr_alloc_cyclic(&root->ino_idr, kn, 1, 0, GFP_ATOMIC);
624622
if (ret >= 0 && ret < root->last_id_lowbits)
625623
root->id_highbits++;
626624
id_highbits = root->id_highbits;
627625
root->last_id_lowbits = ret;
628-
raw_spin_unlock_irqrestore(&kernfs_idr_lock, irqflags);
626+
spin_unlock(&kernfs_idr_lock);
629627
idr_preload_end();
630628
if (ret < 0)
631629
goto err_out2;
@@ -661,9 +659,9 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
661659
return kn;
662660

663661
err_out3:
664-
raw_spin_lock_irqsave(&kernfs_idr_lock, irqflags);
662+
spin_lock(&kernfs_idr_lock);
665663
idr_remove(&root->ino_idr, (u32)kernfs_ino(kn));
666-
raw_spin_unlock_irqrestore(&kernfs_idr_lock, irqflags);
664+
spin_unlock(&kernfs_idr_lock);
667665
err_out2:
668666
kmem_cache_free(kernfs_node_cache, kn);
669667
err_out1:
@@ -716,9 +714,8 @@ struct kernfs_node *kernfs_find_and_get_node_by_id(struct kernfs_root *root,
716714
struct kernfs_node *kn;
717715
ino_t ino = kernfs_id_ino(id);
718716
u32 gen = kernfs_id_gen(id);
719-
unsigned long flags;
720717

721-
raw_spin_lock_irqsave(&kernfs_idr_lock, flags);
718+
spin_lock(&kernfs_idr_lock);
722719

723720
kn = idr_find(&root->ino_idr, (u32)ino);
724721
if (!kn)
@@ -742,10 +739,10 @@ struct kernfs_node *kernfs_find_and_get_node_by_id(struct kernfs_root *root,
742739
if (unlikely(!__kernfs_active(kn) || !atomic_inc_not_zero(&kn->count)))
743740
goto err_unlock;
744741

745-
raw_spin_unlock_irqrestore(&kernfs_idr_lock, flags);
742+
spin_unlock(&kernfs_idr_lock);
746743
return kn;
747744
err_unlock:
748-
raw_spin_unlock_irqrestore(&kernfs_idr_lock, flags);
745+
spin_unlock(&kernfs_idr_lock);
749746
return NULL;
750747
}
751748

0 commit comments

Comments
 (0)