Re: Kernel deadlock in 6.7.5 + hacks, maybe debugfs related.

[Ben Greear <greearb@candelatech.com>]

On 2/27/24 06:32, Johannes Berg wrote:
> On Tue, 2024-02-27 at 06:29 -0800, Ben Greear wrote:
>>> --- a/fs/debugfs/inode.c
>>> +++ b/fs/debugfs/inode.c
>>> @@ -751,13 +751,19 @@ static void __debugfs_file_removed(struct dentry *dentry)
>>>    	if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
>>>    		return;
>>>    
>>> -	/* if we hit zero, just wait for all to finish */
>>> -	if (!refcount_dec_and_test(&fsd->active_users)) {
>>> -		wait_for_completion(&fsd->active_users_drained);
>>> -		return;
>>> -	}
>>> +	/*
>>> +	 * Now that debugfs_file_get() no longer sees a valid entry,
>>> +	 * decrement the refcount to remove the initial reference.
>>> +	 */
>>> +	refcount_dec(&fsd->active_users);
> 
>>
>> [   94.576688] ------------[ cut here ]------------
>> [   94.576699] refcount_t: decrement hit 0; leaking memory.
>>
> 
> Ah ... right, refcount_dec() doesn't like to hit 0, it's not meant for
> this path.
> 
> I guess we can
> 
> if (refcount_dec_and_test(...))
>     return;
> 
> while (refcount_read(...)) { ... }
> 
> johannes
> 

Like this?

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 034a617cb1a5..166053095610 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -751,13 +751,20 @@ static void __debugfs_file_removed(struct dentry *dentry)
         if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
                 return;

-       /* if we hit zero, just wait for all to finish */
-       if (!refcount_dec_and_test(&fsd->active_users)) {
-               wait_for_completion(&fsd->active_users_drained);
+       /*
+        * Now that debugfs_file_get() no longer sees a valid entry,
+        * decrement the refcount to remove the initial reference.
+        */
+       if (!refcount_dec_and_test(&fsd->active_users))
                 return;
-       }

-       /* if we didn't hit zero, try to cancel any we can */
+       /*
+        * As long as it's not zero, try to cancel any cancellations,
+        * new incoming ones will wake up the completion as we might
+        * have raced: debugfs_file_get() had already been done, but
+        * debugfs_enter_cancellation() hadn't, by the time we got
+        * to this point here.
+        */
         while (refcount_read(&fsd->active_users)) {
                 struct debugfs_cancellation *c;


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com