[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 1/2] xen/link: Introduce .bss.percpu.page_aligned

On 09.08.2019 15:42, Andrew Cooper wrote:
On 09/08/2019 13:32, Jan Beulich wrote:
Insert explicit alignment such that the result is safe even with objects
shorter than a page in length.  The POINTER_ALIGN for __bss_end is to
the lack of SMP_CACHE_BYTES alignment, as the loops which zero the BSS
pointer-sized stores on all architectures.


  * Drop stray trailing ALIGN(). Make DEFINE_PER_CPU_PAGE_ALIGNED() verify
    the alignment rather than specifying it.

My feelings about the stray-ness of ALIGN() notwithstanding, the commit
message now wrong and needs correcting.

Oh, indeed, and not just for this aspect.

--- a/xen/include/xen/percpu.h
+++ b/xen/include/xen/percpu.h
@@ -9,9 +9,17 @@
   * The _##name concatenation is being used here to prevent 'name'
from getting
   * macro expanded, while still allowing a per-architecture symbol
name prefix.
-#define DEFINE_PER_CPU(type, name) __DEFINE_PER_CPU(type, _##name, )
+#define DEFINE_PER_CPU(type, name) \
+    __DEFINE_PER_CPU(__section(".bss.percpu"), type, _ ## name)
+#define DEFINE_PER_CPU_PAGE_ALIGNED(type, name) \
+    typedef char name ## _chk_t[BUILD_BUG_ON_ZERO(__alignof(type) & \
+                                                  (PAGE_SIZE - 1))]; \
+    __DEFINE_PER_CPU(__section(".bss.percpu.page_aligned"), \
+                     type, _ ## name)

I think this would be easier to read as:

#define DEFINE_PER_CPU_PAGE_ALIGNED(type, name)                \
     typedef char name ## _chk_t[                               \
         BUILD_BUG_ON_ZERO(__alignof(type) & (PAGE_SIZE - 1))]; \
     __DEFINE_PER_CPU(__section(".bss.percpu.page_aligned"),    \
                      type, _ ## name)

By not breaking important bit of logic across a newline.

Will do.

Preferably with this changed, but definitely with the commit message
fixed, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks, Jan

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.