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

Powered by blists - more mailing lists