archive mirror
 help / color / mirror / Atom feed
From: Qais Yousef <>
To: Will Deacon <>
	Catalin Marinas <>,
	Marc Zyngier <>,
	Greg Kroah-Hartman <>,
	Peter Zijlstra <>,
	Morten Rasmussen <>,
	Suren Baghdasaryan <>,
	Quentin Perret <>, Tejun Heo <>,
	Li Zefan <>,
	Johannes Weiner <>,
	Ingo Molnar <>,
	Juri Lelli <>,
	Vincent Guittot <>,
Subject: Re: [PATCH v5 00/15] An alternative series for asymmetric AArch32 systems
Date: Wed, 16 Dec 2020 11:16:46 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Will

On 12/08/20 13:28, Will Deacon wrote:
> Hi all,
> Christmas has come early: it's time for version five of these patches
> which have previously appeared here:
>   v1:
>   v2:
>   v3:
>   v4:
> and which started life as a reimplementation of some patches from Qais:
> There's also now a nice writeup on LWN:
> and rumours of a feature film are doing the rounds.
> [subscriber-only, but if you're reading this then you should really
>  subscribe.]
> The aim of this series is to allow 32-bit ARM applications to run on
> arm64 SoCs where not all of the CPUs support the 32-bit instruction set.
> Unfortunately, such SoCs are real and will continue to be productised
> over the next few years at least. I can assure you that I'm not just
> doing this for fun.
> Changes in v5 include:
>   * Teach cpuset_cpus_allowed() about task_cpu_possible_mask() so that
>     we can avoid returning incompatible CPUs for a given task. This
>     means that sched_setaffinity() can be used with larger masks (like
>     the online mask) from userspace and also allows us to take into
>     account the cpuset hierarchy when forcefully overriding the affinity
>     for a task on execve().
>   * Honour task_cpu_possible_mask() when attaching a task to a cpuset,
>     so that the resulting affinity mask does not contain any incompatible
>     CPUs (since it would be rejected by set_cpus_allowed_ptr() otherwise).
>   * Moved overriding of the affinity mask into the scheduler core rather
>     than munge affinity masks directly in the architecture backend.
>   * Extended comments and documentation.
>   * Some renaming and cosmetic changes.
> I'm pretty happy with this now, although it still needs review and will
> require rebasing to play nicely with the SCA changes in -next.

I still have concerns about the cpuset v1 handling. Specifically:

	1. Attaching a 32bit task to 64bit only cpuset is allowed.

	   I think the right behavior here is to prevent that as the
	   intersection will appear as offline cpus for the 32bit tasks. So it
	   shouldn't be allowed to move there.

	2. Modifying cpuset.cpus could result with empty set for 32bit tasks.

	   It is a variation of the above, it's just the cpuset transforms into
	   64bit only after we attach.

	   I think the right behavior here is to move the 32bit tasks to the
	   nearest ancestor like we do when all cpuset.cpus are hotplugged out.

	   We could too return an error if the new set will result an empty set
	   for the 32bit tasks. In a similar manner to how it fails if you
	   write a cpu that is offline.

	3. If a 64bit task belongs to 64bit-only-cpuset execs a 32bit binary,
	   the 32 tasks will inherit the cgroup setting.

	   Like above, we should move this to the nearest ancestor.

I was worried if in a hierarchy the parent cpuset.cpus is modified such that
the childs no longer have a valid cpu for 32bit tasks. But I checked for v1 and
this isn't a problem. You'll get an error if you try to change it in a way that
ends up with an empty cpuset.

I played with v2, and the model allows tasks to remain attached even if cpus
are hotplugged, or cpusets.cpus is modified in such a way we end up with an
empty cpuset. So I think breaking the affinity of the cpuset for v2 is okay.

To simplify the problem for v1, we could say that asym ISA tasks can only live
in the root cpuset for v1. This will simplify the solution too since we will
only need to ensure that these tasks are moved to the root group on exec and
block any future move to anything else. Of course this dictates that such
systems must use cpuset v2 if they care. Not a terrible restriction IMO.

I hacked a patch to fix the exec scenario and it was easy to do. I just need to
block clone3 (cgroup_post_fork()) and task_can_attach() from allowing these
tasks from moving anywhere else.


Qais Yousef

  parent reply	other threads:[~2020-12-16 11:17 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08 13:28 [PATCH v5 00/15] An alternative series for asymmetric AArch32 systems Will Deacon
2020-12-08 13:28 ` [PATCH v5 01/15] arm64: cpuinfo: Split AArch32 registers out into a separate struct Will Deacon
2020-12-08 13:28 ` [PATCH v5 02/15] arm64: Allow mismatched 32-bit EL0 support Will Deacon
2020-12-08 13:28 ` [PATCH v5 03/15] KVM: arm64: Kill 32-bit vCPUs on systems with mismatched " Will Deacon
2020-12-08 13:28 ` [PATCH v5 04/15] arm64: Kill 32-bit applications scheduled on 64-bit-only CPUs Will Deacon
2020-12-08 13:28 ` [PATCH v5 05/15] arm64: Advertise CPUs capable of running 32-bit applications in sysfs Will Deacon
2020-12-08 13:28 ` [PATCH v5 06/15] sched: Introduce task_cpu_possible_mask() to limit fallback rq selection Will Deacon
2020-12-08 13:28 ` [PATCH v5 07/15] cpuset: Don't use the cpu_possible_mask as a last resort for cgroup v1 Will Deacon
2020-12-17 12:15   ` Qais Yousef
2020-12-17 13:44     ` Peter Zijlstra
2020-12-17 14:59       ` Will Deacon
2020-12-17 15:00       ` Qais Yousef
2020-12-08 13:28 ` [PATCH v5 08/15] cpuset: Honour task_cpu_possible_mask() in guarantee_online_cpus() Will Deacon
2020-12-28  3:54   ` Suren Baghdasaryan
2020-12-08 13:28 ` [PATCH v5 09/15] sched: Reject CPU affinity changes based on task_cpu_possible_mask() Will Deacon
2020-12-08 13:28 ` [PATCH v5 10/15] sched: Introduce force_compatible_cpus_allowed_ptr() to limit CPU affinity Will Deacon
2020-12-28  4:29   ` Suren Baghdasaryan
2020-12-08 13:28 ` [PATCH v5 11/15] arm64: Implement task_cpu_possible_mask() Will Deacon
2020-12-08 13:28 ` [PATCH v5 12/15] arm64: exec: Adjust affinity for compat tasks with mismatched 32-bit EL0 Will Deacon
2020-12-08 13:28 ` [PATCH v5 13/15] arm64: Prevent offlining first CPU with 32-bit EL0 on mismatched system Will Deacon
2020-12-08 13:28 ` [PATCH v5 14/15] arm64: Hook up cmdline parameter to allow mismatched 32-bit EL0 Will Deacon
2020-12-08 13:28 ` [PATCH v5 15/15] arm64: Remove logic to kill 32-bit tasks on 64-bit-only cores Will Deacon
2020-12-15 17:36 ` [PATCH v5 00/15] An alternative series for asymmetric AArch32 systems Peter Zijlstra
2020-12-15 18:50   ` Will Deacon
2020-12-17 10:55     ` Peter Zijlstra
2020-12-16 11:16 ` Qais Yousef [this message]
2020-12-16 14:14   ` Will Deacon
2020-12-16 16:48     ` Qais Yousef
2020-12-16 18:21       ` Suren Baghdasaryan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).