From: Catalin Marinas <catalin.marinas@arm.com>
To: Yury Norov <ynorov@caviumnetworks.com>
Cc: Pratyush Anand <panand@redhat.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] arm64: introduce mm_context_t flags
Date: Wed, 2 Aug 2017 17:39:01 +0100 [thread overview]
Message-ID: <20170802163900.66gnhogililb3uak@armageddon.cambridge.arm.com> (raw)
In-Reply-To: <20170731144825.31322-1-ynorov@caviumnetworks.com>
Hi Yury,
On Mon, Jul 31, 2017 at 05:48:25PM +0300, Yury Norov wrote:
> In patch 06beb72fbe23e ("arm64: introduce mm context flag to keep 32 bit task
> information") you introduce the field flags but use it only for a single flag -
> TIF_32BIT. It looks hacky to me for three reasons:
> - The flag is introduced for the case where it's impossible to get the thread
> info structure for the thread associated with mm. So thread_info flags (TIF)
> may also be unavailable at place. This is not the case for the only existing
> user of if - uprobes, but in general this approach requires to include thread
> headers in mm code, which may become unwanted dependency.
> - New flag, if it uses TIF bits, for consistency should for example set/clear
> TIF_32BIT_AARCH64 for ILP32 tasks. And to be completely consistent, with
> current approach we'd mirror thread_info flags to mm_context flags. And keep
> it syncronized.
> - If we start using TIF flags here, we cannot easily add new mm_context
> specific bits because they may mess with TIF ones.
>
> I think that this is not what was intended when you added new field in
> mm_context_t.
TIF_32BIT was handy at the time but it indeed denotes AArch32 user
task. For ILP32 we wouldn't need to set this bit since the instruction
set is A64 and uprobe should support it (though not sure anyone tried).
I noticed in your patch introducing binfmt_ilp32.c that SET_PERSONALITY
actually sets this flag in the mm context.
As with the TIF bits, the PERSONALITY macros become more complicated
with more bits to set/clear. Since we don't have any plans for other mm
context flags (independent of TIF), should we simply rename it to
thread_flags and just copy the thread_info flags:
current->mm->context.thread_flags = current_thread_info()->flags;
--
Catalin
next prev parent reply other threads:[~2017-08-02 16:39 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-31 14:48 Yury Norov
2017-08-01 3:40 ` Pratyush Anand
2017-08-01 11:37 ` Yury Norov
2017-08-02 16:39 ` Catalin Marinas [this message]
2017-08-02 17:29 ` Yury Norov
2017-08-04 17:38 ` Catalin Marinas
2017-08-04 21:49 ` Yury Norov
2017-08-07 11:08 ` Catalin Marinas
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170802163900.66gnhogililb3uak@armageddon.cambridge.arm.com \
--to=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=panand@redhat.com \
--cc=ynorov@caviumnetworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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
all inboxes | Powered by JetHome®