Re: [PATCH v10 13/17] fs/resctrl: Call architecture hooks for every mount/unmount

Hi Tony,

On 8/18/26 11:20 AM, Luck, Tony wrote:
> On Mon, Aug 17, 2026 at 06:02:31PM -0700, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 7/29/26 10:27 AM, Tony Luck wrote:
>>>  static int rdt_get_tree(struct fs_context *fc)
>>> @@ -3175,9 +3176,11 @@ static int rdt_get_tree(struct fs_context *fc)
>>>  	struct kernfs_node *rdt_root_kn;
>>>  	struct rdt_l3_mon_domain *dom;
>>>  	struct rdt_resource *r;
>>> +	bool cleanup = true;
>>>  	int ret;
>>>  
>>> -	DO_ONCE_SLEEPABLE(resctrl_arch_pre_mount);
>>> +	if (resctrl_arch_pre_mount() == -EBUSY)
>>> +		return -EBUSY;
>>>  
>> This does not look right. Are you intending to add new meanings to EBUSY returned
>> by resctrl so that user space now need to choose between "resctrl fs is already mounted"
>> and "the underlying architecture is busy with something else"? Based on the implementation
>> the architecture could now also return EBUSY when resctrl fs is mounted, but what prevents
>> an architecture from returning EBUSY in some other scenario? This error code to user space
>> does not seem like a responsibility that the architecture code needs to have.
> 
> I've been struggling with how to handle races between multiple mount/unmount
> operations when the resctrl_arch_pre_mount() and resctrl_arch_unmount()
> calls are not protected by any locks.
> 
> As you have seen, I have some (poorly documented) locking at the architecture
> level that attempts to solve this by keeping its own idea of the mount status
> of resctrl.
> 
> A call to resctrl_arch_pre_mount() when the filesystem is not mounted
> will do AET enumeration, and if that succeeds create the domains. A nested
> call does not need to do anything. But when I had that simply return, that
> opened a race against an unmount request.
> 
> If the unmount wins the race to acquire rdtgroup_mutex, then the file
> system is unmounted. At the end of the unmount the mutex is released and
> resctrl_arch_unmount() called. This will execute in parallel with that mount
> request, so various bad things may happen as the mount proceeds while AET
> is being torn down.
> 
> My solution is to have architecture return status to let filessytem code
> know that it did nothing because the file system was already mounted. I
> think that filesystem code should just return at that point.
> 
> Is there a better way to code this? Or is the problem that I didn't describe
> the race, and thus the need for architecture code to tell file system code
> not to proceed with the mount?
It is always helpful if the changelog describes why things are done in a particular
way.

When considering the race you describe I do think the locking you introduced in [1] is
a better solution. Specifically, a new mutex that (together with rdtgroup_mutex) protects
resctrl_mounted which is held during resctrl_arch_unmount() and resctrl_arch_pre_mount().
This should eliminate the duplicate "is resctrl fs mounted" state in resctrl filesystem and
x86 resctrl while also eliminating the race you describe.

I tried to read the discussion around [1] again but it went on some tangents and then stopped
after moving to the new solution without new fs locks [2]. My original concerns seem to be
new asymmetrical resctrl fs locking to appease architecture code. I believe that you clearly
demonstrated that this cannot be accomplished cleanly by architecture alone.

Reinette

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/aeqdOlO3BkKQqSQ9@agluck-desk3/