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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d6345cf7-159b-ab90-2df4-1a998358a710@intel.com>
Date:   Mon, 19 Feb 2018 19:21:44 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     fenghua.yu@...el.com, tony.luck@...el.com, gavin.hindman@...el.com,
        vikas.shivappa@...ux.intel.com, dave.hansen@...el.com,
        mingo@...hat.com, hpa@...or.com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH V2 06/22] x86/intel_rdt: Create pseudo-locked regions

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