* [PATCH RFC] Interface to set SPRN_TIDR
@ 2017-08-30 2:38 Sukadev Bhattiprolu
2017-08-30 7:08 ` Michael Neuling
0 siblings, 1 reply; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2017-08-30 2:38 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, michael.neuling, npiggin
Cc: linuxppc-dev
We need the SPRN_TIDR to be set for use with fast thread-wakeup
(core-to-core wakeup) in VAS. Each user thread that has a receive
window setup and expects to be notified when a sender issues a paste
needs to have a unique SPRN_TIDR value.
The SPRN_TIDR value only needs to unique within the process but for
now we use a globally unique thread id as described below.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
Changelog[v2]
- Michael Ellerman: Use an interface to assign TIDR so it is
assigned to only threads that need it; move assignment to
restore_sprs(). Drop lint from rebase;
arch/powerpc/include/asm/processor.h | 4 ++
arch/powerpc/include/asm/switch_to.h | 3 ++
arch/powerpc/kernel/process.c | 97 ++++++++++++++++++++++++++++++++++++
3 files changed, 104 insertions(+)
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 {
struct thread_struct {
unsigned long ksp; /* Kernel stack pointer */
+#ifdef CONFIG_PPC_VAS
+ unsigned long tidr;
+#endif
+
#ifdef CONFIG_PPC64
unsigned long ksp_vsid;
#endif
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 17c8380..4962455 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
#endif
}
+extern void set_thread_tidr(struct task_struct *t);
+extern void clear_thread_tidr(struct task_struct *t);
+
#endif /* _ASM_POWERPC_SWITCH_TO_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1f0fd36..13abb22 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct *old_thread,
mtspr(SPRN_TAR, new_thread->tar);
}
#endif
+#ifdef CONFIG_PPC_VAS
+ if (old_thread->tidr != new_thread->tidr)
+ mtspr(SPRN_TIDR, new_thread->tidr);
+#endif
}
#ifdef CONFIG_PPC_BOOK3S_64
@@ -1446,9 +1450,97 @@ void flush_thread(void)
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
}
+#ifdef CONFIG_PPC_VAS
+static DEFINE_SPINLOCK(vas_thread_id_lock);
+static DEFINE_IDA(vas_thread_ida);
+
+/*
+ * We need to assign an unique thread id to each thread in a process. This
+ * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
+ * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
+ * 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 thread
+ * 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 the
+ * arch-neutral code.
+ *
+ * So try to assign globally unique thraed ids for now.
+ *
+ * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
+ * For now, only threads that expect to be notified by the VAS
+ * hardware need a TIDR value and we assign values > 0 for those.
+ */
+#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
+static int assign_thread_tidr(void)
+{
+ int index;
+ int err;
+
+again:
+ if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
+ return -ENOMEM;
+
+ spin_lock(&vas_thread_id_lock);
+ err = ida_get_new_above(&vas_thread_ida, 1, &index);
+ spin_unlock(&vas_thread_id_lock);
+
+ if (err == -EAGAIN)
+ goto again;
+ else if (err)
+ return err;
+
+ if (index > MAX_THREAD_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_tidr(int id)
+{
+ spin_lock(&vas_thread_id_lock);
+ ida_remove(&vas_thread_ida, id);
+ spin_unlock(&vas_thread_id_lock);
+}
+
+void clear_thread_tidr(struct task_struct *t)
+{
+ if (t->thread.tidr) {
+ free_thread_tidr(t->thread.tidr);
+ t->thread.tidr = 0;
+ mtspr(SPRN_TIDR, 0);
+ }
+}
+
+/*
+ * Assign an unique thread id for this thread and set it in the
+ * thread structure. For now, we need this interface only for
+ * the current task.
+ */
+void set_thread_tidr(struct task_struct *t)
+{
+ WARN_ON(t != current);
+ t->thread.tidr = assign_thread_tidr();
+ mtspr(SPRN_TIDR, t->thread.tidr);
+}
+
+#endif /* CONFIG_PPC_VAS */
+
+
void
release_thread(struct task_struct *t)
{
+#ifdef CONFIG_PPC_VAS
+ clear_thread_tidr(t);
+#endif
}
/*
@@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
clear_task_ebb(dst);
+ dst->thread.tidr = 0;
+
return 0;
}
@@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
#endif
setup_ksp_vsid(p, sp);
+#ifdef CONFIG_PPC_VAS
+ p->thread.tidr = 0;
+#endif
#ifdef CONFIG_PPC64
if (cpu_has_feature(CPU_FTR_DSCR)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] Interface to set SPRN_TIDR
2017-08-30 2:38 [PATCH RFC] Interface to set SPRN_TIDR Sukadev Bhattiprolu
@ 2017-08-30 7:08 ` Michael Neuling
2017-08-30 23:32 ` Sukadev Bhattiprolu
0 siblings, 1 reply; 6+ messages in thread
From: Michael Neuling @ 2017-08-30 7:08 UTC (permalink / raw)
To: Sukadev Bhattiprolu, Michael Ellerman, Benjamin Herrenschmidt, npiggin
Cc: linuxppc-dev, Christophe Lombard
Suka,
Please CC Christophe who as an alternative way of doing this. We ned to get
agreement across all users of TIDR/AS_notify...
His patch is here:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/161582.html
Mikey
On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> We need the SPRN_TIDR to be set for use with fast thread-wakeup
> (core-to-core wakeup) in VAS. Each user thread that has a receive
> window setup and expects to be notified when a sender issues a paste
> needs to have a unique SPRN_TIDR value.
>=20
> The SPRN_TIDR value only needs to unique within the process but for
> now we use a globally unique thread id as described below.
>=20
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> Changelog[v2]
> - Michael Ellerman: Use an interface to assign TIDR so it is
> =C2=A0=C2=A0assigned to only threads that need it; move assignment to
> =C2=A0=C2=A0restore_sprs(). Drop lint from rebase;
>=20
>=20
> =C2=A0arch/powerpc/include/asm/processor.h |=C2=A0=C2=A04 ++
> =C2=A0arch/powerpc/include/asm/switch_to.h |=C2=A0=C2=A03 ++
> =C2=A0arch/powerpc/kernel/process.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0| 97
> ++++++++++++++++++++++++++++++++++++
> =C2=A03 files changed, 104 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 */
>=20
> +#ifdef CONFIG_PPC_VAS
> + unsigned long tidr;
> +#endif
> +
> =C2=A0#ifdef CONFIG_PPC64
> =C2=A0 unsigned long ksp_vsid;
> =C2=A0#endif
> diff --git a/arch/powerpc/include/asm/switch_to.h
> b/arch/powerpc/include/asm/switch_to.h
> index 17c8380..4962455 100644
> --- a/arch/powerpc/include/asm/switch_to.h
> +++ b/arch/powerpc/include/asm/switch_to.h
> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t=
)
> =C2=A0#endif
> =C2=A0}
>=20
> +extern void set_thread_tidr(struct task_struct *t);
> +extern void clear_thread_tidr(struct task_struct *t);
> +
> =C2=A0#endif /* _ASM_POWERPC_SWITCH_TO_H */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index 1f0fd36..13abb22 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_stru=
ct
> *old_thread,
> =C2=A0 mtspr(SPRN_TAR, new_thread->tar);
> =C2=A0 }
> =C2=A0#endif
> +#ifdef CONFIG_PPC_VAS
> + if (old_thread->tidr !=3D new_thread->tidr)
> + mtspr(SPRN_TIDR, new_thread->tidr);
> +#endif
> =C2=A0}
>=20
> =C2=A0#ifdef CONFIG_PPC_BOOK3S_64
> @@ -1446,9 +1450,97 @@ void flush_thread(void)
> =C2=A0#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> =C2=A0}
>=20
> +#ifdef CONFIG_PPC_VAS
> +static DEFINE_SPINLOCK(vas_thread_id_lock);
> +static DEFINE_IDA(vas_thread_ida);
> +
> +/*
> + * 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.
> + *
> + * So try to assign globally unique thraed ids for now.
> + *
> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> + *=C2=A0 =C2=A0For now, only threads that expect to be notified by the V=
AS
> + *=C2=A0 =C2=A0hardware need a TIDR value and we assign values > 0 for t=
hose.
> + */
> +#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
> +static int assign_thread_tidr(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);
> + spin_unlock(&vas_thread_id_lock);
> +
> + if (err =3D=3D -EAGAIN)
> + goto again;
> + else if (err)
> + return err;
> +
> + if (index > MAX_THREAD_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_tidr(int id)
> +{
> + spin_lock(&vas_thread_id_lock);
> + ida_remove(&vas_thread_ida, id);
> + spin_unlock(&vas_thread_id_lock);
> +}
> +
> +void clear_thread_tidr(struct task_struct *t)
> +{
> + if (t->thread.tidr) {
> + free_thread_tidr(t->thread.tidr);
> + t->thread.tidr =3D 0;
> + mtspr(SPRN_TIDR, 0);
> + }
> +}
> +
> +/*
> + * Assign an unique thread id for this thread and set it in the
> + * thread structure. For now, we need this interface only for
> + * the current task.
> + */
> +void set_thread_tidr(struct task_struct *t)
> +{
> + WARN_ON(t !=3D current);
> + t->thread.tidr =3D assign_thread_tidr();
> + mtspr(SPRN_TIDR, t->thread.tidr);
> +}
> +
> +#endif /* CONFIG_PPC_VAS */
> +
> +
> =C2=A0void
> =C2=A0release_thread(struct task_struct *t)
> =C2=A0{
> +#ifdef CONFIG_PPC_VAS
> + clear_thread_tidr(t);
> +#endif
> =C2=A0}
>=20
> =C2=A0/*
> @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, s=
truct
> task_struct *src)
>=20
> =C2=A0 clear_task_ebb(dst);
>=20
> + dst->thread.tidr =3D 0;
> +
> =C2=A0 return 0;
> =C2=A0}
>=20
> @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned=
long
> usp,
> =C2=A0#endif
>=20
> =C2=A0 setup_ksp_vsid(p, sp);
> +#ifdef CONFIG_PPC_VAS
> + p->thread.tidr =3D 0;
> +#endif
>=20
> =C2=A0#ifdef CONFIG_PPC64=C2=A0
> =C2=A0 if (cpu_has_feature(CPU_FTR_DSCR)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] Interface to set SPRN_TIDR
2017-08-30 7:08 ` Michael Neuling
@ 2017-08-30 23:32 ` Sukadev Bhattiprolu
2017-08-31 15:14 ` felix
0 siblings, 1 reply; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2017-08-30 23:32 UTC (permalink / raw)
To: Michael Neuling
Cc: Michael Ellerman, Benjamin Herrenschmidt, npiggin, linuxppc-dev,
Christophe Lombard
Michael Neuling [mikey@neuling.org] wrote:
> Suka,
>=20
> Please CC Christophe who as an alternative way of doing this. We ned to g=
et
> agreement across all users of TIDR/AS_notify...
Mikey,
Thanks. There is overlap between the two patches. I will send a patch on
top of Christophe's for the interfaces to assign/clear the TIDR value and
clear the thread->tidr during arch_dup_task_struct(). I will also drop the
CONFIG_VAS check since its not only for VAS.
Christophe, can you let me know of any other comments on this patch?
Suka
>=20
> His patch is here:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/161582.html
>=20
> Mikey
>=20
> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> > We need the SPRN_TIDR to be set for use with fast thread-wakeup
> > (core-to-core wakeup) in VAS. Each user thread that has a receive
> > window setup and expects to be notified when a sender issues a paste
> > needs to have a unique SPRN_TIDR value.
> >=20
> > The SPRN_TIDR value only needs to unique within the process but for
> > now we use a globally unique thread id as described below.
> >=20
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > ---
> > Changelog[v2]
> > - Michael Ellerman: Use an interface to assign TIDR so it is
> > =A0=A0assigned to only threads that need it; move assignment to
> > =A0=A0restore_sprs(). Drop lint from rebase;
> >=20
> >=20
> > =A0arch/powerpc/include/asm/processor.h |=A0=A04 ++
> > =A0arch/powerpc/include/asm/switch_to.h |=A0=A03 ++
> > =A0arch/powerpc/kernel/process.c=A0=A0=A0=A0=A0=A0=A0=A0| 97
> > ++++++++++++++++++++++++++++++++++++
> > =A03 files changed, 104 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 {
> > =A0struct thread_struct {
> > =A0 unsigned long ksp; /* Kernel stack pointer */
> >=20
> > +#ifdef CONFIG_PPC_VAS
> > + unsigned long tidr;
> > +#endif
> > +
> > =A0#ifdef CONFIG_PPC64
> > =A0 unsigned long ksp_vsid;
> > =A0#endif
> > diff --git a/arch/powerpc/include/asm/switch_to.h
> > b/arch/powerpc/include/asm/switch_to.h
> > index 17c8380..4962455 100644
> > --- a/arch/powerpc/include/asm/switch_to.h
> > +++ b/arch/powerpc/include/asm/switch_to.h
> > @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct =
*t)
> > =A0#endif
> > =A0}
> >=20
> > +extern void set_thread_tidr(struct task_struct *t);
> > +extern void clear_thread_tidr(struct task_struct *t);
> > +
> > =A0#endif /* _ASM_POWERPC_SWITCH_TO_H */
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/proces=
s.c
> > index 1f0fd36..13abb22 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_st=
ruct
> > *old_thread,
> > =A0 mtspr(SPRN_TAR, new_thread->tar);
> > =A0 }
> > =A0#endif
> > +#ifdef CONFIG_PPC_VAS
> > + if (old_thread->tidr !=3D new_thread->tidr)
> > + mtspr(SPRN_TIDR, new_thread->tidr);
> > +#endif
> > =A0}
> >=20
> > =A0#ifdef CONFIG_PPC_BOOK3S_64
> > @@ -1446,9 +1450,97 @@ void flush_thread(void)
> > =A0#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> > =A0}
> >=20
> > +#ifdef CONFIG_PPC_VAS
> > +static DEFINE_SPINLOCK(vas_thread_id_lock);
> > +static DEFINE_IDA(vas_thread_ida);
> > +
> > +/*
> > + * We need to assign an unique thread id to each thread in a process. =
This
> > + * thread id is intended to be used with the Fast Thread-wakeup (aka C=
ore-
> > + * to-core wakeup) mechanism being implemented on top of Virtual Accel=
erator
> > + * 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 =
thread
> > + * when copy_thread() is called. Fixing that would require changing mo=
re
> > + * 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 =
the
> > + * arch-neutral code.
> > + *
> > + * So try to assign globally unique thraed ids for now.
> > + *
> > + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> > + *=A0 =A0For now, only threads that expect to be notified by the VAS
> > + *=A0 =A0hardware need a TIDR value and we assign values > 0 for those.
> > + */
> > +#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
> > +static int assign_thread_tidr(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);
> > + spin_unlock(&vas_thread_id_lock);
> > +
> > + if (err =3D=3D -EAGAIN)
> > + goto again;
> > + else if (err)
> > + return err;
> > +
> > + if (index > MAX_THREAD_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_tidr(int id)
> > +{
> > + spin_lock(&vas_thread_id_lock);
> > + ida_remove(&vas_thread_ida, id);
> > + spin_unlock(&vas_thread_id_lock);
> > +}
> > +
> > +void clear_thread_tidr(struct task_struct *t)
> > +{
> > + if (t->thread.tidr) {
> > + free_thread_tidr(t->thread.tidr);
> > + t->thread.tidr =3D 0;
> > + mtspr(SPRN_TIDR, 0);
> > + }
> > +}
> > +
> > +/*
> > + * Assign an unique thread id for this thread and set it in the
> > + * thread structure. For now, we need this interface only for
> > + * the current task.
> > + */
> > +void set_thread_tidr(struct task_struct *t)
> > +{
> > + WARN_ON(t !=3D current);
> > + t->thread.tidr =3D assign_thread_tidr();
> > + mtspr(SPRN_TIDR, t->thread.tidr);
> > +}
> > +
> > +#endif /* CONFIG_PPC_VAS */
> > +
> > +
> > =A0void
> > =A0release_thread(struct task_struct *t)
> > =A0{
> > +#ifdef CONFIG_PPC_VAS
> > + clear_thread_tidr(t);
> > +#endif
> > =A0}
> >=20
> > =A0/*
> > @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst,=
struct
> > task_struct *src)
> >=20
> > =A0 clear_task_ebb(dst);
> >=20
> > + dst->thread.tidr =3D 0;
> > +
> > =A0 return 0;
> > =A0}
> >=20
> > @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsign=
ed long
> > usp,
> > =A0#endif
> >=20
> > =A0 setup_ksp_vsid(p, sp);
> > +#ifdef CONFIG_PPC_VAS
> > + p->thread.tidr =3D 0;
> > +#endif
> >=20
> > =A0#ifdef CONFIG_PPC64=A0
> > =A0 if (cpu_has_feature(CPU_FTR_DSCR)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] Interface to set SPRN_TIDR
2017-08-30 23:32 ` Sukadev Bhattiprolu
@ 2017-08-31 15:14 ` felix
2017-08-31 18:06 ` Sukadev Bhattiprolu
0 siblings, 1 reply; 6+ messages in thread
From: felix @ 2017-08-31 15:14 UTC (permalink / raw)
To: Sukadev Bhattiprolu, Michael Neuling
Cc: linuxppc-dev, Christophe Lombard, npiggin
On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
> Michael Neuling [mikey@neuling.org] wrote:
>> Suka,
>>
>> Please CC Christophe who as an alternative way of doing this. We ned to get
>> agreement across all users of TIDR/AS_notify...
> Mikey,
>
> Thanks. There is overlap between the two patches. I will send a patch on
> top of Christophe's for the interfaces to assign/clear the TIDR value and
> clear the thread->tidr during arch_dup_task_struct(). I will also drop the
> CONFIG_VAS check since its not only for VAS.
>
> Christophe, can you let me know of any other comments on this patch?
>
> Suka
Suka,
I am seconding Christophe on this matter. I think that your patch now
fulfills the CAPI use case requirements, with one exception: CAPI does
not restrict assigning a thread id to the current task. Please find a
few minor questions below.
Philippe
>> His patch is here:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
>>
>> Mikey
>>
>> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
>>> We need the SPRN_TIDR to be set for use with fast thread-wakeup
>>> (core-to-core wakeup) in VAS. Each user thread that has a receive
>>> window setup and expects to be notified when a sender issues a paste
>>> needs to have a unique SPRN_TIDR value.
>>>
>>> The SPRN_TIDR value only needs to unique within the process but for
>>> now we use a globally unique thread id as described below.
>>>
>>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>>> ---
>>> Changelog[v2]
>>> - Michael Ellerman: Use an interface to assign TIDR so it is
>>> assigned to only threads that need it; move assignment to
>>> restore_sprs(). Drop lint from rebase;
>>>
>>>
>>> arch/powerpc/include/asm/processor.h | 4 ++
>>> arch/powerpc/include/asm/switch_to.h | 3 ++
>>> arch/powerpc/kernel/process.c | 97
>>> ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 104 insertions(+)
>>>
>>> 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 {
>>> struct thread_struct {
>>> unsigned long ksp; /* Kernel stack pointer */
>>>
>>> +#ifdef CONFIG_PPC_VAS
>>> + unsigned long tidr;
>>> +#endif
>>> +
>>> #ifdef CONFIG_PPC64
>>> unsigned long ksp_vsid;
>>> #endif
>>> diff --git a/arch/powerpc/include/asm/switch_to.h
>>> b/arch/powerpc/include/asm/switch_to.h
>>> index 17c8380..4962455 100644
>>> --- a/arch/powerpc/include/asm/switch_to.h
>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
>>> #endif
>>> }
>>>
>>> +extern void set_thread_tidr(struct task_struct *t);
>>> +extern void clear_thread_tidr(struct task_struct *t);
>>> +
>>> #endif /* _ASM_POWERPC_SWITCH_TO_H */
>>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>>> index 1f0fd36..13abb22 100644
>>> --- a/arch/powerpc/kernel/process.c
>>> +++ b/arch/powerpc/kernel/process.c
>>> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
>>> *old_thread,
>>> mtspr(SPRN_TAR, new_thread->tar);
>>> }
>>> #endif
>>> +#ifdef CONFIG_PPC_VAS
>>> + if (old_thread->tidr != new_thread->tidr)
>>> + mtspr(SPRN_TIDR, new_thread->tidr);
>>> +#endif
>>> }
>>>
>>> #ifdef CONFIG_PPC_BOOK3S_64
>>> @@ -1446,9 +1450,97 @@ void flush_thread(void)
>>> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>>> }
>>>
>>> +#ifdef CONFIG_PPC_VAS
>>> +static DEFINE_SPINLOCK(vas_thread_id_lock);
>>> +static DEFINE_IDA(vas_thread_ida);
>>> +
>>> +/*
>>> + * We need to assign an unique thread id to each thread in a process. This
>>> + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
>>> + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
>>> + * 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 thread
>>> + * 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 the
>>> + * arch-neutral code.
>>> + *
>>> + * So try to assign globally unique thraed ids for now.
>>> + *
>>> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
>>> + * For now, only threads that expect to be notified by the VAS
>>> + * hardware need a TIDR value and we assign values > 0 for those.
>>> + */
>>> +#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
Why are you excluding ((1 << 15) - 1)?
>>> +static int assign_thread_tidr(void)
>>> +{
>>> + int index;
>>> + int err;
>>> +
>>> +again:
>>> + if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
>>> + return -ENOMEM;
>>> +
>>> + spin_lock(&vas_thread_id_lock);
>>> + err = ida_get_new_above(&vas_thread_ida, 1, &index);
Why are you excluding 1?
>>> + spin_unlock(&vas_thread_id_lock);
>>> +
>>> + if (err == -EAGAIN)
>>> + goto again;
>>> + else if (err)
>>> + return err;
>>> +
>>> + if (index > MAX_THREAD_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_tidr(int id)
>>> +{
>>> + spin_lock(&vas_thread_id_lock);
>>> + ida_remove(&vas_thread_ida, id);
>>> + spin_unlock(&vas_thread_id_lock);
>>> +}
>>> +
>>> +void clear_thread_tidr(struct task_struct *t)
>>> +{
>>> + if (t->thread.tidr) {
>>> + free_thread_tidr(t->thread.tidr);
>>> + t->thread.tidr = 0;
>>> + mtspr(SPRN_TIDR, 0);
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * Assign an unique thread id for this thread and set it in the
>>> + * thread structure. For now, we need this interface only for
>>> + * the current task.
>>> + */
>>> +void set_thread_tidr(struct task_struct *t)
>>> +{
>>> + WARN_ON(t != current);
CAPI does not have this restriction. It should be possible to assign a
thread id to an arbitrary task.
>>> + t->thread.tidr = assign_thread_tidr();
>>> + mtspr(SPRN_TIDR, t->thread.tidr);
>>> +}
>>> +
>>> +#endif /* CONFIG_PPC_VAS */
>>> +
>>> +
>>> void
>>> release_thread(struct task_struct *t)
>>> {
>>> +#ifdef CONFIG_PPC_VAS
>>> + clear_thread_tidr(t);
>>> +#endif
>>> }
>>>
>>> /*
>>> @@ -1474,6 +1566,8 @@ int arch_dup_task_struct(struct task_struct *dst, struct
>>> task_struct *src)
>>>
>>> clear_task_ebb(dst);
>>>
>>> + dst->thread.tidr = 0;
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1584,6 +1678,9 @@ int copy_thread(unsigned long clone_flags, unsigned long
>>> usp,
>>> #endif
>>>
>>> setup_ksp_vsid(p, sp);
>>> +#ifdef CONFIG_PPC_VAS
>>> + p->thread.tidr = 0;
>>> +#endif
>>>
>>> #ifdef CONFIG_PPC64
>>> if (cpu_has_feature(CPU_FTR_DSCR)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] Interface to set SPRN_TIDR
2017-08-31 15:14 ` felix
@ 2017-08-31 18:06 ` Sukadev Bhattiprolu
2017-09-01 9:18 ` Philippe Bergheaud
0 siblings, 1 reply; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2017-08-31 18:06 UTC (permalink / raw)
To: felix; +Cc: Michael Neuling, linuxppc-dev, Christophe Lombard, npiggin
felix [felix@linux.vnet.ibm.com] wrote:
> On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
> > Michael Neuling [mikey@neuling.org] wrote:
> > > Suka,
> > >
> > > Please CC Christophe who as an alternative way of doing this. We ned to get
> > > agreement across all users of TIDR/AS_notify...
> > Mikey,
> >
> > Thanks. There is overlap between the two patches. I will send a patch on
> > top of Christophe's for the interfaces to assign/clear the TIDR value and
> > clear the thread->tidr during arch_dup_task_struct(). I will also drop the
> > CONFIG_VAS check since its not only for VAS.
> >
> > Christophe, can you let me know of any other comments on this patch?
> >
> > Suka
> Suka,
>
> I am seconding Christophe on this matter. I think that your patch now
> fulfills the CAPI use case requirements, with one exception: CAPI does not
> restrict assigning a thread id to the current task. Please find a few minor
> questions below.
>
> Philippe
>
> > > His patch is here:
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
> > >
> > > Mikey
> > >
> > > On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
> > > > We need the SPRN_TIDR to be set for use with fast thread-wakeup
> > > > (core-to-core wakeup) in VAS. Each user thread that has a receive
> > > > window setup and expects to be notified when a sender issues a paste
> > > > needs to have a unique SPRN_TIDR value.
> > > >
> > > > The SPRN_TIDR value only needs to unique within the process but for
> > > > now we use a globally unique thread id as described below.
> > > >
> > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > > > ---
> > > > Changelog[v2]
> > > > - Michael Ellerman: Use an interface to assign TIDR so it is
> > > > assigned to only threads that need it; move assignment to
> > > > restore_sprs(). Drop lint from rebase;
> > > >
> > > >
> > > > arch/powerpc/include/asm/processor.h | 4 ++
> > > > arch/powerpc/include/asm/switch_to.h | 3 ++
> > > > arch/powerpc/kernel/process.c | 97
> > > > ++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 104 insertions(+)
> > > >
> > > > 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 {
> > > > struct thread_struct {
> > > > unsigned long ksp; /* Kernel stack pointer */
> > > >
> > > > +#ifdef CONFIG_PPC_VAS
> > > > + unsigned long tidr;
> > > > +#endif
> > > > +
> > > > #ifdef CONFIG_PPC64
> > > > unsigned long ksp_vsid;
> > > > #endif
> > > > diff --git a/arch/powerpc/include/asm/switch_to.h
> > > > b/arch/powerpc/include/asm/switch_to.h
> > > > index 17c8380..4962455 100644
> > > > --- a/arch/powerpc/include/asm/switch_to.h
> > > > +++ b/arch/powerpc/include/asm/switch_to.h
> > > > @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
> > > > #endif
> > > > }
> > > >
> > > > +extern void set_thread_tidr(struct task_struct *t);
> > > > +extern void clear_thread_tidr(struct task_struct *t);
> > > > +
> > > > #endif /* _ASM_POWERPC_SWITCH_TO_H */
> > > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > > > index 1f0fd36..13abb22 100644
> > > > --- a/arch/powerpc/kernel/process.c
> > > > +++ b/arch/powerpc/kernel/process.c
> > > > @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
> > > > *old_thread,
> > > > mtspr(SPRN_TAR, new_thread->tar);
> > > > }
> > > > #endif
> > > > +#ifdef CONFIG_PPC_VAS
> > > > + if (old_thread->tidr != new_thread->tidr)
> > > > + mtspr(SPRN_TIDR, new_thread->tidr);
> > > > +#endif
> > > > }
> > > >
> > > > #ifdef CONFIG_PPC_BOOK3S_64
> > > > @@ -1446,9 +1450,97 @@ void flush_thread(void)
> > > > #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> > > > }
> > > >
> > > > +#ifdef CONFIG_PPC_VAS
> > > > +static DEFINE_SPINLOCK(vas_thread_id_lock);
> > > > +static DEFINE_IDA(vas_thread_ida);
> > > > +
> > > > +/*
> > > > + * We need to assign an unique thread id to each thread in a process. This
> > > > + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
> > > > + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
> > > > + * 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 thread
> > > > + * 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 the
> > > > + * arch-neutral code.
> > > > + *
> > > > + * So try to assign globally unique thraed ids for now.
> > > > + *
> > > > + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
> > > > + * For now, only threads that expect to be notified by the VAS
> > > > + * hardware need a TIDR value and we assign values > 0 for those.
> > > > + */
> > > > +#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
> Why are you excluding ((1 << 15) - 1)?
You are right. I don't need to exclude that. Also, TIDR is a 16-bit (0:15 in
VAS's Local Notify TID) value right? So, will change to
#define MAX_THREAD_CONTEXT ((1 << 16) - 1)
> > > > +static int assign_thread_tidr(void)
> > > > +{
> > > > + int index;
> > > > + int err;
> > > > +
> > > > +again:
> > > > + if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
> > > > + return -ENOMEM;
> > > > +
> > > > + spin_lock(&vas_thread_id_lock);
> > > > + err = ida_get_new_above(&vas_thread_ida, 1, &index);
> Why are you excluding 1?
I think the "_above" in the name is misleading. The function header for
ida_get_new_above() says "above or equal" so 1 is not excluded?
> > > > + spin_unlock(&vas_thread_id_lock);
> > > > +
> > > > + if (err == -EAGAIN)
> > > > + goto again;
> > > > + else if (err)
> > > > + return err;
> > > > +
> > > > + if (index > MAX_THREAD_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_tidr(int id)
> > > > +{
> > > > + spin_lock(&vas_thread_id_lock);
> > > > + ida_remove(&vas_thread_ida, id);
> > > > + spin_unlock(&vas_thread_id_lock);
> > > > +}
> > > > +
> > > > +void clear_thread_tidr(struct task_struct *t)
> > > > +{
> > > > + if (t->thread.tidr) {
> > > > + free_thread_tidr(t->thread.tidr);
> > > > + t->thread.tidr = 0;
> > > > + mtspr(SPRN_TIDR, 0);
> > > > + }
> > > > +}
> > > > +
> > > > +/*
> > > > + * Assign an unique thread id for this thread and set it in the
> > > > + * thread structure. For now, we need this interface only for
> > > > + * the current task.
> > > > + */
> > > > +void set_thread_tidr(struct task_struct *t)
> > > > +{
> > > > + WARN_ON(t != current);
> CAPI does not have this restriction. It should be possible to assign a
> thread id to an arbitrary task.
I see. We (or caller) just have to figure out the locking for the task.
> > > > + t->thread.tidr = assign_thread_tidr();
> > > > + mtspr(SPRN_TIDR, t->thread.tidr);
> > > > +}
> > > > +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] Interface to set SPRN_TIDR
2017-08-31 18:06 ` Sukadev Bhattiprolu
@ 2017-09-01 9:18 ` Philippe Bergheaud
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Bergheaud @ 2017-09-01 9:18 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: Michael Neuling, linuxppc-dev, Christophe Lombard, npiggin
On 31/08/2017 20:06, Sukadev Bhattiprolu wrote:
> felix [felix@linux.vnet.ibm.com] wrote:
>> On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
>>> Michael Neuling [mikey@neuling.org] wrote:
>>>> Suka,
>>>>
>>>> Please CC Christophe who as an alternative way of doing this. We ned to get
>>>> agreement across all users of TIDR/AS_notify...
>>> Mikey,
>>>
>>> Thanks. There is overlap between the two patches. I will send a patch on
>>> top of Christophe's for the interfaces to assign/clear the TIDR value and
>>> clear the thread->tidr during arch_dup_task_struct(). I will also drop the
>>> CONFIG_VAS check since its not only for VAS.
>>>
>>> Christophe, can you let me know of any other comments on this patch?
>>>
>>> Suka
>> Suka,
>>
>> I am seconding Christophe on this matter. I think that your patch now
>> fulfills the CAPI use case requirements, with one exception: CAPI does not
>> restrict assigning a thread id to the current task. Please find a few minor
>> questions below.
>>
>> Philippe
>>
>>>> His patch is here:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2017-2DAugust_161582.html&d=DwIFAw&c=jf_iaSHvJObTbx-siA1ZOg&r=KC0fX9VGJYXlSiH9qN2ZONDbh8vUCZFX8GUhF3rHAvg&m=XQenBfWewOBjWopgf1Fh2UAVGnlzq766MNuzx7jYfuA&s=07WOVTh9f_4IBZfCJes4lvc7LWenBlqVfAXIXxL2QH4&e=
>>>>
>>>> Mikey
>>>>
>>>> On Tue, 2017-08-29 at 19:38 -0700, Sukadev Bhattiprolu wrote:
>>>>> We need the SPRN_TIDR to be set for use with fast thread-wakeup
>>>>> (core-to-core wakeup) in VAS. Each user thread that has a receive
>>>>> window setup and expects to be notified when a sender issues a paste
>>>>> needs to have a unique SPRN_TIDR value.
>>>>>
>>>>> The SPRN_TIDR value only needs to unique within the process but for
>>>>> now we use a globally unique thread id as described below.
>>>>>
>>>>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changelog[v2]
>>>>> - Michael Ellerman: Use an interface to assign TIDR so it is
>>>>> assigned to only threads that need it; move assignment to
>>>>> restore_sprs(). Drop lint from rebase;
>>>>>
>>>>>
>>>>> arch/powerpc/include/asm/processor.h | 4 ++
>>>>> arch/powerpc/include/asm/switch_to.h | 3 ++
>>>>> arch/powerpc/kernel/process.c | 97
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 104 insertions(+)
>>>>>
>>>>> 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 {
>>>>> struct thread_struct {
>>>>> unsigned long ksp; /* Kernel stack pointer */
>>>>>
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> + unsigned long tidr;
>>>>> +#endif
>>>>> +
>>>>> #ifdef CONFIG_PPC64
>>>>> unsigned long ksp_vsid;
>>>>> #endif
>>>>> diff --git a/arch/powerpc/include/asm/switch_to.h
>>>>> b/arch/powerpc/include/asm/switch_to.h
>>>>> index 17c8380..4962455 100644
>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>> @@ -91,4 +91,7 @@ static inline void clear_task_ebb(struct task_struct *t)
>>>>> #endif
>>>>> }
>>>>>
>>>>> +extern void set_thread_tidr(struct task_struct *t);
>>>>> +extern void clear_thread_tidr(struct task_struct *t);
>>>>> +
>>>>> #endif /* _ASM_POWERPC_SWITCH_TO_H */
>>>>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>>>>> index 1f0fd36..13abb22 100644
>>>>> --- a/arch/powerpc/kernel/process.c
>>>>> +++ b/arch/powerpc/kernel/process.c
>>>>> @@ -1132,6 +1132,10 @@ static inline void restore_sprs(struct thread_struct
>>>>> *old_thread,
>>>>> mtspr(SPRN_TAR, new_thread->tar);
>>>>> }
>>>>> #endif
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> + if (old_thread->tidr != new_thread->tidr)
>>>>> + mtspr(SPRN_TIDR, new_thread->tidr);
>>>>> +#endif
>>>>> }
>>>>>
>>>>> #ifdef CONFIG_PPC_BOOK3S_64
>>>>> @@ -1446,9 +1450,97 @@ void flush_thread(void)
>>>>> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>>>>> }
>>>>>
>>>>> +#ifdef CONFIG_PPC_VAS
>>>>> +static DEFINE_SPINLOCK(vas_thread_id_lock);
>>>>> +static DEFINE_IDA(vas_thread_ida);
>>>>> +
>>>>> +/*
>>>>> + * We need to assign an unique thread id to each thread in a process. This
>>>>> + * thread id is intended to be used with the Fast Thread-wakeup (aka Core-
>>>>> + * to-core wakeup) mechanism being implemented on top of Virtual Accelerator
>>>>> + * 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 thread
>>>>> + * 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 the
>>>>> + * arch-neutral code.
>>>>> + *
>>>>> + * So try to assign globally unique thraed ids for now.
>>>>> + *
>>>>> + * NOTE: TIDR 0 indicates that the thread does not need a TIDR value.
>>>>> + * For now, only threads that expect to be notified by the VAS
>>>>> + * hardware need a TIDR value and we assign values > 0 for those.
>>>>> + */
>>>>> +#define MAX_THREAD_CONTEXT ((1 << 15) - 2)
>> Why are you excluding ((1 << 15) - 1)?
> You are right. I don't need to exclude that. Also, TIDR is a 16-bit (0:15 in
> VAS's Local Notify TID) value right? So, will change to
>
> #define MAX_THREAD_CONTEXT ((1 << 16) - 1)
Yes.
>
>>>>> +static int assign_thread_tidr(void)
>>>>> +{
>>>>> + int index;
>>>>> + int err;
>>>>> +
>>>>> +again:
>>>>> + if (!ida_pre_get(&vas_thread_ida, GFP_KERNEL))
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + spin_lock(&vas_thread_id_lock);
>>>>> + err = ida_get_new_above(&vas_thread_ida, 1, &index);
>> Why are you excluding 1?
> I think the "_above" in the name is misleading. The function header for
> ida_get_new_above() says "above or equal" so 1 is not excluded?
Understood.
>
>>>>> + spin_unlock(&vas_thread_id_lock);
>>>>> +
>>>>> + if (err == -EAGAIN)
>>>>> + goto again;
>>>>> + else if (err)
>>>>> + return err;
>>>>> +
>>>>> + if (index > MAX_THREAD_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_tidr(int id)
>>>>> +{
>>>>> + spin_lock(&vas_thread_id_lock);
>>>>> + ida_remove(&vas_thread_ida, id);
>>>>> + spin_unlock(&vas_thread_id_lock);
>>>>> +}
>>>>> +
>>>>> +void clear_thread_tidr(struct task_struct *t)
>>>>> +{
>>>>> + if (t->thread.tidr) {
>>>>> + free_thread_tidr(t->thread.tidr);
>>>>> + t->thread.tidr = 0;
>>>>> + mtspr(SPRN_TIDR, 0);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Assign an unique thread id for this thread and set it in the
>>>>> + * thread structure. For now, we need this interface only for
>>>>> + * the current task.
>>>>> + */
>>>>> +void set_thread_tidr(struct task_struct *t)
>>>>> +{
>>>>> + WARN_ON(t != current);
>> CAPI does not have this restriction. It should be possible to assign a
>> thread id to an arbitrary task.
> I see. We (or caller) just have to figure out the locking for the task.
The extension to any thread can be implemented in a separate patch.
'current' will do for now.
>
>>>>> + t->thread.tidr = assign_thread_tidr();
>>>>> + mtspr(SPRN_TIDR, t->thread.tidr);
>>>>> +}
>>>>> +
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-01 9:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 2:38 [PATCH RFC] Interface to set SPRN_TIDR Sukadev Bhattiprolu
2017-08-30 7:08 ` Michael Neuling
2017-08-30 23:32 ` Sukadev Bhattiprolu
2017-08-31 15:14 ` felix
2017-08-31 18:06 ` Sukadev Bhattiprolu
2017-09-01 9:18 ` Philippe Bergheaud
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.