Skip to contentSkip to conversation

Generic Alignment API

PATCH In review Mathieu Desnoyers 4 participants 10 messages Aug 21, 2010 – Aug 22, 2010

lore
Share & tools
lore

Mathieu Desnoyers

author
#0 lore raw

cover Generic Alignment API

Hi,

These two patches implement a generic alignment header needed by the "Generic Ring Buffer" patchset and by xchg() fallback implementation on some ARM architectures.

The goal of this patch is to move pointer alignment arithmetic to one single location in the kernel rather than spreading these all over the place as this is currently the case in the Linux kernel (especially when an alignment on the floor value is needed).

I submit this patch for inclusion through -tip. Meanwhile, I will pull it in a git tree (based on -tip) on top of which I will rebase the "Generic Ring Buffer" patchset:

git://git.kernel.org/pub/scm/linux/kernel/git/compudj/linux-2.6-ringbuffer.git
branch: tip-pull-queue

Ingo, to merge these patches, you can either take these 2 patches or pull from the "tip-pull-queue", at your convenience (tip-pull-queue is based on tip/master).

Thanks,

Mathieu

Signature · 4 lines
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
#2 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

Review

Quoted 3 lines · Mathieu Desnoyers
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n)		\
+	MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Sorry, this is tasteless macro.

author
#3 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

Quoted 6 lines · Alexey Dobriyan (adobriyan at gmail.com)

Review

Quoted 3 lines · Mathieu Desnoyers
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n)		\
+	MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Sorry, this is tasteless macro.

Let's look at the surrounding where I added this macro in kernel.h:

/* Force a compilation error if condition is true */ #define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))

/* Force a compilation error if condition is constant and true */ #define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))

/* Force a compilation error if a constant expression is not a power of 2 */

#define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

/* Force a compilation error if condition is true, but also produce a
result (of value 0 and type size_t), so the expression can be used e.g. in a structure initializer (or where-ever else comma expressions aren't permitted). */
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))

So I am guessing that you plan to rewrite all of these ? Or perharps you have other suggestions ? Commit cc8ef6eb21e964b1c5eb97b2d0e8ac9893e1bf86 introduced "BUILD_BUG_ON_NOT_POWER_OF_2()" btw.

Thanks,

Mathieu

Signature · 4 lines
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
#4 lore raw

PATCH 1/2 kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

Quoted 29 lines · Mathieu Desnoyers
Quoted 6 lines · Alexey Dobriyan (adobriyan at gmail.com)

Review

Quoted 3 lines · Mathieu Desnoyers
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n)		\
+	MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Sorry, this is tasteless macro.

Let's look at the surrounding where I added this macro in kernel.h:

/* Force a compilation error if condition is true */ #define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))

/* Force a compilation error if condition is constant and true */ #define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))

/* Force a compilation error if a constant expression is not a power of 2 */

#define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

/* Force a compilation error if condition is true, but also produce a
result (of value 0 and type size_t), so the expression can be used e.g. in a structure initializer (or where-ever else comma expressions aren't permitted). */
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))

Quoted 1 line

So I am guessing that you plan to rewrite all of these ?

Of course not.

Quoted 1 line

Or perharps you have other suggestions ?

Learn that n & (n - 1) is idiomatic way to check for power of two in C. Done that, encoding that information in identifier won't make sense.

#5 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

In reply to Mathieu Desnoyers

Review

Quoted 3 lines
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n)		\
+	MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Looks super-ugly. IMHO just writing MAYBE_BUILD_BUG_ON(!n || n & (n - 1)) directly would be clear enough. If you really think that's unclear define a generic is_power_of_two() macro.

-Andi

author
#6 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

Quoted 7 lines · Andi Kleen (andi at firstfloor.org)

Review

Quoted 3 lines
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n)		\
+	MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Looks super-ugly. IMHO just writing MAYBE_BUILD_BUG_ON(!n || n & (n - 1)) directly would be clear enough. If you really think that's unclear define a generic is_power_of_two() macro.

There is already a is_power_of_two macro in log2.h, but I fear it might incorrectly interact with "MAYBE_BUILD_BUG_ON" (for some reason passing the constant result of a static inline is not treated as a constant by the macro, and thus it always "passes" the test).

So if everyone object to this new macro, I'd be tempted to just go with your suggestion. However the fact that we already have BUILD_BUG_ON_NOT_POWER_OF_2(n) made me think that some people prefer to have it done as a macro.

Other opinions ?

Thanks,

Mathieu

Signature · 4 lines
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
#7 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

I think about some test at the last bit if it is on, the number is not a power of 2

Quoted 36 lines · Mathieu Desnoyers
Quoted 7 lines · Andi Kleen (andi at firstfloor.org)

Review

Quoted 3 lines
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n) ? ? ? ? ? ? ? \
+ ? MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Looks super-ugly. IMHO just writing MAYBE_BUILD_BUG_ON(!n || n & (n - 1)) directly would be clear enough. If you really think that's unclear define a generic is_power_of_two() macro.

There is already a is_power_of_two macro in log2.h, but I fear it might incorrectly interact with "MAYBE_BUILD_BUG_ON" (for some reason passing the constant result of a static inline is not treated as a constant by the macro, and thus it always "passes" the test).

So if everyone object to this new macro, I'd be tempted to just go with your suggestion. However the fact that we already have BUILD_BUG_ON_NOT_POWER_OF_2(n) made me think that some people prefer to have it done as a macro.

Other opinions ?

Thanks,

Mathieu

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html Please read the FAQ at ?http://www.tux.org/lkml/

author
#8 lore raw

Re: [PATCH for -tip 1/2] kernel.h: add MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2

Quoted 2 lines · Anca Emanuel (anca.emanuel at gmail.com)

I think about some test at the last bit if it is on, the number is not a power of 2

Can you give an example, along with the types you have in mind ?

Thanks,

Mathieu

Quoted 39 lines
Quoted 36 lines · Mathieu Desnoyers
Quoted 7 lines · Andi Kleen (andi at firstfloor.org)

Review

Quoted 3 lines
(quoted diff) +3 -0
+/* Force a compilation error if condition is constant and not a power of 2 */
+#define MAYBE_BUILD_BUG_ON_NOT_POWER_OF_2(n) ? ? ? ? ? ? ? \
+ ? MAYBE_BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))

Looks super-ugly. IMHO just writing MAYBE_BUILD_BUG_ON(!n || n & (n - 1)) directly would be clear enough. If you really think that's unclear define a generic is_power_of_two() macro.

There is already a is_power_of_two macro in log2.h, but I fear it might incorrectly interact with "MAYBE_BUILD_BUG_ON" (for some reason passing the constant result of a static inline is not treated as a constant by the macro, and thus it always "passes" the test).

So if everyone object to this new macro, I'd be tempted to just go with your suggestion. However the fact that we already have BUILD_BUG_ON_NOT_POWER_OF_2(n) made me think that some people prefer to have it done as a macro.

Other opinions ?

Thanks,

Mathieu

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html Please read the FAQ at ?http://www.tux.org/lkml/

Signature · 4 lines
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com