linux-kernel - Re: [PATCH 2/2] x86/resctrl: Don't workqueue local event counter reads

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: <5ec7a1c1-43b1-4c18-9ba5-5cf4c42ba3f1@intel.com>
Date: Tue, 5 Nov 2024 16:13:27 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>, Peter Newman <peternewman@...gle.com>,
	"Yu, Fenghua" <fenghua.yu@...el.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"x86@...nel.org" <x86@...nel.org>, "H . Peter Anvin" <hpa@...or.com>, "Babu
 Moger" <babu.moger@....com>, James Morse <james.morse@....com>, "Martin
 Kletzander" <nert.pinx@...il.com>, Shaopeng Tan <tan.shaopeng@...itsu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Eranian,
 Stephane" <eranian@...gle.com>
Subject: Re: [PATCH 2/2] x86/resctrl: Don't workqueue local event counter
 reads

Hi Tony,

On 11/5/24 3:39 PM, Luck, Tony wrote:
>> I think this change already undoes the motivation for 09909e098113
>> ("x86/resctrl: Queue mon_event_read() instead of sending an IPI")? As you mention in
>> changelog the goal of that work was to enable resctrl_arch_rmid_read() to sleep.
>> This change will call resctrl_arch_rmid_read() with preemption disabled if
>> it happens to be called on CPU in monitoring domain. Would that not cause
>> MPAM monitor count reads from CPU in domain to be a bug?
>>
>> Could you please try out this patch with CONFIG_DEBUG_ATOMIC_SLEEP=y?
> 
> How is this all going to look after the split into fs/resctrl and arch/* ?

Unclear to me at this point. Peter exposed an issue with current implementation
and this needs to be fixed. Since this involves preparatory work that impacts
systems currently supported we could also consider reverting to original behavior
and go back to drawing board with the preparatory work.

> Is the file system code going to have implementation choices that prevent
> performance sensitive users like Peter from optimizing monitor event
> reads by binding the monitor process to a CPU in the right domain
> to avoid IPI?

Apologies for not clearly stating it but I do agree that there is an issue
that needs to be fixed.

My response was not intended to be interpreted as a NACK but instead an attempt
to engage in discussion by pointing out that the proposed fix may not be ideal.

I tried out my own suggestion and indeed when just trying to mount resctrl
on x86 with this patch applied results in:
	BUG: scheduling while atomic

I do not object to optimizing monitor event reads but the proposed fix
is not appropriate in its current form.

Reinette