* [RFC PATCH] create generic alignment api (v2)
@ 2010-03-28 1:22 Mathieu Desnoyers
2010-03-28 15:40 ` Imre Deak
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-03-28 1:22 UTC (permalink / raw)
To: linux-arm-kernel
Rather than re-doing the "alignment on a type size" trick all over again at
different levels, import the "ltt_align" from LTTng into kernel.h and make this
available to everyone. Renaming to:
- offset_align
- offset_align_floor
- object_align
- object_align_floor
Changelog since v1:
- Align on the object natural alignment
(rather than min(arch word alignment, natural alignment))
The advantage of separating the API in "object alignment" and "offset alignment"
is that it gives more freedom to play with offset alignment. Very useful to
implement a tracer ring-buffer alignment. (hint hint)
Typical users will use "object alignment", but infrastructures like tracers
which need to perform dynamic alignment will typically use "offset alignment",
because it allows to align with respect to a base rather than to pass an
absolute address.
We use "sizeof(object)" rather than "__alignof__()" object because alignof
returns "recommended" object alignment for the architecture, which can be
sub-optimal on some architectures. By ensuring alignment on the object size, we
are sure to make the right choice.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Alexander Shishkin <virtuoso@slind.org>,
CC: linux-arm-kernel at lists.infradead.org
CC: Imre Deak <imre.deak@nokia.com>
CC: Jamie Lokier <jamie@shareable.org>
CC: rostedt at goodmis.org
CC: mingo at elte.hu
---
include/linux/kernel.h | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
Index: linux-2.6-lttng/include/linux/kernel.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/kernel.h 2010-03-27 20:46:07.000000000 -0400
+++ linux-2.6-lttng/include/linux/kernel.h 2010-03-27 21:17:52.000000000 -0400
@@ -42,6 +42,41 @@ extern const char linux_proc_banner[];
#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+/**
+ * offset_align - Calculate the offset needed to align an object on its natural
+ * alignment towards higher addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be added to align towards higher
+ * addresses.
+ */
+static inline size_t offset_align(size_t align_drift, size_t alignment)
+{
+ return (alignment - align_drift) & (alignment - 1);
+}
+
+/**
+ * offset_align_floor - Calculate the offset needed to align an object
+ * on its natural alignment towards lower addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be substracted to align towards lower addresses.
+ */
+static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
+{
+ return (align_drift - alignment) & (alignment - 1);
+}
+
+#define object_align(object) \
+ ((typeof(object))((size_t) object + offset_align((size_t) object, \
+ sizeof(object))))
+
+#define object_align_floor(object) \
+ ((typeof(object))((size_t) object - offset_align_floor((size_t) object,\
+ sizeof(object))))
+
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH] create generic alignment api (v2)
2010-03-28 1:22 [RFC PATCH] create generic alignment api (v2) Mathieu Desnoyers
@ 2010-03-28 15:40 ` Imre Deak
2010-03-28 18:29 ` Mathieu Desnoyers
2010-03-29 0:09 ` [RFC PATCH] create generic alignment api (v5) Mathieu Desnoyers
0 siblings, 2 replies; 11+ messages in thread
From: Imre Deak @ 2010-03-28 15:40 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Mar 28, 2010 at 03:22:47AM +0200, ext Mathieu Desnoyers wrote:
> [...]
> +/**
> + * offset_align - Calculate the offset needed to align an object on its natural
> + * alignment towards higher addresses.
> + * @align_drift: object offset from an "alignment"-aligned address.
> + * @alignment: natural object alignment. Must be non-zero, power of 2.
> + *
> + * Returns the offset that must be added to align towards higher
> + * addresses.
> + */
> +static inline size_t offset_align(size_t align_drift, size_t alignment)
> +{
> + return (alignment - align_drift) & (alignment - 1);
> +}
> +
> +/**
> + * offset_align_floor - Calculate the offset needed to align an object
> + * on its natural alignment towards lower addresses.
> + * @align_drift: object offset from an "alignment"-aligned address.
> + * @alignment: natural object alignment. Must be non-zero, power of 2.
> + *
> + * Returns the offset that must be substracted to align towards lower addresses.
> + */
> +static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
> +{
> + return (align_drift - alignment) & (alignment - 1);
> +}
> +
> +#define object_align(object) \
> + ((typeof(object))((size_t) object + offset_align((size_t) object, \
> + sizeof(object))))
> +
> +#define object_align_floor(object) \
> + ((typeof(object))((size_t) object - offset_align_floor((size_t) object,\
> + sizeof(object))))
> +
Here object must be a pointer, but then sizeof(object) will result in
aligning to the arch word size not the object's natural alignment. Is
this what you intended?
--Imre
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH] create generic alignment api (v2)
2010-03-28 15:40 ` Imre Deak
@ 2010-03-28 18:29 ` Mathieu Desnoyers
2010-03-29 0:09 ` [RFC PATCH] create generic alignment api (v5) Mathieu Desnoyers
1 sibling, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-03-28 18:29 UTC (permalink / raw)
To: linux-arm-kernel
* Imre Deak (imre.deak at nokia.com) wrote:
> On Sun, Mar 28, 2010 at 03:22:47AM +0200, ext Mathieu Desnoyers wrote:
> > [...]
> > +/**
> > + * offset_align - Calculate the offset needed to align an object on its natural
> > + * alignment towards higher addresses.
> > + * @align_drift: object offset from an "alignment"-aligned address.
> > + * @alignment: natural object alignment. Must be non-zero, power of 2.
> > + *
> > + * Returns the offset that must be added to align towards higher
> > + * addresses.
> > + */
> > +static inline size_t offset_align(size_t align_drift, size_t alignment)
> > +{
> > + return (alignment - align_drift) & (alignment - 1);
> > +}
> > +
> > +/**
> > + * offset_align_floor - Calculate the offset needed to align an object
> > + * on its natural alignment towards lower addresses.
> > + * @align_drift: object offset from an "alignment"-aligned address.
> > + * @alignment: natural object alignment. Must be non-zero, power of 2.
> > + *
> > + * Returns the offset that must be substracted to align towards lower addresses.
> > + */
> > +static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
> > +{
> > + return (align_drift - alignment) & (alignment - 1);
> > +}
> > +
> > +#define object_align(object) \
> > + ((typeof(object))((size_t) object + offset_align((size_t) object, \
> > + sizeof(object))))
> > +
> > +#define object_align_floor(object) \
> > + ((typeof(object))((size_t) object - offset_align_floor((size_t) object,\
> > + sizeof(object))))
> > +
>
> Here object must be a pointer, but then sizeof(object) will result in
> aligning to the arch word size not the object's natural alignment. Is
> this what you intended?
Nope, should be sizeof(*object). Good catch. I did the object_align*() macros
specifically as "helper" functions for your needs.
I'll repost a v3.
Thanks,
Mathieu
>
> --Imre
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH] create generic alignment api (v5)
2010-03-28 15:40 ` Imre Deak
2010-03-28 18:29 ` Mathieu Desnoyers
@ 2010-03-29 0:09 ` Mathieu Desnoyers
[not found] ` <20100501183544.GC27062@shisha.kicks-ass.net>
2010-05-01 19:23 ` [PATCH 1/2] create generic alignment api (v6) Alexander Shishkin
1 sibling, 2 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-03-29 0:09 UTC (permalink / raw)
To: linux-arm-kernel
Rather than re-doing the "alignment on a type size" trick all over again at
different levels, import the "ltt_align" from LTTng into kernel.h and make this
available to everyone. Renaming to:
- object_align()
- object_align_floor()
- offset_align()
- offset_align_floor()
Changelog since v4:
- add missing ( ) around parameters within object_align() and
object_align_floor().
- More coding style cleanups to ALIGN() (checkpatch.pl is happy now).
Changelog since v3:
- optimize object_align*() so fewer instructions are needed for alignment of
addresses known dynamically. Use the (already existing) "ALIGN()", and create
the "ALIGN_FLOOR()" macro.
- While we are there, let's clean up the ALIGN() macros wrt coding style. e.g.
missing parenthesis around the first use of the "x" parameter in ALIGN().
Changelog since v2:
- Fix object_align*(): should use object size alignment, not pointer alignment.
Changelog since v1:
- Align on the object natural alignment
(rather than min(arch word alignment, natural alignment))
The advantage of separating the API in "object alignment" and "offset alignment"
is that it gives more freedom to play with offset alignment. Very useful to
implement a tracer ring-buffer alignment. (hint hint)
Typical users will use "object alignment", but infrastructures like tracers
which need to perform alignment of statically known base+offsets will typically
use "offset alignment", because it allows to align with respect to a base rather
than to pass an absolute address.
We use "sizeof(object)" rather than "__alignof__()" object because alignof
returns "recommended" object alignment for the architecture, which can be
sub-optimal on some architectures. By ensuring alignment on the object size, we
are sure to make the right choice.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Alexander Shishkin <virtuoso@slind.org>,
CC: linux-arm-kernel at lists.infradead.org
CC: Imre Deak <imre.deak@nokia.com>
CC: Jamie Lokier <jamie@shareable.org>
CC: rostedt at goodmis.org
CC: mingo at elte.hu
---
include/linux/kernel.h | 45 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 4 deletions(-)
Index: linux-2.6-lttng/include/linux/kernel.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/kernel.h 2010-03-27 20:46:07.000000000 -0400
+++ linux-2.6-lttng/include/linux/kernel.h 2010-03-28 20:06:27.000000000 -0400
@@ -37,10 +37,47 @@ extern const char linux_proc_banner[];
#define STACK_MAGIC 0xdeadbeef
-#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
-#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
-#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
-#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+#define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x)) (a) - 1)
+#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
+#define PTR_ALIGN(p, a) ((typeof(p)) ALIGN((unsigned long) (p), (a)))
+#define ALIGN_FLOOR(x, a) __ALIGN_FLOOR_MASK((x), (typeof(x)) (a) - 1)
+#define __ALIGN_FLOOR_MASK(x, mask) ((x) & ~(mask))
+#define PTR_ALIGN_FLOOR(p, a) \
+ ((typeof(p)) ALIGN_FLOOR((unsigned long) (p), (a)))
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x)) (a) - 1)) == 0)
+
+/*
+ * Align pointer on natural object alignment. Object size must be power of two.
+ */
+#define object_align(obj) PTR_ALIGN((obj), sizeof(*(obj)))
+#define object_align_floor(obj) PTR_ALIGN_FLOOR((obj), sizeof(*(obj)))
+
+/**
+ * offset_align - Calculate the offset needed to align an object on its natural
+ * alignment towards higher addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be added to align towards higher
+ * addresses.
+ */
+static inline size_t offset_align(size_t align_drift, size_t alignment)
+{
+ return (alignment - align_drift) & (alignment - 1);
+}
+
+/**
+ * offset_align_floor - Calculate the offset needed to align an object
+ * on its natural alignment towards lower addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be substracted to align towards lower addresses.
+ */
+static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
+{
+ return (align_drift - alignment) & (alignment - 1);
+}
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH] create generic alignment api (v5)
[not found] ` <20100501183544.GC27062@shisha.kicks-ass.net>
@ 2010-05-01 19:10 ` Mathieu Desnoyers
[not found] ` <20100501192915.GD27062@shisha.kicks-ass.net>
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-05-01 19:10 UTC (permalink / raw)
To: linux-arm-kernel
* Alexander Shishkin (virtuoso at slind.org) wrote:
> On Sun, Mar 28, 2010 at 08:09:00 -0400, Mathieu Desnoyers wrote:
> > Rather than re-doing the "alignment on a type size" trick all over again at
> > different levels, import the "ltt_align" from LTTng into kernel.h and make this
> > available to everyone. Renaming to:
> >
> > - object_align()
> > - object_align_floor()
> > - offset_align()
> > - offset_align_floor()
>
> Do you plan to have this integrated to the mainline? I'd like to use one of
> these in my next version of the __xchg patch. So, do you need any help or
> feedback or whatnot with this one?
Given that no one seems to oppose to this patch, I think I'll just re-submit it
as-is for real this time.
Thanks,
Mathieu
>
> > Changelog since v4:
> > - add missing ( ) around parameters within object_align() and
> > object_align_floor().
> > - More coding style cleanups to ALIGN() (checkpatch.pl is happy now).
> >
> > Changelog since v3:
> > - optimize object_align*() so fewer instructions are needed for alignment of
> > addresses known dynamically. Use the (already existing) "ALIGN()", and create
> > the "ALIGN_FLOOR()" macro.
> > - While we are there, let's clean up the ALIGN() macros wrt coding style. e.g.
> > missing parenthesis around the first use of the "x" parameter in ALIGN().
> >
> > Changelog since v2:
> > - Fix object_align*(): should use object size alignment, not pointer alignment.
> >
> > Changelog since v1:
> > - Align on the object natural alignment
> > (rather than min(arch word alignment, natural alignment))
> >
> > The advantage of separating the API in "object alignment" and "offset alignment"
> > is that it gives more freedom to play with offset alignment. Very useful to
> > implement a tracer ring-buffer alignment. (hint hint)
> >
> > Typical users will use "object alignment", but infrastructures like tracers
> > which need to perform alignment of statically known base+offsets will typically
> > use "offset alignment", because it allows to align with respect to a base rather
> > than to pass an absolute address.
> >
> > We use "sizeof(object)" rather than "__alignof__()" object because alignof
> > returns "recommended" object alignment for the architecture, which can be
> > sub-optimal on some architectures. By ensuring alignment on the object size, we
> > are sure to make the right choice.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > CC: Russell King - ARM Linux <linux@arm.linux.org.uk>
> > Cc: Alexander Shishkin <virtuoso@slind.org>,
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: Imre Deak <imre.deak@nokia.com>
> > CC: Jamie Lokier <jamie@shareable.org>
> > CC: rostedt at goodmis.org
> > CC: mingo at elte.hu
>
> Acked-by: Alexander Shishkin <virtuoso@slind.org>
>
> > ---
> > include/linux/kernel.h | 45 +++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 41 insertions(+), 4 deletions(-)
> >
> > Index: linux-2.6-lttng/include/linux/kernel.h
> > ===================================================================
> > --- linux-2.6-lttng.orig/include/linux/kernel.h 2010-03-27 20:46:07.000000000 -0400
> > +++ linux-2.6-lttng/include/linux/kernel.h 2010-03-28 20:06:27.000000000 -0400
> > @@ -37,10 +37,47 @@ extern const char linux_proc_banner[];
> >
> > #define STACK_MAGIC 0xdeadbeef
> >
> > -#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
> > -#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
> > -#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
> > -#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
> > +#define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x)) (a) - 1)
> > +#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> > +#define PTR_ALIGN(p, a) ((typeof(p)) ALIGN((unsigned long) (p), (a)))
> > +#define ALIGN_FLOOR(x, a) __ALIGN_FLOOR_MASK((x), (typeof(x)) (a) - 1)
> > +#define __ALIGN_FLOOR_MASK(x, mask) ((x) & ~(mask))
> > +#define PTR_ALIGN_FLOOR(p, a) \
> > + ((typeof(p)) ALIGN_FLOOR((unsigned long) (p), (a)))
> > +#define IS_ALIGNED(x, a) (((x) & ((typeof(x)) (a) - 1)) == 0)
> > +
> > +/*
> > + * Align pointer on natural object alignment. Object size must be power of two.
> > + */
> > +#define object_align(obj) PTR_ALIGN((obj), sizeof(*(obj)))
> > +#define object_align_floor(obj) PTR_ALIGN_FLOOR((obj), sizeof(*(obj)))
> > +
> > +/**
> > + * offset_align - Calculate the offset needed to align an object on its natural
> > + * alignment towards higher addresses.
> > + * @align_drift: object offset from an "alignment"-aligned address.
> > + * @alignment: natural object alignment. Must be non-zero, power of 2.
> > + *
> > + * Returns the offset that must be added to align towards higher
> > + * addresses.
> > + */
> > +static inline size_t offset_align(size_t align_drift, size_t alignment)
> > +{
> > + return (alignment - align_drift) & (alignment - 1);
> > +}
> > +
> > +/**
> > + * offset_align_floor - Calculate the offset needed to align an object
> > + * on its natural alignment towards lower addresses.
> > + * @align_drift: object offset from an "alignment"-aligned address.
> > + * @alignment: natural object alignment. Must be non-zero, power of 2.
> > + *
> > + * Returns the offset that must be substracted to align towards lower addresses.
> > + */
> > +static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
> > +{
> > + return (align_drift - alignment) & (alignment - 1);
> > +}
> >
> > #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
> >
> >
> > --
> > Mathieu Desnoyers
> > Operating System Efficiency R&D Consultant
> > EfficiOS Inc.
> > http://www.efficios.com
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] create generic alignment api (v6)
2010-03-29 0:09 ` [RFC PATCH] create generic alignment api (v5) Mathieu Desnoyers
[not found] ` <20100501183544.GC27062@shisha.kicks-ass.net>
@ 2010-05-01 19:23 ` Alexander Shishkin
2010-05-01 19:24 ` [PATCH 2/2] [RFCv3] arm: add half-word __xchg Alexander Shishkin
2010-05-06 18:12 ` [PATCH 1/2] create generic alignment api (v6) Mathieu Desnoyers
1 sibling, 2 replies; 11+ messages in thread
From: Alexander Shishkin @ 2010-05-01 19:23 UTC (permalink / raw)
To: linux-arm-kernel
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Rather than re-doing the "alignment on a type size" trick all over again at
different levels, import the "ltt_align" from LTTng into kernel.h and make this
available to everyone. Renaming to:
- object_align()
- object_align_floor()
- offset_align()
- offset_align_floor()
Changelog since v5:
- moved alignment apis to a separate header file so that it is possible
to use them from other header files which are, for example, included
from kernel.h.
Changelog since v4:
- add missing ( ) around parameters within object_align() and
object_align_floor().
- More coding style cleanups to ALIGN() (checkpatch.pl is happy now).
Changelog since v3:
- optimize object_align*() so fewer instructions are needed for alignment of
addresses known dynamically. Use the (already existing) "ALIGN()", and create
the "ALIGN_FLOOR()" macro.
- While we are there, let's clean up the ALIGN() macros wrt coding style. e.g.
missing parenthesis around the first use of the "x" parameter in ALIGN().
Changelog since v2:
- Fix object_align*(): should use object size alignment, not pointer alignment.
Changelog since v1:
- Align on the object natural alignment
(rather than min(arch word alignment, natural alignment))
The advantage of separating the API in "object alignment" and "offset alignment"
is that it gives more freedom to play with offset alignment. Very useful to
implement a tracer ring-buffer alignment. (hint hint)
Typical users will use "object alignment", but infrastructures like tracers
which need to perform alignment of statically known base+offsets will typically
use "offset alignment", because it allows to align with respect to a base rather
than to pass an absolute address.
We use "sizeof(object)" rather than "__alignof__()" object because alignof
returns "recommended" object alignment for the architecture, which can be
sub-optimal on some architectures. By ensuring alignment on the object size, we
are sure to make the right choice.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
CC: Russell King - ARM Linux <linux@arm.linux.org.uk>
CC: linux-arm-kernel at lists.infradead.org
CC: Imre Deak <imre.deak@nokia.com>
CC: Jamie Lokier <jamie@shareable.org>
CC: rostedt at goodmis.org
CC: mingo at elte.hu
---
include/linux/align.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/kernel.h | 6 +-----
2 files changed, 49 insertions(+), 5 deletions(-)
create mode 100644 include/linux/align.h
diff --git a/include/linux/align.h b/include/linux/align.h
new file mode 100644
index 0000000..8aa2967
--- /dev/null
+++ b/include/linux/align.h
@@ -0,0 +1,48 @@
+#ifndef _LINUX_ALIGN_H
+#define _LINUX_ALIGN_H
+
+#include <linux/types.h>
+
+#define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x)) (a) - 1)
+#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
+#define PTR_ALIGN(p, a) ((typeof(p)) ALIGN((unsigned long) (p), (a)))
+#define ALIGN_FLOOR(x, a) __ALIGN_FLOOR_MASK((x), (typeof(x)) (a) - 1)
+#define __ALIGN_FLOOR_MASK(x, mask) ((x) & ~(mask))
+#define PTR_ALIGN_FLOOR(p, a) \
+ ((typeof(p)) ALIGN_FLOOR((unsigned long) (p), (a)))
+#define IS_ALIGNED(x, a) (((x) & ((typeof(x)) (a) - 1)) == 0)
+
+/*
+ * Align pointer on natural object alignment. Object size must be power of two.
+ */
+#define object_align(obj) PTR_ALIGN((obj), sizeof(*(obj)))
+#define object_align_floor(obj) PTR_ALIGN_FLOOR((obj), sizeof(*(obj)))
+
+/**
+ * offset_align - Calculate the offset needed to align an object on its natural
+ * alignment towards higher addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be added to align towards higher
+ * addresses.
+ */
+static inline size_t offset_align(size_t align_drift, size_t alignment)
+{
+ return (alignment - align_drift) & (alignment - 1);
+}
+
+/**
+ * offset_align_floor - Calculate the offset needed to align an object
+ * on its natural alignment towards lower addresses.
+ * @align_drift: object offset from an "alignment"-aligned address.
+ * @alignment: natural object alignment. Must be non-zero, power of 2.
+ *
+ * Returns the offset that must be substracted to align towards lower addresses.
+ */
+static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
+{
+ return (align_drift - alignment) & (alignment - 1);
+}
+
+#endif
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f4e3184..81d0d15 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -12,6 +12,7 @@
#include <linux/stddef.h>
#include <linux/types.h>
#include <linux/compiler.h>
+#include <linux/align.h>
#include <linux/bitops.h>
#include <linux/log2.h>
#include <linux/typecheck.h>
@@ -38,11 +39,6 @@ extern const char linux_proc_banner[];
#define STACK_MAGIC 0xdeadbeef
-#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
-#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
-#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
-#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
-
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
--
1.7.1.1.g15764
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] [RFCv3] arm: add half-word __xchg
2010-05-01 19:23 ` [PATCH 1/2] create generic alignment api (v6) Alexander Shishkin
@ 2010-05-01 19:24 ` Alexander Shishkin
2010-05-03 17:27 ` Mathieu Desnoyers
2010-05-06 18:12 ` [PATCH 1/2] create generic alignment api (v6) Mathieu Desnoyers
1 sibling, 1 reply; 11+ messages in thread
From: Alexander Shishkin @ 2010-05-01 19:24 UTC (permalink / raw)
To: linux-arm-kernel
On systems where ldrexh/strexh are not available,
* for pre-v6 systems, use a generic local version,
* for v6 without v6K, emulate xchg2 using 32-bit cmpxchg()
(it is not yet clear if xchg1 has to be emulated on such
systems as well, thus the "size" parameter).
The __xchg_generic() function is based on the code that Jamie
posted earlier.
Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
CC: linux-arm-kernel-bounces at lists.infradead.org
CC: Imre Deak <imre.deak@nokia.com>
CC: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Jamie Lokier <jamie@shareable.org>
---
arch/arm/include/asm/system.h | 56 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index d65b2f5..7a5983f 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -218,6 +218,39 @@ do { \
last = __switch_to(prev,task_thread_info(prev), task_thread_info(next)); \
} while (0)
+#if __LINUX_ARM_ARCH__ >= 6
+
+#include <linux/align.h>
+
+static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
+ unsigned long new, int size);
+
+/*
+ * emulate __xchg() using 32-bit __cmpxchg()
+ */
+static inline unsigned long __xchg_generic(unsigned long x,
+ volatile void *ptr, int size)
+{
+ unsigned long *ptrbig = object_align_floor((unsigned long *)ptr);
+ int shift = ((unsigned)ptr - (unsigned)ptrbig) * 8;
+ unsigned long mask, add, ret;
+
+ mask = ~(((1 << (size * 8)) - 1) << shift);
+ add = x << shift;
+
+ ret = *ptrbig;
+ while (1) {
+ unsigned long tmp = __cmpxchg(ptrbig, ret, (ret & mask) | add,
+ 4);
+ if (tmp == ret)
+ break;
+ ret = tmp;
+ }
+
+ return ret;
+}
+#endif
+
#if defined(CONFIG_CPU_SA1100) || defined(CONFIG_CPU_SA110)
/*
* On the StrongARM, "swp" is terminally broken since it bypasses the
@@ -262,6 +295,22 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
: "r" (x), "r" (ptr)
: "memory", "cc");
break;
+#ifdef CONFIG_CPU_32v6K
+ case 2:
+ asm volatile("@ __xchg2\n"
+ "1: ldrexh %0, [%3]\n"
+ " strexh %1, %2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (ret), "=&r" (tmp)
+ : "r" (x), "r" (ptr)
+ : "memory", "cc");
+ break;
+#else
+ case 2:
+ ret = __xchg_generic(x, ptr, 2);
+ break;
+#endif
case 4:
asm volatile("@ __xchg4\n"
"1: ldrex %0, [%3]\n"
@@ -283,6 +332,13 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
raw_local_irq_restore(flags);
break;
+ case 2:
+ raw_local_irq_save(flags);
+ ret = *(volatile unsigned short *)ptr;
+ *(volatile unsigned short *)ptr = x;
+ raw_local_irq_restore(flags);
+ break;
+
case 4:
raw_local_irq_save(flags);
ret = *(volatile unsigned long *)ptr;
--
1.7.1.1.g15764
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH] create generic alignment api (v5)
[not found] ` <20100501192915.GD27062@shisha.kicks-ass.net>
@ 2010-05-01 19:45 ` Mathieu Desnoyers
0 siblings, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-05-01 19:45 UTC (permalink / raw)
To: linux-arm-kernel
* Alexander Shishkin (virtuoso at slind.org) wrote:
> On Sat, May 01, 2010 at 03:10:45 -0400, Mathieu Desnoyers wrote:
> > * Alexander Shishkin (virtuoso at slind.org) wrote:
> > > On Sun, Mar 28, 2010 at 08:09:00 -0400, Mathieu Desnoyers wrote:
> > > > Rather than re-doing the "alignment on a type size" trick all over again at
> > > > different levels, import the "ltt_align" from LTTng into kernel.h and make this
> > > > available to everyone. Renaming to:
> > > >
> > > > - object_align()
> > > > - object_align_floor()
> > > > - offset_align()
> > > > - offset_align_floor()
> > >
> > > Do you plan to have this integrated to the mainline? I'd like to use one of
> > > these in my next version of the __xchg patch. So, do you need any help or
> > > feedback or whatnot with this one?
> >
> > Given that no one seems to oppose to this patch, I think I'll just re-submit it
> > as-is for real this time.
>
> Sorry, a short explanation: it has to be a separate header file in order to be of
> use to me, since I need those in system.h, which gets included implicitly through
> bitops.h earlier in kernel.h.
>
> So I took the liberty to hack up a v6 of this patch.
Yep, I'm ok with this change.
Thanks,
Mathieu
>
> Regards,
> --
> Alex
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] [RFCv3] arm: add half-word __xchg
2010-05-01 19:24 ` [PATCH 2/2] [RFCv3] arm: add half-word __xchg Alexander Shishkin
@ 2010-05-03 17:27 ` Mathieu Desnoyers
[not found] ` <20100505080155.GF27062@shisha.kicks-ass.net>
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-05-03 17:27 UTC (permalink / raw)
To: linux-arm-kernel
* Alexander Shishkin (virtuoso at slind.org) wrote:
> On systems where ldrexh/strexh are not available,
> * for pre-v6 systems, use a generic local version,
> * for v6 without v6K, emulate xchg2 using 32-bit cmpxchg()
> (it is not yet clear if xchg1 has to be emulated on such
> systems as well, thus the "size" parameter).
>
> The __xchg_generic() function is based on the code that Jamie
> posted earlier.
>
> Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
> CC: linux-arm-kernel-bounces at lists.infradead.org
> CC: Imre Deak <imre.deak@nokia.com>
> CC: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: Jamie Lokier <jamie@shareable.org>
> ---
> arch/arm/include/asm/system.h | 56 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index d65b2f5..7a5983f 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -218,6 +218,39 @@ do { \
> last = __switch_to(prev,task_thread_info(prev), task_thread_info(next)); \
> } while (0)
>
> +#if __LINUX_ARM_ARCH__ >= 6
> +
> +#include <linux/align.h>
> +
> +static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
> + unsigned long new, int size);
> +
> +/*
> + * emulate __xchg() using 32-bit __cmpxchg()
> + */
> +static inline unsigned long __xchg_generic(unsigned long x,
> + volatile void *ptr, int size)
> +{
> + unsigned long *ptrbig = object_align_floor((unsigned long *)ptr);
ptrbig could be renamed to something more relevant. Maybe ptralign ?
> + int shift = ((unsigned)ptr - (unsigned)ptrbig) * 8;
"unsigned int" everywhere above would be cleaner. Also, it's worth
checking the generated assembly: does gcc perform the transformation:
* 8 -> << 3 automatically ?
> + unsigned long mask, add, ret;
> +
> + mask = ~(((1 << (size * 8)) - 1) << shift);
Maybe better to do:
+ mask = ~((((1UL << 3) << size) - 1) << shift);
But, other question: what assumptions are you doing about endianness
here ? I recall that ARM supports switchable endianness. Dunno about the
Linux-specific case though.
Thanks,
Mathieu
> + add = x << shift;
> +
> + ret = *ptrbig;
> + while (1) {
> + unsigned long tmp = __cmpxchg(ptrbig, ret, (ret & mask) | add,
> + 4);
> + if (tmp == ret)
> + break;
> + ret = tmp;
> + }
> +
> + return ret;
> +}
> +#endif
> +
> #if defined(CONFIG_CPU_SA1100) || defined(CONFIG_CPU_SA110)
> /*
> * On the StrongARM, "swp" is terminally broken since it bypasses the
> @@ -262,6 +295,22 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
> : "r" (x), "r" (ptr)
> : "memory", "cc");
> break;
> +#ifdef CONFIG_CPU_32v6K
> + case 2:
> + asm volatile("@ __xchg2\n"
> + "1: ldrexh %0, [%3]\n"
> + " strexh %1, %2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (ret), "=&r" (tmp)
> + : "r" (x), "r" (ptr)
> + : "memory", "cc");
> + break;
> +#else
> + case 2:
> + ret = __xchg_generic(x, ptr, 2);
> + break;
> +#endif
> case 4:
> asm volatile("@ __xchg4\n"
> "1: ldrex %0, [%3]\n"
> @@ -283,6 +332,13 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
> raw_local_irq_restore(flags);
> break;
>
> + case 2:
> + raw_local_irq_save(flags);
> + ret = *(volatile unsigned short *)ptr;
> + *(volatile unsigned short *)ptr = x;
> + raw_local_irq_restore(flags);
> + break;
> +
> case 4:
> raw_local_irq_save(flags);
> ret = *(volatile unsigned long *)ptr;
> --
> 1.7.1.1.g15764
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] create generic alignment api (v6)
2010-05-01 19:23 ` [PATCH 1/2] create generic alignment api (v6) Alexander Shishkin
2010-05-01 19:24 ` [PATCH 2/2] [RFCv3] arm: add half-word __xchg Alexander Shishkin
@ 2010-05-06 18:12 ` Mathieu Desnoyers
1 sibling, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-05-06 18:12 UTC (permalink / raw)
To: linux-arm-kernel
* Alexander Shishkin (virtuoso at slind.org) wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> Rather than re-doing the "alignment on a type size" trick all over again at
> different levels, import the "ltt_align" from LTTng into kernel.h and make this
> available to everyone. Renaming to:
>
> - object_align()
> - object_align_floor()
> - offset_align()
> - offset_align_floor()
>
> Changelog since v5:
> - moved alignment apis to a separate header file so that it is possible
> to use them from other header files which are, for example, included
> from kernel.h.
>
> Changelog since v4:
> - add missing ( ) around parameters within object_align() and
> object_align_floor().
> - More coding style cleanups to ALIGN() (checkpatch.pl is happy now).
>
> Changelog since v3:
> - optimize object_align*() so fewer instructions are needed for alignment of
> addresses known dynamically. Use the (already existing) "ALIGN()", and create
> the "ALIGN_FLOOR()" macro.
> - While we are there, let's clean up the ALIGN() macros wrt coding style. e.g.
> missing parenthesis around the first use of the "x" parameter in ALIGN().
>
> Changelog since v2:
> - Fix object_align*(): should use object size alignment, not pointer alignment.
>
> Changelog since v1:
> - Align on the object natural alignment
> (rather than min(arch word alignment, natural alignment))
>
> The advantage of separating the API in "object alignment" and "offset alignment"
> is that it gives more freedom to play with offset alignment. Very useful to
> implement a tracer ring-buffer alignment. (hint hint)
>
> Typical users will use "object alignment", but infrastructures like tracers
> which need to perform alignment of statically known base+offsets will typically
> use "offset alignment", because it allows to align with respect to a base rather
> than to pass an absolute address.
>
> We use "sizeof(object)" rather than "__alignof__()" object because alignof
> returns "recommended" object alignment for the architecture, which can be
> sub-optimal on some architectures. By ensuring alignment on the object size, we
> are sure to make the right choice.
Hrm... thinking about this again, maybe it's better to use __alignof__() for
object_align() and object_align_floor(), because using "sizeof()" will cause
unexpected effects if a structure is passed as an object (you typically want to
align on the size of the largest object in the structure (or bigger) rather than
the structure size).
Thanks,
Mathieu
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
> CC: Russell King - ARM Linux <linux@arm.linux.org.uk>
> CC: linux-arm-kernel at lists.infradead.org
> CC: Imre Deak <imre.deak@nokia.com>
> CC: Jamie Lokier <jamie@shareable.org>
> CC: rostedt at goodmis.org
> CC: mingo at elte.hu
> ---
> include/linux/align.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/kernel.h | 6 +-----
> 2 files changed, 49 insertions(+), 5 deletions(-)
> create mode 100644 include/linux/align.h
>
> diff --git a/include/linux/align.h b/include/linux/align.h
> new file mode 100644
> index 0000000..8aa2967
> --- /dev/null
> +++ b/include/linux/align.h
> @@ -0,0 +1,48 @@
> +#ifndef _LINUX_ALIGN_H
> +#define _LINUX_ALIGN_H
> +
> +#include <linux/types.h>
> +
> +#define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x)) (a) - 1)
> +#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> +#define PTR_ALIGN(p, a) ((typeof(p)) ALIGN((unsigned long) (p), (a)))
> +#define ALIGN_FLOOR(x, a) __ALIGN_FLOOR_MASK((x), (typeof(x)) (a) - 1)
> +#define __ALIGN_FLOOR_MASK(x, mask) ((x) & ~(mask))
> +#define PTR_ALIGN_FLOOR(p, a) \
> + ((typeof(p)) ALIGN_FLOOR((unsigned long) (p), (a)))
> +#define IS_ALIGNED(x, a) (((x) & ((typeof(x)) (a) - 1)) == 0)
> +
> +/*
> + * Align pointer on natural object alignment. Object size must be power of two.
> + */
> +#define object_align(obj) PTR_ALIGN((obj), sizeof(*(obj)))
> +#define object_align_floor(obj) PTR_ALIGN_FLOOR((obj), sizeof(*(obj)))
> +
> +/**
> + * offset_align - Calculate the offset needed to align an object on its natural
> + * alignment towards higher addresses.
> + * @align_drift: object offset from an "alignment"-aligned address.
> + * @alignment: natural object alignment. Must be non-zero, power of 2.
> + *
> + * Returns the offset that must be added to align towards higher
> + * addresses.
> + */
> +static inline size_t offset_align(size_t align_drift, size_t alignment)
> +{
> + return (alignment - align_drift) & (alignment - 1);
> +}
> +
> +/**
> + * offset_align_floor - Calculate the offset needed to align an object
> + * on its natural alignment towards lower addresses.
> + * @align_drift: object offset from an "alignment"-aligned address.
> + * @alignment: natural object alignment. Must be non-zero, power of 2.
> + *
> + * Returns the offset that must be substracted to align towards lower addresses.
> + */
> +static inline size_t offset_align_floor(size_t align_drift, size_t alignment)
> +{
> + return (align_drift - alignment) & (alignment - 1);
> +}
> +
> +#endif
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f4e3184..81d0d15 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -12,6 +12,7 @@
> #include <linux/stddef.h>
> #include <linux/types.h>
> #include <linux/compiler.h>
> +#include <linux/align.h>
> #include <linux/bitops.h>
> #include <linux/log2.h>
> #include <linux/typecheck.h>
> @@ -38,11 +39,6 @@ extern const char linux_proc_banner[];
>
> #define STACK_MAGIC 0xdeadbeef
>
> -#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
> -#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
> -#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
> -#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
> -
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
>
> #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
> --
> 1.7.1.1.g15764
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] [RFCv3] arm: add half-word __xchg
[not found] ` <20100505080155.GF27062@shisha.kicks-ass.net>
@ 2010-05-08 23:30 ` Mathieu Desnoyers
0 siblings, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2010-05-08 23:30 UTC (permalink / raw)
To: linux-arm-kernel
* Alexander Shishkin (virtuoso at slind.org) wrote:
> On Mon, May 03, 2010 at 01:27:55 -0400, Mathieu Desnoyers wrote:
> > * Alexander Shishkin (virtuoso at slind.org) wrote:
> > > On systems where ldrexh/strexh are not available,
> > > * for pre-v6 systems, use a generic local version,
> > > * for v6 without v6K, emulate xchg2 using 32-bit cmpxchg()
> > > (it is not yet clear if xchg1 has to be emulated on such
> > > systems as well, thus the "size" parameter).
> > >
> > > The __xchg_generic() function is based on the code that Jamie
> > > posted earlier.
> > >
> > > Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
> > > CC: linux-arm-kernel-bounces at lists.infradead.org
> > > CC: Imre Deak <imre.deak@nokia.com>
> > > CC: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > CC: Jamie Lokier <jamie@shareable.org>
> > > ---
> > > arch/arm/include/asm/system.h | 56 +++++++++++++++++++++++++++++++++++++++++
> > > 1 files changed, 56 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > > index d65b2f5..7a5983f 100644
> > > --- a/arch/arm/include/asm/system.h
> > > +++ b/arch/arm/include/asm/system.h
> > > @@ -218,6 +218,39 @@ do { \
> > > last = __switch_to(prev,task_thread_info(prev), task_thread_info(next)); \
> > > } while (0)
> > >
> > > +#if __LINUX_ARM_ARCH__ >= 6
> > > +
> > > +#include <linux/align.h>
> > > +
> > > +static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
> > > + unsigned long new, int size);
> > > +
> > > +/*
> > > + * emulate __xchg() using 32-bit __cmpxchg()
> > > + */
> > > +static inline unsigned long __xchg_generic(unsigned long x,
> > > + volatile void *ptr, int size)
> > > +{
> > > + unsigned long *ptrbig = object_align_floor((unsigned long *)ptr);
> >
> > ptrbig could be renamed to something more relevant. Maybe ptralign ?
>
> Well, yes, it's not any bigger than any other pointer here. :)
>
:)
> > > + int shift = ((unsigned)ptr - (unsigned)ptrbig) * 8;
> >
> > "unsigned int" everywhere above would be cleaner. Also, it's worth
> > checking the generated assembly: does gcc perform the transformation:
> >
> > * 8 -> << 3 automatically ?
>
> Yes, it's such a basic optimisation that even tinycc will perform it without
> asking. I've checked anyway, thought. It is indeed a shift instruction for
> both << 3 and * 8.
OK
>
> > > + unsigned long mask, add, ret;
> > > +
> > > + mask = ~(((1 << (size * 8)) - 1) << shift);
> >
> > Maybe better to do:
> >
> > + mask = ~((((1UL << 3) << size) - 1) << shift);
>
> I think it's slightly less readable for little gain.
I'm fine either way, as I expect the resulting code to be the same.
>
> > But, other question: what assumptions are you doing about endianness
> > here ? I recall that ARM supports switchable endianness. Dunno about the
> > Linux-specific case though.
>
[side-node: the variable "add" should be renamed to e.g. "new" in
__xchg_generic]
> Isn't the endiannes case dealt with by the object_align_floor() there?
Pointer alignment and endianness are two completely separate issues.
> The difference would be (for size==2 case, for example) whether shift is
> 0 or 16 (for say, LE and BE), the mask should always be placed correctly
> regardless.
> Please correct me if I'm missing something here.
>
Well, the __xchg_generic code seems broken on big endian at least in
the size==2 case, because no shift is needed (but __xchg_generic is
doing one). The mask is just selecting the rest of the bits, so if the
shift is incorrect in the first place, I expect the mask to be similarly
broken.
Thanks,
Mathieu
> Regards,
> --
> Alex
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-05-08 23:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-28 1:22 [RFC PATCH] create generic alignment api (v2) Mathieu Desnoyers
2010-03-28 15:40 ` Imre Deak
2010-03-28 18:29 ` Mathieu Desnoyers
2010-03-29 0:09 ` [RFC PATCH] create generic alignment api (v5) Mathieu Desnoyers
[not found] ` <20100501183544.GC27062@shisha.kicks-ass.net>
2010-05-01 19:10 ` Mathieu Desnoyers
[not found] ` <20100501192915.GD27062@shisha.kicks-ass.net>
2010-05-01 19:45 ` Mathieu Desnoyers
2010-05-01 19:23 ` [PATCH 1/2] create generic alignment api (v6) Alexander Shishkin
2010-05-01 19:24 ` [PATCH 2/2] [RFCv3] arm: add half-word __xchg Alexander Shishkin
2010-05-03 17:27 ` Mathieu Desnoyers
[not found] ` <20100505080155.GF27062@shisha.kicks-ass.net>
2010-05-08 23:30 ` Mathieu Desnoyers
2010-05-06 18:12 ` [PATCH 1/2] create generic alignment api (v6) Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).