Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <09751242-365d-bf0a-a031-d636e34e08d0@intel.com>
Date:   Thu, 15 Jun 2023 15:26:43 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.2
Subject: Re: [PATCH v4 24/24] x86/resctrl: Separate arch and fs resctrl locks
Content-Language: en-US
To:     James Morse <james.morse@arm.com>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        H Peter Anvin <hpa@zytor.com>,
        Babu Moger <Babu.Moger@amd.com>,
        <shameerali.kolothum.thodi@huawei.com>,
        D Scott Phillips OS <scott@os.amperecomputing.com>,
        <carl@os.amperecomputing.com>, <lcherian@marvell.com>,
        <bobo.shaobowang@huawei.com>, <tan.shaopeng@fujitsu.com>,
        <xingxin.hx@openanolis.org>, <baolin.wang@linux.alibaba.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Xin Hao <xhao@linux.alibaba.com>, <peternewman@google.com>,
        <dfustini@baylibre.com>
References: <20230525180209.19497-1-james.morse@arm.com>
 <20230525180209.19497-25-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230525180209.19497-25-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OVNsYndseTJCRTMyVy9GRlVnenI3ZXE1NSs2R3lrNENQVFlYY3VlV0pPc0I0?=
 =?utf-8?B?cndFcEZ0YjZvTWl2UzFZZndCL0N6dmpLN3V0UUVlT1NIUmpNVVZSUnY5UTIr?=
 =?utf-8?B?UHBiL0lOcVZCcm1FNE1ZTDlzaHVLSnA2VmJNei9NcHgrN1pVRU0yQWlXbjJH?=
 =?utf-8?B?bW8yT2s0cmJ1ZzJQMTVFSE1xWi9mUWZ4VjNKUFRRdm1mSG92SWVNQm81b3VQ?=
 =?utf-8?B?SFNiQjB5MnVwZURBNGw2K3BYVUU4WllxcVYxOUNZbENMbDFLc3ZJUG5UVmxv?=
 =?utf-8?B?bXhWUlRKV3NXMU1yb2ZiZGY4V3ppcUJTWFdTZTRXNDlGOGFZd0lkM24xWlFy?=
 =?utf-8?B?YjNwSnZqYWpxYTFIUmFESzJlRHhwUTQwTnloMjZMVUticzQ2a2xKRXo3aWNU?=
 =?utf-8?B?akFSTWY2THBUMWYrM3p2aHhtckxLVUY1WjlzTFBrTTJJdlA0ZU5ZSXdHcGd0?=
 =?utf-8?B?Z3RrLzBtMUhIandhSlNzd0RsNXBsOUNTSnJJNmVWNURtRGFaUHl0RVJPWnU4?=
 =?utf-8?B?Tk5XYkgvQlFNcUJBa0RWN3UyYzQ1b1VHczVDcStNbXhoS0REM3FRbk9NNDdz?=
 =?utf-8?B?TkhxOTIxRU54cHBnZXhDVzlOUzcxNFNHL01uM3pYWllXczg3TmhZNWh2WkFF?=
 =?utf-8?B?WEpQZ3dTdlZXM3JTUUhDQVlVVmJpbHI2eWJBWi84K0NMZ2RRRFc3ZEJCQ2F5?=
 =?utf-8?B?OTBLTVIvSS9EaUExWHZKd3hqTWdTTHg5c3Q0dFhQYmVDbDNJeU1TS25XbGRV?=
 =?utf-8?B?Z1ZvZVk0TFFpN1JVdGUrQ1F4L2JhK0x6MXI2UHJwSTFzV01zMzMzcGdaV3ly?=
 =?utf-8?B?d3VTaXhNR0FhOGc5eDUvTjVpYmQ4bDRyZ05FUkR1MmVOOHYzZ1pRMG1nQ0I1?=
 =?utf-8?B?T2tnTENUZWVZK1Q4cHp5djJDWThqM1g2NUJHV3plQyszclVWR1hTdVVuWHcy?=
 =?utf-8?B?VlR1RDVDVGo5WEhySkV4b0QwTGpPVFQ1NU5XNldjalhlL3hXWU1XR2ZlbXY3?=
 =?utf-8?B?RTV1Q0Vpd1VoRUUrU0d0RmFCZXdjeFJDQkhoTllrY0QvUjR5WkxpVFFiSkU0?=
 =?utf-8?B?dm9rQ0lUbDgycWRjbmxrd2xySkFGUjYyYzJPc2hBdVhZZWUxNWVXZVdtOGZw?=
 =?utf-8?B?cHJxOEhIK3N2NVAwV1dYenFSY2c2Q1NNUmV6dW9hRDFYb2dySzJDVXpFS3Nt?=
 =?utf-8?B?UFlQS3VxRzg4WkVwYUVWNUsxVEhQTlJFeE5YVzBqeGYrRmY1VWJ5NnZsVG9i?=
 =?utf-8?B?UzFYTWtDTDFFWjZUMmtSWlRlSXJOOWF0TXl0Q2VLR0FVRU56VnVTWkQweG5Z?=
 =?utf-8?B?Z3I1WXV1MlNuU2FDak16dGJhRG1xVFZnRWxWUmpkdmhGMmtlbUJZVjJQbWRV?=
 =?utf-8?B?WERMaXZYdXFUQWk1QVhWWW5ob09BYkpvSFVQMlg0VEp3UDg5dGtZS2hzQ01K?=
 =?utf-8?B?RDhvK3hlRElkN0dvaGRTQ2dnSTg5WEY2VDlqZ2FrL1hTMGZuSmtlMk5qMEZK?=
 =?utf-8?B?WDJrZSt3ZjMrYTlkbjNrLzA3bzZxR01ucTAvTVdjL3VkMmxBZXVlM1cwR1BV?=
 =?utf-8?B?Q0JOSTFwRzJJSlFPcmVLNThpK0Q2bThFQ2VQUU1UZlVVVWNORHZ4VDV2bUhC?=
 =?utf-8?B?S24wK09md2tKcm04dmxiQ2s3NVpHR1BMVE43d0oxRGR1cHhNZnJya3RVSlo4?=
 =?utf-8?B?WHNEUVZkT1dIZUw2RlZSSVpMNUVSY05UbWhBWE9kUVU5a3oyV1JHci9xbTNY?=
 =?utf-8?B?TGwrVHRwZnJEQTRWc3JTWUgwcU1penlOVkJtODNXYUVBRlU5Qk1HOXk0MTg5?=
 =?utf-8?B?SFkzczJJcDR4UmlvOEhib0dIRVp2MVpveWRQb0o2L3FSMXBoZkpqVGZRZU5T?=
 =?utf-8?B?ejRLTk1nQ2FKRG0xcndLalJxYk9iQzgwaDlOYlg5N3plL3BIS3l2U3VyZ3pC?=
 =?utf-8?B?YVo3MmlRcjlySk1ldFNKTHFwY1VPM21nWXlYUFpqM3NzYitiNmIrd1JOakxR?=
 =?utf-8?B?YjdGV0taQ1h2K2J2M0x6M3BCQi9sSlIybXhGOFRyQUVRL1dsYjN1OHBHWWZG?=
 =?utf-8?B?UkV0N3FVNjZVSmszL08vSHp1NkU3U0ttRm1uL1IwbHNOZFlETytwUDJGamhr?=
 =?utf-8?B?MUVqbm1ZeTYybVFraG1IOXZPR3B1OXBra0l1S0Z4UFQyQ0tsQ3VuSm4vWnRr?=
 =?utf-8?B?MWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d535230-eb0a-42a2-7c76-08db6def9ab1
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2023 22:26:46.4453
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EZdhSB7jymqU1QIA0EglxEdcYHLpE5mwTNEr8JdPsBQDux6mjToxJkTCaDEeIsirkL1k2+VOAOieN8TltNeGwGcc/IrsYLV+O1NhSr+0LtE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7036
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi James,

On 5/25/2023 11:02 AM, James Morse wrote:
> resctrl has one mutex that is taken by the architecture specific code,
> and the filesystem parts. The two interact via cpuhp, where the
> architecture code updates the domain list. Filesystem handlers that
> walk the domains list should not run concurrently with the cpuhp
> callback modifying the list.
> 
> Exposing a lock from the filesystem code means the interface is not
> cleanly defined, and creates the possibility of cross-architecture
> lock ordering headaches. The interaction only exists so that certain
> filesystem paths are serialised against cpu hotplug. The cpu hotplug
> code already has a mechanism to do this using cpus_read_lock().
> 
> MPAM's monitors have an overflow interrupt, so it needs to be possible
> to walk the domains list in irq context. RCU is ideal for this,
> but some paths need to be able to sleep to allocate memory.
> 
> Because resctrl_{on,off}line_cpu() take the rdtgroup_mutex as part
> of a cpuhp callback, cpus_read_lock() must always be taken first.
> rdtgroup_schemata_write() already does this.
> 
> Most of the filesystem code's domain list walkers are currently
> protected by the rdtgroup_mutex taken in rdtgroup_kn_lock_live().
> The exceptions are rdt_bit_usage_show() and the mon_config helpers
> which take the lock directly.
> 
> Make the domain list protected by RCU. An architecture-specific
> lock prevents concurrent writers. rdt_bit_usage_show() can

This does not sound right to me. The domain list belongs to resctrl,
the filesystem, having an architecture specific lock protect it does
not seem like the right thing to do. Could this instead be a resctrl
owned lock that is hidden behind helpers for the architecture
code to add domains?

Reinette