linux-kernel - Re: [PATCH v6 13/24] x86/resctrl: Queue mon_event

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]

Date:   Wed, 25 Oct 2023 18:56:47 +0100
From:   James Morse <james.morse@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Babu Moger <Babu.Moger@....com>,
        shameerali.kolothum.thodi@...wei.com,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        carl@...amperecomputing.com, lcherian@...vell.com,
        bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
        xingxin.hx@...nanolis.org, baolin.wang@...ux.alibaba.com,
        Jamie Iles <quic_jiles@...cinc.com>,
        Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
        dfustini@...libre.com, amitsinght@...vell.com
Subject: Re: [PATCH v6 13/24] x86/resctrl: Queue mon_event_read() instead of
 sending an IPI

Hi Reinette,

On 03/10/2023 22:17, Reinette Chatre wrote:
> On 9/14/2023 10:21 AM, James Morse wrote:
>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> index b44c487727d4..bd263b9a0abd 100644
>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/kernfs.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/slab.h>
>> +#include <linux/tick.h>
>>  #include "internal.h"
>>  
> 
> Please keep the empty line between groups of header files.

(in this case, adding one, but sure)


>> @@ -520,12 +521,24 @@ int rdtgroup_schemata_show(struct kernfs_open_file *of,
>>  	return ret;
>>  }
>>  
>> +static int smp_mon_event_count(void *arg)
>> +{
>> +	mon_event_count(arg);
>> +
>> +	return 0;
>> +}
>> +
>>  void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
>>  		    struct rdt_domain *d, struct rdtgroup *rdtgrp,
>>  		    int evtid, int first)
>>  {
>> +	int cpu;
>> +
>> +	/* When picking a CPU from cpu_mask, ensure it can't race with cpuhp */
> 
> This comment is not accurate at this point. It should accompany the code it applies to.
> 
>> +	lockdep_assert_held(&rdtgroup_mutex);

This refers to the d->cpu_mask calls further down this function. These are written to by
the cpuhp callbacks, rdtgroup_mutex is what prevents the cpuhp callback from running at
the same time as mon_event_read(). If that mutex weren't held, you could pick an offline CPU.

Patch 24 changes this to be lockdep_asser_cpus_held(), as the mutex is no longer used for
this purpose.

This got added here instead of patch-24 because I've added additional use of d->cpu_mask,
these things serve to document how that is safe. If you prefer I'll leave it unsaid here,
and add it with all the others in patch24.


Thanks,

James