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

[Reinette Chatre]

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?

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.
	 /
	rdt_root_kn = rdtgroup_default.kn;
	mutex_unlock(&rdtgroup_mutex);
	cpus_read_unlock();

	ret = kernfs_get_tree(fc);
	if (!ctx->kfc.new_sb_created)
		resctrl_unmount();

	kernfs_put(rdt_root_kn);
	

Reinette