Add the notifier chain to notify the cpu SMT status changes
Signed-off-by: Wenyou Yang <[email protected]>
---
include/linux/cpu.h | 5 +++++
kernel/cpu.c | 10 +++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 314802f98b9d..9a842317fe2d 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -213,6 +213,11 @@ enum cpuhp_smt_control {
CPU_SMT_NOT_IMPLEMENTED,
};
+enum cpuhp_smt_status {
+ SMT_ENABLED,
+ SMT_DISABLED,
+};
+
#if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT)
extern enum cpuhp_smt_control cpu_smt_control;
extern void cpu_smt_disable(bool force);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6c0a92ca6bb5..1af66a3ffd99 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -89,6 +89,9 @@ static DEFINE_PER_CPU(struct cpuhp_cpu_state, cpuhp_state) = {
cpumask_t cpus_booted_once_mask;
#endif
+RAW_NOTIFIER_HEAD(smt_notifier_head);
+EXPORT_SYMBOL(smt_notifier_head);
+
#if defined(CONFIG_LOCKDEP) && defined(CONFIG_SMP)
static struct lockdep_map cpuhp_state_up_map =
STATIC_LOCKDEP_MAP_INIT("cpuhp_state-up", &cpuhp_state_up_map);
@@ -2281,8 +2284,10 @@ int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
*/
cpuhp_offline_cpu_device(cpu);
}
- if (!ret)
+ if (!ret) {
cpu_smt_control = ctrlval;
+ raw_notifier_call_chain(&smt_notifier_head, SMT_DISABLED, NULL);
+ }
cpu_maps_update_done();
return ret;
}
@@ -2303,6 +2308,9 @@ int cpuhp_smt_enable(void)
/* See comment in cpuhp_smt_disable() */
cpuhp_online_cpu_device(cpu);
}
+ if (!ret)
+ raw_notifier_call_chain(&smt_notifier_head, SMT_ENABLED, NULL);
+
cpu_maps_update_done();
return ret;
}
--
2.39.2
On Wed, Mar 29, 2023 at 09:51:48AM +0800, Wenyou Yang wrote:
> Add the notifier chain to notify the cpu SMT status changes
>
Why!?!? What's the purpose of all this? IIRC this doesn't trigger if you
manually disable all the siblings. And because you didn't tell us why
you need this I can't tell you if that matters or not :/
[AMD Official Use Only - General]
Hi Peter,
Thank you for your review.
The purpose of the patch set is to improve the performance when playing game for some AMD APUs with SMT enabled/disabled.
When change the SMT state on the fly through " echo on/off > /sys/devices/system/cpu/smt/control", the kernel needs to send a message to notify PMFW to adjust a variable's value, which impacts the performance.
Best Regards,
Wenyou
> -----Original Message-----
> From: Peter Zijlstra <[email protected]>
> Sent: Wednesday, March 29, 2023 3:10 PM
> To: Yang, WenYou <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>; Quan, Evan
> <[email protected]>; Limonciello, Mario <[email protected]>;
> [email protected]; [email protected]; Phillips, Kim <[email protected]>;
> [email protected]; Yuan, Perry <[email protected]>; Liang, Richard qi
> <[email protected]>; Li, Ying <[email protected]>; Liu, Kun
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v3 1/2] cpu/smt: add a notifier to notify the SMT changes
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Mar 29, 2023 at 09:51:48AM +0800, Wenyou Yang wrote:
> > Add the notifier chain to notify the cpu SMT status changes
> >
>
> Why!?!? What's the purpose of all this? IIRC this doesn't trigger if you manually
> disable all the siblings. And because you didn't tell us why you need this I can't
> tell you if that matters or not :/
On Wed, Mar 29, 2023 at 07:23:29AM +0000, Yang, WenYou wrote:
> [AMD Official Use Only - General]
^^^ that has no business being in a public email.
> Hi Peter,
>
> Thank you for your review.
>
> The purpose of the patch set is to improve the performance when playing game for some AMD APUs with SMT enabled/disabled.
>
> When change the SMT state on the fly through " echo on/off > /sys/devices/system/cpu/smt/control", the kernel needs to send a message to notify PMFW to adjust a variable's value, which impacts the performance.
When top posting I normally ignore the email. When not wrapping email I
typically get cranky. You 'win' *3* 'I cannot use email' trophies in a
singly try. Surely AMD has a HOWTO somewhere you can read?
So what do you want to have happen when someone goes and manually
offlines all the SMT siblings using /sys/devices/system/cpu/cpu*/online
?
I'm thinking that wants the same PMFW (whatever the heck that is)
notification change done, right?
If the answer is "yes", then your patch does not meet the goals and is
inadequate.
[AMD Official Use Only - General]
> -----Original Message-----
> From: Peter Zijlstra <[email protected]>
> Sent: Wednesday, March 29, 2023 4:50 PM
> To: Yang, WenYou <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>; Quan, Evan
> <[email protected]>; Limonciello, Mario <[email protected]>;
> [email protected]; [email protected]; Phillips, Kim <[email protected]>;
> [email protected]; Yuan, Perry <[email protected]>; Liang, Richard qi
> <[email protected]>; Li, Ying <[email protected]>; Liu, Kun
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v3 1/2] cpu/smt: add a notifier to notify the SMT changes
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Mar 29, 2023 at 07:23:29AM +0000, Yang, WenYou wrote:
> > [AMD Official Use Only - General]
>
> ^^^ that has no business being in a public email.
>
> > Hi Peter,
> >
> > Thank you for your review.
> >
> > The purpose of the patch set is to improve the performance when playing
> game for some AMD APUs with SMT enabled/disabled.
> >
> > When change the SMT state on the fly through " echo on/off >
> /sys/devices/system/cpu/smt/control", the kernel needs to send a message to
> notify PMFW to adjust a variable's value, which impacts the performance.
>
> When top posting I normally ignore the email. When not wrapping email I
> typically get cranky. You 'win' *3* 'I cannot use email' trophies in a singly try.
> Surely AMD has a HOWTO somewhere you can read?
Yes. It is my fault. Sorry.
>
> So what do you want to have happen when someone goes and manually offlines
> all the SMT siblings using /sys/devices/system/cpu/cpu*/online
> ?
I don't consider this situation. Any suggestions will be deeply appreciated.
>
> I'm thinking that wants the same PMFW (whatever the heck that is) notification
> change done, right?
>
> If the answer is "yes", then your patch does not meet the goals and is
> inadequate.
Yes. Need to improve.
Thank you.
Wenyou
[AMD Official Use Only - General]
> -----Original Message-----
> From: Yang, WenYou
> Sent: Wednesday, March 29, 2023 5:43 PM
> To: Peter Zijlstra <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>; Quan, Evan
> <[email protected]>; Limonciello, Mario <[email protected]>;
> [email protected]; [email protected]; Phillips, Kim <[email protected]>;
> [email protected]; Yuan, Perry <[email protected]>; Liang, Richard qi
> <[email protected]>; Li, Ying <[email protected]>; Liu, Kun
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: RE: [PATCH v3 1/2] cpu/smt: add a notifier to notify the SMT changes
>
> [AMD Official Use Only - General]
>
>
>
> > -----Original Message-----
> > From: Peter Zijlstra <[email protected]>
> > Sent: Wednesday, March 29, 2023 4:50 PM
> > To: Yang, WenYou <[email protected]>
> > Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> > <[email protected]>; Pan, Xinhui <[email protected]>; Quan,
> > Evan <[email protected]>; Limonciello, Mario
> > <[email protected]>; [email protected]; [email protected];
> > Phillips, Kim <[email protected]>; [email protected]; Yuan, Perry
> > <[email protected]>; Liang, Richard qi <[email protected]>; Li,
> > Ying <[email protected]>; Liu, Kun <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH v3 1/2] cpu/smt: add a notifier to notify the SMT
> > changes
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On Wed, Mar 29, 2023 at 07:23:29AM +0000, Yang, WenYou wrote:
> > > [AMD Official Use Only - General]
> >
> > ^^^ that has no business being in a public email.
> >
> > > Hi Peter,
> > >
> > > Thank you for your review.
> > >
> > > The purpose of the patch set is to improve the performance when
> > > playing
> > game for some AMD APUs with SMT enabled/disabled.
> > >
> > > When change the SMT state on the fly through " echo on/off >
> > /sys/devices/system/cpu/smt/control", the kernel needs to send a
> > message to notify PMFW to adjust a variable's value, which impacts the
> performance.
> >
> > When top posting I normally ignore the email. When not wrapping email
> > I typically get cranky. You 'win' *3* 'I cannot use email' trophies in a singly try.
> > Surely AMD has a HOWTO somewhere you can read?
>
> Yes. It is my fault. Sorry.
>
> >
> > So what do you want to have happen when someone goes and manually
> > offlines all the SMT siblings using
> > /sys/devices/system/cpu/cpu*/online
> > ?
>
> I don't consider this situation. Any suggestions will be deeply appreciated.
Hi Peter,
I don't find a good method to handle this situation.
Yes, manually offlining all the SMT sibling will get the same result of SMT disabling on the fly.
Actually, the normal way to enable/disable SMT on the fly is to echo on/off > /sys/device/system/cpu/smt/control
What are your opinions?
Best Regards,
Wenyou
>
> >
> > I'm thinking that wants the same PMFW (whatever the heck that is)
> > notification change done, right?
> >
> > If the answer is "yes", then your patch does not meet the goals and is
> > inadequate.
>
> Yes. Need to improve.
>
> Thank you.
> Wenyou
On Fri, Mar 31 2023 at 05:49, WenYou Yang wrote:
<SNIP>
Removing pointlessly copied mail headers. Please fix your email
client
</SNIP>
>> >
>> > So what do you want to have happen when someone goes and manually
>> > offlines all the SMT siblings using
>> > /sys/devices/system/cpu/cpu*/online
>> > ?
>>
>> I don't consider this situation. Any suggestions will be deeply appreciated.
>
> Hi Peter,
>
> I don't find a good method to handle this situation.
> Yes, manually offlining all the SMT sibling will get the same result of SMT disabling on the fly.
>
> Actually, the normal way to enable/disable SMT on the fly is to echo on/off > /sys/device/system/cpu/smt/control
That's the most convenient way, right.
But why do we need a kernel notifier for this, if you can do the same
with a sysfs knob for your driver?
Then user space can fiddle with SMT control in sysfs and afterwards tell
the driver that it should reconfigure.
That makes a ton more sense than this random notifier.
Thanks,
tglx