On 9/26/23 08:20, Justin Stitt wrote:

Ping.

Looking to get this patch and [1] slated for 6.7 which fixes some kselftest builds on older kernels.

On Wed, Sep 13, 2023 at 6:03 AM Justin Stitt justinstitt@google.com wrote:

...

When building with Clang, I am getting many warnings from the selftests/rseq tree.

Here's one such example from rseq tree: | param_test.c:1234:10: error: address argument to atomic operation must be a pointer to _Atomic type ('intptr_t *' (aka 'long *') invalid) | 1234 | while (!atomic_load(&args->percpu_list_ptr)) {} | | ^ ~~~~~~~~~~~~~~~~~~~~~~ | /usr/local/google/home/justinstitt/repos/tc-build/build/llvm/final/lib/clang/18/include/stdatomic.h:140:29: note: expanded from macro 'atomic_load' | 140 | #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST) | | ^ ~~~~~~

Use compiler builtins `__atomic_load_n()` and `__atomic_store_n()` with accompanying __ATOMIC_ACQUIRE and __ATOMIC_RELEASE, respectively. This will fix the warnings because the compiler builtins do not expect their arguments to have _Atomic type. This should also make TSAN happier.

Link: https://github.com/ClangBuiltLinux/linux/issues/1698 Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61 Suggested-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com Signed-off-by: Justin Stitt justinstitt@google.com

Reviewed-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com

Peter, should this go through tip ?

Shuah, should to go through selftests ?

Thanks,

Mathieu

...

---

Note: Previous RFC https://lore.kernel.org/r/20230908-kselftest-param_test-c-v1-1-e35bd9052d61@...

tools/testing/selftests/rseq/param_test.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/rseq/param_test.c b/tools/testing/selftests/rseq/param_test.c index bf951a490bb4..20403d58345c 100644 --- a/tools/testing/selftests/rseq/param_test.c +++ b/tools/testing/selftests/rseq/param_test.c @@ -1231,7 +1231,7 @@ void *test_membarrier_worker_thread(void *arg) }

     /* Wait for initialization. */

•   while (!atomic_load(&args->percpu_list_ptr)) {}
  

•   while (!__atomic_load_n(&args->percpu_list_ptr, __ATOMIC_ACQUIRE)) {}
  
     for (i = 0; i < iters; ++i) {
             int ret;
  

@@ -1299,22 +1299,22 @@ void *test_membarrier_manager_thread(void *arg) test_membarrier_init_percpu_list(&list_a); test_membarrier_init_percpu_list(&list_b);

•   atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a);
  

•   __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_a, __ATOMIC_RELEASE);
  
  

•   while (!atomic_load(&args->stop)) {
  

•   while (!__atomic_load_n(&args->stop, __ATOMIC_ACQUIRE)) {
             /* list_a is "active". */
             cpu_a = rand() % CPU_SETSIZE;
             /*
              * As list_b is "inactive", we should never see changes
              * to list_b.
              */
  

•           if (expect_b != atomic_load(&list_b.c[cpu_b].head->data)) {
  

•           if (expect_b != __atomic_load_n(&list_b.c[cpu_b].head->data, __ATOMIC_ACQUIRE)) {
                     fprintf(stderr, "Membarrier test failed\n");
                     abort();
             }
  
             /* Make list_b "active". */
  

•           atomic_store(&args->percpu_list_ptr, (intptr_t)&list_b);
  

•           __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_b, __ATOMIC_RELEASE);
             if (rseq_membarrier_expedited(cpu_a) &&
                             errno != ENXIO /* missing CPU */) {
                     perror("sys_membarrier");
  

@@ -1324,27 +1324,27 @@ void *test_membarrier_manager_thread(void *arg) * Cpu A should now only modify list_b, so the values * in list_a should be stable. */

•           expect_a = atomic_load(&list_a.c[cpu_a].head->data);
  

•           expect_a = __atomic_load_n(&list_a.c[cpu_a].head->data, __ATOMIC_ACQUIRE);
  
             cpu_b = rand() % CPU_SETSIZE;
             /*
              * As list_a is "inactive", we should never see changes
              * to list_a.
              */
  

•           if (expect_a != atomic_load(&list_a.c[cpu_a].head->data)) {
  

•           if (expect_a != __atomic_load_n(&list_a.c[cpu_a].head->data, __ATOMIC_ACQUIRE)) {
                     fprintf(stderr, "Membarrier test failed\n");
                     abort();
             }
  
             /* Make list_a "active". */
  

•           atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a);
  

•           __atomic_store_n(&args->percpu_list_ptr, (intptr_t)&list_a, __ATOMIC_RELEASE);
             if (rseq_membarrier_expedited(cpu_b) &&
                             errno != ENXIO /* missing CPU*/) {
                     perror("sys_membarrier");
                     abort();
             }
             /* Remember a value from list_b. */
  

•           expect_b = atomic_load(&list_b.c[cpu_b].head->data);
  

•           expect_b = __atomic_load_n(&list_b.c[cpu_b].head->data, __ATOMIC_ACQUIRE);
     }
  
     test_membarrier_free_percpu_list(&list_a);
  

@@ -1401,7 +1401,7 @@ void test_membarrier(void) } }

•   atomic_store(&thread_args.stop, 1);
  

•   __atomic_store_n(&thread_args.stop, 1, __ATOMIC_RELEASE);
     ret = pthread_join(manager_thread, NULL);
     if (ret) {
             errno = ret;
  
  

---

base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c change-id: 20230908-kselftest-param_test-c-1763b62e762f

Best regards,

Justin Stitt justinstitt@google.com

Thanks Justin