Messages in this thread | | | Date | Mon, 15 Aug 2022 11:23:21 -0400 | Subject | Re: [PATCH v3 1/3] sched: Use user_cpus_ptr for saving user provided cpumask in sched_setaffinity() | From | Waiman Long <> |
| |
On 8/15/22 10:25, Peter Zijlstra wrote: > On Mon, Aug 15, 2022 at 09:52:27AM -0400, Waiman Long wrote: >> On 8/15/22 04:57, Peter Zijlstra wrote: >>> On Fri, Aug 12, 2022 at 04:39:27PM -0400, Waiman Long wrote: >>>> The user_cpus_ptr field is added by commit b90ca8badbd1 ("sched: >>>> Introduce task_struct::user_cpus_ptr to track requested affinity"). It >>>> is currently used only by arm64 arch due to possible asymmetric cpu >>>> setup. This patch extends its usage to save user provided cpumask when >>>> sched_setaffinity() is called for all arches. >>>> >>>> To preserve the existing arm64 use case, a new cpus_affinity_set flag is >>>> added to differentiate if user_cpus_ptr is set up by sched_setaffinity() >>>> or by force_compatible_cpus_allowed_ptr(). user_cpus_ptr >>>> set by sched_setaffinity() has priority and won't be >>>> overwritten by force_compatible_cpus_allowed_ptr() or >>>> relax_compatible_cpus_allowed_ptr(). >>> What why ?! The only possible case where >>> restrict_cpus_allowed_ptr() will now need that weird new state is when >>> the affinity has never been set before, in that case cpus_ptr should be >>> possible_mask. >> Since I don't have a full history for this particular patch series that add >> user_cpus_ptr, I am hesitant to change the current behavior for arm64 >> systems. However, given the statement that user_cpus_ptr is for tracking >> "requested affinity" which I assume is when user applications call >> sched_setaffinity(). It does make sense we may not really need this if >> sched_setaffinity() is never called. > So it comes from the asymmetric arm stuff, where only little cores can > still run arm32 code. This means that on those machines, 32bit code > needs to be contrained so a subset of CPUs. > > A direct consequence of that was that if you have any 32bit program in > your process hierarchy, you loose the big cores from you affinity mask. > > For some reason that wasn't popular.. Hence the save/restore of cpumasks.
I am aware of that part of the patch series.
>>> Please just make a single consistent rule and don't make weird corner >>> cases like this. >> I will take a closer look to try to simplify the rule here. > I think something like: > > mask = p->user_cpus_ptr; > if (!mask) > mask = &init_task.cpus_mask; > > // impose cpuset masks > > should 'just-work'.
I think that should work in relax_compatible_cpus_allowed_ptr().
Thanks, Longman
| |