Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-18 Thread Michal Hocko
On Wed 18-09-24 11:37:42, Frederic Weisbecker wrote:
> Le Tue, Sep 17, 2024 at 01:07:25PM +0200, Michal Hocko a écrit :
[...]
> > I am not objecting to patch per se. I am just not sure this is really
> > needed. It is great to have kernel threads bound to non isolated cpus by
> > default if they have node preferences. But as soon as somebody starts
> > offlining cpus excessively and make the initial cpumask empty then
> > select_fallback_rq sounds like the right thing to do.
> > 
> > Not my call though. I was just curious why this is needed and it seems
> > to me you are looking for some sort of correctness for broken setups.
> 
> It looks like it makes sense to explore that path. We still need the
> cpu up probe to reaffine when a suitable target comes up. But it seems
> the CPU down part can be handled by select_fallback_rq. I'll try that.

THanks! Btw. when you are looking at this, would it make sense to make
select_fallback_rq more cpu isolation aware as well? I mean using
housekeeping cpus before falling back to task_cpu_possible_mask?
-- 
Michal Hocko
SUSE Labs



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-18 Thread Frederic Weisbecker
Le Tue, Sep 17, 2024 at 01:07:25PM +0200, Michal Hocko a écrit :
> On Tue 17-09-24 12:34:51, Frederic Weisbecker wrote:
> > Le Tue, Sep 17, 2024 at 08:26:49AM +0200, Michal Hocko a écrit :
> > > On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
> > > > Kthreads attached to a preferred NUMA node for their task structure
> > > > allocation can also be assumed to run preferrably within that same node.
> > > > 
> > > > A more precise affinity is usually notified by calling
> > > > kthread_create_on_cpu() or kthread_bind[_mask]() before the first 
> > > > wakeup.
> > > > 
> > > > For the others, a default affinity to the node is desired and sometimes
> > > > implemented with more or less success when it comes to deal with hotplug
> > > > events and nohz_full / CPU Isolation interactions:
> > > > 
> > > > - kcompactd is affine to its node and handles hotplug but not CPU 
> > > > Isolation
> > > > - kswapd is affine to its node and ignores hotplug and CPU Isolation
> > > > - A bunch of drivers create their kthreads on a specific node and
> > > >   don't take care about affining further.
> > > > 
> > > > Handle that default node affinity preference at the generic level
> > > > instead, provided a kthread is created on an actual node and doesn't
> > > > apply any specific affinity such as a given CPU or a custom cpumask to
> > > > bind to before its first wake-up.
> > > 
> > > Makes sense.
> > > 
> > > > This generic handling is aware of CPU hotplug events and CPU isolation
> > > > such that:
> > > > 
> > > > * When a housekeeping CPU goes up and is part of the node of a given
> > > >   kthread, it is added to its applied affinity set (and
> > > >   possibly the default last resort online housekeeping set is removed
> > > >   from the set).
> > > > 
> > > > * When a housekeeping CPU goes down while it was part of the node of a
> > > >   kthread, it is removed from the kthread's applied
> > > >   affinity. The last resort is to affine the kthread to all online
> > > >   housekeeping CPUs.
> > > 
> > > But I am not really sure about this part. Sure it makes sense to set the
> > > affinity to exclude isolated CPUs but why do we care about hotplug
> > > events at all. Let's say we offline all cpus from a given node (or
> > > that all but isolated cpus are offline - is this even
> > > realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
> > > affinity in such a case? In other words how is that different from
> > > tasksetting an userspace task to a cpu that goes offline? We still do
> > > allow such a task to run, right? We just do not care about affinity
> > > anymore.
> > 
> > Suppose we have this artificial online set:
> > 
> > NODE 0 -> CPU 0
> > NODE 1 -> CPU 1
> > NODE 2 -> CPU 2
> > 
> > And we have nohz_full=1,2
> > 
> > So there is kswapd/2 that is affine to NODE 2 and thus CPU 2 for now.
> > 
> > Now CPU 2 goes offline. The scheduler migrates off all
> > tasks. select_fallback_rq() for kswapd/2 doesn't find a suitable CPU
> > to run to so it affines kswapd/2 to all remaining online CPUs (CPU 0, CPU 1)
> > (see the "No more Mr. Nice Guy" comment).
> > 
> > But CPU 1 is nohz_full, so kswapd/2 could run on that isolated CPU. Unless 
> > we
> > handle things before, like this patchset does.
> 
> But that is equally broken as before, no? CPU2 is isolated as well so it
> doesn't really make much of a difference.

Right. I should correct my example with nohz_full=1 only.

> 
> > And note that adding isolcpus=domain,1,2 or setting 1,2 as isolated
> > cpuset partition (like most isolated workloads should do) is not helping
> > here. And I'm not sure this last resort scheduler code is the right place
> > to handle isolated cpumasks.
> 
> Well, we would have the same situation with userspace tasks, no? Say I
> have taskset -p 2 (because I want bidning to node2) and that CPU2 goes
> offline. The task needs to be moved somewhere. And it would be last
> resort logic to do that unless I am missing anything. Why should kernel
> threads be any different?

Good point.

> 
> > So it looks necessary, unless I am missing something else?
> 
> I am not objecting to patch per se. I am just not sure this is really
> needed. It is great to have kernel threads bound to non isolated cpus by
> default if they have node preferences. But as soon as somebody starts
> offlining cpus excessively and make the initial cpumask empty then
> select_fallback_rq sounds like the right thing to do.
> 
> Not my call though. I was just curious why this is needed and it seems
> to me you are looking for some sort of correctness for broken setups.

It looks like it makes sense to explore that path. We still need the
cpu up probe to reaffine when a suitable target comes up. But it seems
the CPU down part can be handled by select_fallback_rq. I'll try that.

Thanks.

> -- 
> Michal Hocko
> SUSE Labs
> 



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-17 Thread Michal Hocko
On Tue 17-09-24 12:34:51, Frederic Weisbecker wrote:
> Le Tue, Sep 17, 2024 at 08:26:49AM +0200, Michal Hocko a écrit :
> > On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
> > > Kthreads attached to a preferred NUMA node for their task structure
> > > allocation can also be assumed to run preferrably within that same node.
> > > 
> > > A more precise affinity is usually notified by calling
> > > kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.
> > > 
> > > For the others, a default affinity to the node is desired and sometimes
> > > implemented with more or less success when it comes to deal with hotplug
> > > events and nohz_full / CPU Isolation interactions:
> > > 
> > > - kcompactd is affine to its node and handles hotplug but not CPU 
> > > Isolation
> > > - kswapd is affine to its node and ignores hotplug and CPU Isolation
> > > - A bunch of drivers create their kthreads on a specific node and
> > >   don't take care about affining further.
> > > 
> > > Handle that default node affinity preference at the generic level
> > > instead, provided a kthread is created on an actual node and doesn't
> > > apply any specific affinity such as a given CPU or a custom cpumask to
> > > bind to before its first wake-up.
> > 
> > Makes sense.
> > 
> > > This generic handling is aware of CPU hotplug events and CPU isolation
> > > such that:
> > > 
> > > * When a housekeeping CPU goes up and is part of the node of a given
> > >   kthread, it is added to its applied affinity set (and
> > >   possibly the default last resort online housekeeping set is removed
> > >   from the set).
> > > 
> > > * When a housekeeping CPU goes down while it was part of the node of a
> > >   kthread, it is removed from the kthread's applied
> > >   affinity. The last resort is to affine the kthread to all online
> > >   housekeeping CPUs.
> > 
> > But I am not really sure about this part. Sure it makes sense to set the
> > affinity to exclude isolated CPUs but why do we care about hotplug
> > events at all. Let's say we offline all cpus from a given node (or
> > that all but isolated cpus are offline - is this even
> > realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
> > affinity in such a case? In other words how is that different from
> > tasksetting an userspace task to a cpu that goes offline? We still do
> > allow such a task to run, right? We just do not care about affinity
> > anymore.
> 
> Suppose we have this artificial online set:
> 
> NODE 0 -> CPU 0
> NODE 1 -> CPU 1
> NODE 2 -> CPU 2
> 
> And we have nohz_full=1,2
> 
> So there is kswapd/2 that is affine to NODE 2 and thus CPU 2 for now.
> 
> Now CPU 2 goes offline. The scheduler migrates off all
> tasks. select_fallback_rq() for kswapd/2 doesn't find a suitable CPU
> to run to so it affines kswapd/2 to all remaining online CPUs (CPU 0, CPU 1)
> (see the "No more Mr. Nice Guy" comment).
> 
> But CPU 1 is nohz_full, so kswapd/2 could run on that isolated CPU. Unless we
> handle things before, like this patchset does.

But that is equally broken as before, no? CPU2 is isolated as well so it
doesn't really make much of a difference.

> And note that adding isolcpus=domain,1,2 or setting 1,2 as isolated
> cpuset partition (like most isolated workloads should do) is not helping
> here. And I'm not sure this last resort scheduler code is the right place
> to handle isolated cpumasks.

Well, we would have the same situation with userspace tasks, no? Say I
have taskset -p 2 (because I want bidning to node2) and that CPU2 goes
offline. The task needs to be moved somewhere. And it would be last
resort logic to do that unless I am missing anything. Why should kernel
threads be any different?

> So it looks necessary, unless I am missing something else?

I am not objecting to patch per se. I am just not sure this is really
needed. It is great to have kernel threads bound to non isolated cpus by
default if they have node preferences. But as soon as somebody starts
offlining cpus excessively and make the initial cpumask empty then
select_fallback_rq sounds like the right thing to do.

Not my call though. I was just curious why this is needed and it seems
to me you are looking for some sort of correctness for broken setups.
-- 
Michal Hocko
SUSE Labs



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-17 Thread Frederic Weisbecker
Le Tue, Sep 17, 2024 at 08:26:49AM +0200, Michal Hocko a écrit :
> On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
> > Kthreads attached to a preferred NUMA node for their task structure
> > allocation can also be assumed to run preferrably within that same node.
> > 
> > A more precise affinity is usually notified by calling
> > kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.
> > 
> > For the others, a default affinity to the node is desired and sometimes
> > implemented with more or less success when it comes to deal with hotplug
> > events and nohz_full / CPU Isolation interactions:
> > 
> > - kcompactd is affine to its node and handles hotplug but not CPU Isolation
> > - kswapd is affine to its node and ignores hotplug and CPU Isolation
> > - A bunch of drivers create their kthreads on a specific node and
> >   don't take care about affining further.
> > 
> > Handle that default node affinity preference at the generic level
> > instead, provided a kthread is created on an actual node and doesn't
> > apply any specific affinity such as a given CPU or a custom cpumask to
> > bind to before its first wake-up.
> 
> Makes sense.
> 
> > This generic handling is aware of CPU hotplug events and CPU isolation
> > such that:
> > 
> > * When a housekeeping CPU goes up and is part of the node of a given
> >   kthread, it is added to its applied affinity set (and
> >   possibly the default last resort online housekeeping set is removed
> >   from the set).
> > 
> > * When a housekeeping CPU goes down while it was part of the node of a
> >   kthread, it is removed from the kthread's applied
> >   affinity. The last resort is to affine the kthread to all online
> >   housekeeping CPUs.
> 
> But I am not really sure about this part. Sure it makes sense to set the
> affinity to exclude isolated CPUs but why do we care about hotplug
> events at all. Let's say we offline all cpus from a given node (or
> that all but isolated cpus are offline - is this even
> realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
> affinity in such a case? In other words how is that different from
> tasksetting an userspace task to a cpu that goes offline? We still do
> allow such a task to run, right? We just do not care about affinity
> anymore.

Suppose we have this artificial online set:

NODE 0 -> CPU 0
NODE 1 -> CPU 1
NODE 2 -> CPU 2

And we have nohz_full=1,2

So there is kswapd/2 that is affine to NODE 2 and thus CPU 2 for now.

Now CPU 2 goes offline. The scheduler migrates off all
tasks. select_fallback_rq() for kswapd/2 doesn't find a suitable CPU
to run to so it affines kswapd/2 to all remaining online CPUs (CPU 0, CPU 1)
(see the "No more Mr. Nice Guy" comment).

But CPU 1 is nohz_full, so kswapd/2 could run on that isolated CPU. Unless we
handle things before, like this patchset does.

And note that adding isolcpus=domain,1,2 or setting 1,2 as isolated
cpuset partition (like most isolated workloads should do) is not helping
here. And I'm not sure this last resort scheduler code is the right place
to handle isolated cpumasks.

So it looks necessary, unless I am missing something else?

And that is just for reaffine on CPU down. CPU up needs mirroring treatment
and also it must handle new CPUs freshly added to a node.

Thanks.

> -- 
> Michal Hocko
> SUSE Labs
> 



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-17 Thread Vlastimil Babka
On 9/17/24 9:05 AM, Michal Hocko wrote:
> On Tue 17-09-24 09:01:08, Vlastimil Babka wrote:
>> On 9/17/24 8:26 AM, Michal Hocko wrote:
>>> On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
 Kthreads attached to a preferred NUMA node for their task structure
 allocation can also be assumed to run preferrably within that same node.

 A more precise affinity is usually notified by calling
 kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.

 For the others, a default affinity to the node is desired and sometimes
 implemented with more or less success when it comes to deal with hotplug
 events and nohz_full / CPU Isolation interactions:

 - kcompactd is affine to its node and handles hotplug but not CPU Isolation
 - kswapd is affine to its node and ignores hotplug and CPU Isolation
 - A bunch of drivers create their kthreads on a specific node and
   don't take care about affining further.

 Handle that default node affinity preference at the generic level
 instead, provided a kthread is created on an actual node and doesn't
 apply any specific affinity such as a given CPU or a custom cpumask to
 bind to before its first wake-up.
>>>
>>> Makes sense.
>>>
 This generic handling is aware of CPU hotplug events and CPU isolation
 such that:

 * When a housekeeping CPU goes up and is part of the node of a given
   kthread, it is added to its applied affinity set (and
   possibly the default last resort online housekeeping set is removed
   from the set).

 * When a housekeeping CPU goes down while it was part of the node of a
   kthread, it is removed from the kthread's applied
   affinity. The last resort is to affine the kthread to all online
   housekeeping CPUs.
>>>
>>> But I am not really sure about this part. Sure it makes sense to set the
>>> affinity to exclude isolated CPUs but why do we care about hotplug
>>> events at all. Let's say we offline all cpus from a given node (or
>>> that all but isolated cpus are offline - is this even
>>> realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
>>> affinity in such a case? In other words how is that different from
>>> tasksetting an userspace task to a cpu that goes offline? We still do
>>> allow such a task to run, right? We just do not care about affinity
>>> anymore.
>>
>> AFAIU it handles better the situation where all houskeeping cpus from
>> the preferred node go down, then it affines to houskeeping cpus from any
>> node vs any cpu including isolated ones.
> 
> Doesn't that happen automagically? Or can it end up on a random
> isolated cpu?

Good question, perhaps it can and there's no automagic, as I see code like:

+   /* Make sure the kthread never gets re-affined globally */
+   set_cpus_allowed_ptr(current, 
housekeeping_cpumask(HK_TYPE_KTHREAD));
 



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-17 Thread Michal Hocko
On Tue 17-09-24 09:01:08, Vlastimil Babka wrote:
> On 9/17/24 8:26 AM, Michal Hocko wrote:
> > On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
> >> Kthreads attached to a preferred NUMA node for their task structure
> >> allocation can also be assumed to run preferrably within that same node.
> >>
> >> A more precise affinity is usually notified by calling
> >> kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.
> >>
> >> For the others, a default affinity to the node is desired and sometimes
> >> implemented with more or less success when it comes to deal with hotplug
> >> events and nohz_full / CPU Isolation interactions:
> >>
> >> - kcompactd is affine to its node and handles hotplug but not CPU Isolation
> >> - kswapd is affine to its node and ignores hotplug and CPU Isolation
> >> - A bunch of drivers create their kthreads on a specific node and
> >>   don't take care about affining further.
> >>
> >> Handle that default node affinity preference at the generic level
> >> instead, provided a kthread is created on an actual node and doesn't
> >> apply any specific affinity such as a given CPU or a custom cpumask to
> >> bind to before its first wake-up.
> > 
> > Makes sense.
> > 
> >> This generic handling is aware of CPU hotplug events and CPU isolation
> >> such that:
> >>
> >> * When a housekeeping CPU goes up and is part of the node of a given
> >>   kthread, it is added to its applied affinity set (and
> >>   possibly the default last resort online housekeeping set is removed
> >>   from the set).
> >>
> >> * When a housekeeping CPU goes down while it was part of the node of a
> >>   kthread, it is removed from the kthread's applied
> >>   affinity. The last resort is to affine the kthread to all online
> >>   housekeeping CPUs.
> > 
> > But I am not really sure about this part. Sure it makes sense to set the
> > affinity to exclude isolated CPUs but why do we care about hotplug
> > events at all. Let's say we offline all cpus from a given node (or
> > that all but isolated cpus are offline - is this even
> > realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
> > affinity in such a case? In other words how is that different from
> > tasksetting an userspace task to a cpu that goes offline? We still do
> > allow such a task to run, right? We just do not care about affinity
> > anymore.
> 
> AFAIU it handles better the situation where all houskeeping cpus from
> the preferred node go down, then it affines to houskeeping cpus from any
> node vs any cpu including isolated ones.

Doesn't that happen automagically? Or can it end up on a random
isolated cpu?

-- 
Michal Hocko
SUSE Labs



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-16 Thread Vlastimil Babka
On 9/17/24 8:26 AM, Michal Hocko wrote:
> On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
>> Kthreads attached to a preferred NUMA node for their task structure
>> allocation can also be assumed to run preferrably within that same node.
>>
>> A more precise affinity is usually notified by calling
>> kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.
>>
>> For the others, a default affinity to the node is desired and sometimes
>> implemented with more or less success when it comes to deal with hotplug
>> events and nohz_full / CPU Isolation interactions:
>>
>> - kcompactd is affine to its node and handles hotplug but not CPU Isolation
>> - kswapd is affine to its node and ignores hotplug and CPU Isolation
>> - A bunch of drivers create their kthreads on a specific node and
>>   don't take care about affining further.
>>
>> Handle that default node affinity preference at the generic level
>> instead, provided a kthread is created on an actual node and doesn't
>> apply any specific affinity such as a given CPU or a custom cpumask to
>> bind to before its first wake-up.
> 
> Makes sense.
> 
>> This generic handling is aware of CPU hotplug events and CPU isolation
>> such that:
>>
>> * When a housekeeping CPU goes up and is part of the node of a given
>>   kthread, it is added to its applied affinity set (and
>>   possibly the default last resort online housekeeping set is removed
>>   from the set).
>>
>> * When a housekeeping CPU goes down while it was part of the node of a
>>   kthread, it is removed from the kthread's applied
>>   affinity. The last resort is to affine the kthread to all online
>>   housekeeping CPUs.
> 
> But I am not really sure about this part. Sure it makes sense to set the
> affinity to exclude isolated CPUs but why do we care about hotplug
> events at all. Let's say we offline all cpus from a given node (or
> that all but isolated cpus are offline - is this even
> realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
> affinity in such a case? In other words how is that different from
> tasksetting an userspace task to a cpu that goes offline? We still do
> allow such a task to run, right? We just do not care about affinity
> anymore.

AFAIU it handles better the situation where all houskeeping cpus from
the preferred node go down, then it affines to houskeeping cpus from any
node vs any cpu including isolated ones.
Yes it's probably a scenario that's not recommendable, but someone might
do it anyway...



Re: [PATCH 12/19] kthread: Default affine kthread to its preferred NUMA node

2024-09-16 Thread Michal Hocko
On Tue 17-09-24 00:49:16, Frederic Weisbecker wrote:
> Kthreads attached to a preferred NUMA node for their task structure
> allocation can also be assumed to run preferrably within that same node.
> 
> A more precise affinity is usually notified by calling
> kthread_create_on_cpu() or kthread_bind[_mask]() before the first wakeup.
> 
> For the others, a default affinity to the node is desired and sometimes
> implemented with more or less success when it comes to deal with hotplug
> events and nohz_full / CPU Isolation interactions:
> 
> - kcompactd is affine to its node and handles hotplug but not CPU Isolation
> - kswapd is affine to its node and ignores hotplug and CPU Isolation
> - A bunch of drivers create their kthreads on a specific node and
>   don't take care about affining further.
> 
> Handle that default node affinity preference at the generic level
> instead, provided a kthread is created on an actual node and doesn't
> apply any specific affinity such as a given CPU or a custom cpumask to
> bind to before its first wake-up.

Makes sense.

> This generic handling is aware of CPU hotplug events and CPU isolation
> such that:
> 
> * When a housekeeping CPU goes up and is part of the node of a given
>   kthread, it is added to its applied affinity set (and
>   possibly the default last resort online housekeeping set is removed
>   from the set).
> 
> * When a housekeeping CPU goes down while it was part of the node of a
>   kthread, it is removed from the kthread's applied
>   affinity. The last resort is to affine the kthread to all online
>   housekeeping CPUs.

But I am not really sure about this part. Sure it makes sense to set the
affinity to exclude isolated CPUs but why do we care about hotplug
events at all. Let's say we offline all cpus from a given node (or
that all but isolated cpus are offline - is this even
realistic/reasonable usecase?). Wouldn't scheduler ignore the kthread's
affinity in such a case? In other words how is that different from
tasksetting an userspace task to a cpu that goes offline? We still do
allow such a task to run, right? We just do not care about affinity
anymore.
-- 
Michal Hocko
SUSE Labs