Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

From: Szabolcs Nagy <szabolcs.nagy@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Peter Collingbourne <pcc@google.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Tejas Belagod <Tejas.Belagod@arm.com>
Subject: Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis
Date: Fri, 25 Jun 2021 15:14:40 +0100	[thread overview]
Message-ID: <20210625141438.GK13058@arm.com> (raw)
In-Reply-To: <20210625123959.GB3170@willie-the-truck>

The 06/25/2021 13:39, Will Deacon wrote:
> On Fri, Jun 25, 2021 at 01:01:37PM +0100, Catalin Marinas wrote:
> > Thanks, that's useful. I guess since the _MTAG_ENABLE tunable is not
> > ABI, the user app can't rely on what the glibc has configured. Arguably,
> > since it's driven from outside the application (env), we could say the
> > same for sysfs, though for the glibc case, the user app is still be able
> > to override it before the first thread is created (or per-thread). I
> > assume glibc only issues the prctl() once, not for every new thread.

note: in the end the tunable is like

GLIBC_TUNABLES=glibc.mem.tagging=3 ./exe

not _MTAG_ENABLE.

and yes the setting comes from outside and glibc
calls prctl once.

> > So we can document that the mode requested by the app is an indication,
> > the system may change it to another value (and back-port documentation
> > to 5.10). If we get a request from developers to honour a specific mode,
> > we can add a new PR_MTE_TCF_EXACT bit or something but it's not
> > essential we do it now.
> > 
> > So if we allow the kernel to change the user requested mode (via sysfs),
> > I think we still have two more issues to clarify:
> > 
> > 1. Do we allow only "upgrade" (for some meaning of this) or sysfs can
> >    downgrade to a less strict mode. I'd go for upgrade here to a
> >    stricter check as in Peter's patch.
> > 
> > 2. Should the sysfs upgrade the PR_MTE_TCF_NONE? _MTAG_ENABLE does that,
> >    so I'd say yes.
> > 
> > Any other thoughts are welcome.
> 
> As I mentioned before, I think the sysfs interface should offer:
> 
> 	"task"	: Honour whatever the task has asked for (default)
> 	"async" : Force async on this CPU
> 	"sync"  : Force sync on this CPU
> 
> I don't think we should upgrade PR_MTE_TCF_NONE unless we also have a "none"
> option in here. I originally suggested that, but in hindsight it feels like
> a bad idea because a task could SIGILL on migration. So what we're saying is
> that PR_MTE_TCF_SYNC and PR_MTE_TCF_ASYNC will always enable MTE on success,
> but the reporting mode is a hint.
> 
> I don't think upgrade/downgrade makes a lot of sense given that the sysfs
> controls can be changed at any point in time. It should just be an override.
> 
> This means that we can force async for CPUs where sync mode is horribly
> slow, whilst honouring the task's request on CPUs which are better
> implemented.

i think a user should be able to ask for sync check
mode for a process and get an error if that's not
possible.

at least this is the semantics that makes sense in
glibc. i think it's very confusing if somebody
explicitly asks for sync checks to debug something
but then gets useless diagnostics because somebody
else tried to second guess their performance tradeoff
preferences. (if sync check is too slow on a cpu
then the user can taskset to a cpu that's not slow
or just use other debugging method, silent override
sounds bad.)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel