Re: [RFC PATCH V2 06/22] x86/intel_rdt: Create pseudo-locked regions

[Reinette Chatre]

Hi Thomas,

On 2/19/2018 3:16 PM, Thomas Gleixner wrote:
> On Mon, 19 Feb 2018, Reinette Chatre wrote:
>> On 2/19/2018 12:57 PM, Thomas Gleixner wrote:
>>> On Tue, 13 Feb 2018, Reinette Chatre wrote:
>>>
>>>> System administrator creates/removes pseudo-locked regions by
>>>> creating/removing directories in the pseudo-lock subdirectory of the
>>>> resctrl filesystem. Here we add directory creation and removal support.
>>>>
>>>> A "pseudo-lock region" is introduced, which represents an
>>>> instance of a pseudo-locked cache region. During mkdir a new region is
>>>> created but since we do not know which cache it belongs to at that time
>>>> we maintain a global pointer to it from where it will be moved to the cache
>>>> (rdt_domain) it belongs to after initialization. This implies that
>>>> we only support one uninitialized pseudo-locked region at a time.
>>>
>>> Whats the reason for this restriction? If there are uninitialized
>>> directories, so what?
>>
>> I was thinking about a problematic scenario where an application
>> attempts to create infinite directories. All of these uninitialized
>> directories need to be kept track of before they are initialized as
>> pseudo-locked regions. It seemed simpler to require that one
>> pseudo-locked region is set up at a time.
> 
> If the application is allowed to create directories then it can also create
> a dozen unused resource control groups. This is not a Joe User operation so
> there is no problem.

Thank you for the guidance. I will remove this restriction.

>>>> +/*
>>>> + * rdt_pseudo_lock_rmdir - Remove pseudo-lock region
>>>> + *
>>>> + * LOCKING:
>>>> + * Since the pseudo-locked region can be associated with a RDT domain at
>>>> + * removal we take both rdtgroup_mutex and rdt_pseudo_lock_mutex to protect
>>>> + * the rdt_domain access as well as the pseudo_lock_region access.
>>>
>>> Is there a real reason / benefit for having this second mutex?
>>
>> Some interactions with the pseudo-locked region are currently done
>> without the need for the rdtgroup_mutex. For example, interaction with
>> the character device associated with the pseudo-locked region (the
>> mmap() call) as well as the debugfs operations.
> 
> Well, yes. But none of those operations are hot path so having the double
> locking in lots of the other function is just extra complexity for no real
> value.

I will revise.

Thank you very much.

Reinette