Re: [PATCH] kernfs: convert kernfs_idr_lock to an irq safe raw spinlock

[Tejun Heo <tj@kernel.org>]

On Tue, Jan 09, 2024 at 06:05:09PM +0100, Andrea Righi wrote:
> On Tue, Jan 09, 2024 at 05:35:36PM +0100, Geert Uytterhoeven wrote:
> > Reverting commit c312828c37a72fe2 fixes that.
> > I have seen this issue on several Renesas arm32 and arm64 platforms.
> > 
> > Also, I am wondering if the issue fixed by commit c312828c37a72fe2
> > can still be reproduced on v6.7-rc5 or later?
> 
> Yep, I can still reproduce it (this is with v6.7):
...
> I'm wondering if using a regular spinlock instead of a raw spinlock
> could be a reasonable compromise.

I don't think that'd work on RT as we can end up nesting mutex inside a raw
spinlock.

> We have a GFP_ATOMIC allocation in __kernfs_new_node():
> 
> 	raw_spin_lock_irqsave(&kernfs_idr_lock, irqflags);
> 	ret = idr_alloc_cyclic(&root->ino_idr, kn, 1, 0, GFP_ATOMIC);
> 	...
>         raw_spin_unlock_irqrestore(&kernfs_idr_lock, irqflags);
> 
> That should become valid using a
> spin_lock_irqsave/spin_unlock_irqrestore(), right?

Yeah, this part should be fine. I think the right thing to do here is making
the idr RCU safe so that lookup path doesn't depend on the lock.

Greg, can you please revert c312828c37a72fe2 for now?

Thanks.

-- 
tejun