On 31/08/2017 01:32, Sukadev Bhattiprolu wrote:
Michael Neuling [mi...@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 <suka...@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)) {

Reply via email to