Re: [PATCH 3/4] fs/resctrl: Fix deadlock for errors during mount

["Chen, Yu C" <yu.c.chen@intel.com>]

Hi Reinette,
On 5/12/2026 10:34 PM, Reinette Chatre wrote:
> Hi Chenyu,
> 
> On 5/12/26 12:28 AM, Chen, Yu C wrote:
>> On 5/12/2026 6:53 AM, Reinette Chatre wrote:
>>> Hi Tony,
>>>
>>> On 5/8/26 11:21 AM, Tony Luck wrote:
>>
>>> +     * Obtain reference with locks held to protect against interference
>>> +     * from resctrl_exit().
>>> +    */
>>> +    kernfs_get(rdt_root_kn);
>>
>> [ ... ]
>>
>>> @@ -3130,6 +3144,7 @@ static int rdt_get_tree(struct fs_context *fc)
>>>         */
>>>        if (!ctx->kfc.new_sb_created)
>>>            resctrl_unmount();
>>> +    kernfs_put(rdt_root_kn);
>>
>> I wonder if above should be protected against
>>      cpus_read_lock();
>>      mutex_lock(&rdtgroup_mutex);
>> like kernfs_get()?
> 
> It is not obvious to me what this protection would be needed for.
> Do you have a troublesome scenario in mind?
> 
> rdt_root_kn is a local copy of rdtgroup_default.kn. The latter is indeed
> protected by the mutex. The reason why the kernfs_get() is protected
> by the mutex is to ensure what rdt_root_kn points to, rdtgroup_default.kn, remains
> accessible after the mutex is dropped. Nothing else modifies rdt_root_kn. I
> understand the appeal of symmetry but it is not clear to me what the extra
> locking is needed for here?
> 

Thanks for the detailed explanation. I now agree there is no need to
protect kernfs_put() with a lock here only for symmetry reason. I
previously thought racing conditions would occur if two code paths
concurrently enter kernfs_put() and target the same data area.
However, since kernfs_put() contains an atomic compare, only one
code path can proceed, making the operation safe.

> Could it perhaps make this flow easier to understand if the kernfs_get() is
> of the mutex protected rdtgroup_default.kn while the kernfs_put() is
> of the local backup copy? For example:
> 
> 	/* Ensure root kn remains accessible after mutex is unlocked */
> 	kernfs_get(rdtgroup_default.kn);
> 	/*
> 	 * Make backup of rdtgroup_default.kn just in case one of the
> 	 * following flows (that sets rdtgroup_default.kn to NULL) run after
> 	 * the mutex is unlocked:
> 	 * resctrl_exit()->resctrl_fs_teardown()->rdtgroup_destroy_root()
> 	 * kernfs_get_tree()->deactivate_locked_super()->rdt_kill_sb()->resctrl_unmount()->resctrl_fs_teardown()->rdtgroup_destroy_root()
> 	 * These flows would not actually result in rdtgroup_default.kn
> 	 * being removed thanks to the additional reference.
> 	 /

Yes, this comment is very clear and helpful.

thanks,
Chenyu