From: Michael Neuling <mikey@neuling.org>
To: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>
Cc: stewart@linux.vnet.ibm.com, linuxppc-dev@ozlabs.org,
linux-kernel@vger.kernel.org, apopple@au1.ibm.com,
oohall@gmail.com
Subject: Re: [PATCH v6 14/17] powerpc: Add support for setting SPRN_TIDR
Date: Mon, 14 Aug 2017 17:02:11 +1000 [thread overview]
Message-ID: <1502694131.9844.7.camel@neuling.org> (raw)
In-Reply-To: <1502233622-9330-15-git-send-email-sukadev@linux.vnet.ibm.com>
On Tue, 2017-08-08 at 16:06 -0700, Sukadev Bhattiprolu wrote:
> We need the SPRN_TIDR to bet set for use with fast thread-wakeup
> (core-to-core wakeup).=C2=A0=C2=A0Each thread in a process needs to have =
a
> unique id within the process but as explained below, for now, we
> assign globally unique thread ids to all threads in the system.
>=20
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> =C2=A0arch/powerpc/include/asm/processor.h |=C2=A0=C2=A04 ++
> =C2=A0arch/powerpc/kernel/process.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0| 74
> ++++++++++++++++++++++++++++++++++++
> =C2=A02 files changed, 78 insertions(+)
>=20
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index fab7ff8..bf6ba63 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -232,6 +232,10 @@ struct debug_reg {
> =C2=A0struct thread_struct {
> =C2=A0 unsigned long ksp; /* Kernel stack pointer */
> =C2=A0
> +#ifdef CONFIG_PPC_VAS
I'm tempted to have this always, or a new feature CONFIG_PPC_TID that's PPC=
_VAS
depends on.
> + unsigned long tidr;
> +#endif
> +
> =C2=A0#ifdef CONFIG_PPC64
> =C2=A0 unsigned long ksp_vsid;
> =C2=A0#endif
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index 9f3e2c9..6123859 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1213,6 +1213,16 @@ struct task_struct *__switch_to(struct task_struct
> *prev,
> =C2=A0 hard_irq_disable();
> =C2=A0 }
> =C2=A0
> +#ifdef CONFIG_PPC_VAS
> + mtspr(SPRN_TIDR, new->thread.tidr);
how much does this hurt our context_switch benchmark in
tools/testing/selftests/powerpc/benchmarks/context_switch.c ?
Also you need an CPU_FTR_ARCH_300 test here (and elsewhere)
> +#endif
> + /*
> + =C2=A0* We can't take a PMU exception inside _switch() since there is a
> + =C2=A0* window where the kernel stack SLB and the kernel stack are out
> + =C2=A0* of sync. Hard disable here.
> + =C2=A0*/
> + hard_irq_disable();
> +
What is this?
> =C2=A0 /*
> =C2=A0 =C2=A0* Call restore_sprs() before calling _switch(). If we move i=
t after
> =C2=A0 =C2=A0* _switch() then we miss out on calling it for new tasks. Th=
e reason
> @@ -1449,9 +1459,70 @@ void flush_thread(void)
> =C2=A0#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> =C2=A0}
> =C2=A0
> +#ifdef CONFIG_PPC_VAS
> +static DEFINE_SPINLOCK(vas_thread_id_lock);
> +static DEFINE_IDA(vas_thread_ida);
This IDA be per process, not global.
> +
> +/*
> + * We need to assign an unique thread id to each thread in a process. Th=
is
> + * thread id is intended to be used with the Fast Thread-wakeup (aka Cor=
e-
> + * to-core wakeup) mechanism being implemented on top of Virtual Acceler=
ator
> + * Switchboard (VAS).
> + *
> + * To get a unique thread-id per process we could simply use task_pid_nr=
()
> + * but the problem is that task_pid_nr() is not yet available for the th=
read
> + * when copy_thread() is called. Fixing that would require changing more
> + * intrusive arch-neutral code in code path in copy_process()?.
> + *
> + * Further, to assign unique thread ids within each process, we need an
> + * atomic field (or an IDR) in task_struct, which again intrudes into th=
e
> + * arch-neutral code.
Really?
> + * So try to assign globally unique thraed ids for now.
Yuck!
> + */
> +static int assign_thread_id(void)
> +{
> + int index;
> + int err;
> +
> +again:
> + if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
> + return -ENOMEM;
> +
> + spin_lock(&vas_thread_id_lock);
> + err =3D ida_get_new_above(&vas_thread_ida, 1, &index);
We can't use 0 or 1?
> + spin_unlock(&vas_thread_id_lock);
> +
> + if (err =3D=3D -EAGAIN)
> + goto again;
> + else if (err)
> + return err;
> +
> + if (index > MAX_USER_CONTEXT) {
> + spin_lock(&vas_thread_id_lock);
> + ida_remove(&vas_thread_ida, index);
> + spin_unlock(&vas_thread_id_lock);
> + return -ENOMEM;
> + }
> +
> + return index;
> +}
> +
> +static void free_thread_id(int id)
> +{
> + spin_lock(&vas_thread_id_lock);
> + ida_remove(&vas_thread_ida, id);
> + spin_unlock(&vas_thread_id_lock);
> +}
> +#endif /* CONFIG_PPC_VAS */
> +
> +
> =C2=A0void
> =C2=A0release_thread(struct task_struct *t)
> =C2=A0{
> +#ifdef CONFIG_PPC_VAS
> + free_thread_id(t->thread.tidr);
> +#endif
Can you restructure this to avoid the #ifdef ugliness
> =C2=A0}
> =C2=A0
> =C2=A0/*
> @@ -1587,6 +1658,9 @@ int copy_thread(unsigned long clone_flags, unsigned=
long
> usp,
> =C2=A0#endif
> =C2=A0
> =C2=A0 setup_ksp_vsid(p, sp);
> +#ifdef CONFIG_PPC_VAS
> + p->thread.tidr =3D assign_thread_id();
> +#endif
Same here...=20
> =C2=A0
> =C2=A0#ifdef CONFIG_PPC64=C2=A0
> =C2=A0 if (cpu_has_feature(CPU_FTR_DSCR)) {
next prev parent reply other threads:[~2017-08-14 7:02 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-08 23:06 [PATCH v6 00/17] powerpc/vas: Enable VAS Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 01/17] powerpc/vas: Define macros, register fields and structures Sukadev Bhattiprolu
2017-08-14 5:21 ` Michael Ellerman
2017-08-14 6:05 ` Nicholas Piggin
2017-08-14 11:00 ` Michael Ellerman
2017-08-14 19:14 ` Sukadev Bhattiprolu
2017-08-16 12:07 ` Michael Ellerman
2017-08-16 23:07 ` Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 02/17] powerpc/vas: Move GET_FIELD/SET_FIELD to vas.h Sukadev Bhattiprolu
2017-08-14 7:26 ` Michael Ellerman
2017-08-08 23:06 ` [PATCH v6 03/17] powerpc/vas: Define vas_init() and vas_exit() Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 04/17] powerpc/vas: Define helpers to access MMIO regions Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 05/17] powerpc/vas: Define helpers to init window context Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 06/17] powerpc/vas: Define helpers to alloc/free windows Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 07/17] powerpc/vas: Define vas_win_paste_addr() Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 08/17] powerpc/vas: Define vas_win_id() Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 09/17] powerpc/vas: Define vas_rx_win_open() interface Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 10/17] " Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 11/17] powerpc/vas: Define vas_win_close() interface Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 12/17] powerpc/vas: Define vas_tx_win_open() Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 13/17] powerpc/vas: Define copy/paste interfaces Sukadev Bhattiprolu
2017-08-08 23:06 ` [PATCH v6 14/17] powerpc: Add support for setting SPRN_TIDR Sukadev Bhattiprolu
2017-08-14 7:02 ` Michael Neuling [this message]
2017-08-14 13:30 ` Benjamin Herrenschmidt
2017-08-14 19:27 ` Sukadev Bhattiprolu
2017-08-14 11:16 ` Michael Ellerman
2017-08-14 20:03 ` Sukadev Bhattiprolu
2017-08-14 22:25 ` Benjamin Herrenschmidt
2017-08-14 22:23 ` Benjamin Herrenschmidt
2017-08-08 23:07 ` [PATCH v6 15/17] powerpc/vas: Define window open ioctls API Sukadev Bhattiprolu
2017-08-08 23:07 ` [PATCH v6 16/17] powerpc/vas: Implement a simple FTW driver Sukadev Bhattiprolu
2017-08-14 6:53 ` Michael Ellerman
2017-08-08 23:07 ` [PATCH v6 17/17] powerpc/vas: Document FTW API/usage Sukadev Bhattiprolu
2017-08-14 10:43 ` Michael Neuling
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=1502694131.9844.7.camel@neuling.org \
--to=mikey@neuling.org \
--cc=apopple@au1.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=oohall@gmail.com \
--cc=stewart@linux.vnet.ibm.com \
--cc=sukadev@linux.vnet.ibm.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;
as well as URLs for NNTP newsgroup(s).