2008-11-24 19:14:08

by Harvey Harrison

[permalink] [raw]
Subject: [PATCH-mm] kernel: add common endian load/store API

Add the following API for the 6 endian types in the kernel
__le16,__le32, __le64, __be16, __be32, __be64:

u16 load_le16(const __le16 *p)
u16 load_le16_noalign(const __le16 *p)

void store_le16(__le16 *p, u16 val)
void store_le16_noalign(__le16 *p, u16 val)

get/put_unaligned are being replaced because get/put in the kernel
usually implies some kind of reference is being taken/released, which
is not the case here. They work with void * pointers which defeats
sparse checking. Also, put_unaligned takes its arguments in the
opposite order from what is expected. The new names are chosen
to allow the APIs to live in parallel without breaking compilation.
The get/put_unaligned API can be removed once all users are converted.

load_le16 is a synonym for the existing le16_to_cpup and is added to
be symmetric with the load_le16_noalign API. On arches where unaligned
access is OK, the unaligned calls are replaced with aligned calls. This
name is also shorter than le16_to_cpup which will hopefully encourage its
use as it is generally faster than dereferencing the pointer and using
le16_to_cpu. The only case where this does not hold is when taking the
address of a stack variable, as the work to get the stack variable address
generally outweighs just using le16_to_cpu directly.

store_le16 is a new API and is added to be symmetric with the unaligned
functions. It is implemented as a macro to allow compile-time byteswapping
when the value is a constant. This will also allow use in many places
currently that are of the form:

*(__le16 *)ptr = cpu_to_le16(foo);

In addition, some drivers/filesystems/arches already provide this API
privately, which will allow them to be consolidated into this common
code.

Signed-off-by: Harvey Harrison <[email protected]>
---
include/asm-generic/unaligned.h | 100 +++++++++++++++++++++++++--------------
include/linux/byteorder.h | 14 +++++
2 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 55d1126..d2f3998 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -6,6 +6,20 @@

#ifdef _UNALIGNED_ACCESS_OK

+# define load_le16_noalign load_le16
+# define load_le32_noalign load_le32
+# define load_le64_noalign load_le64
+# define load_be16_noalign load_be16
+# define load_be32_noalign load_be32
+# define load_be64_noalign load_be64
+
+# define store_le16_noalign store_le16
+# define store_le32_noalign store_le32
+# define store_le64_noalign store_le64
+# define store_be16_noalign store_be16
+# define store_be32_noalign store_be32
+# define store_be64_noalign store_be64
+
static inline u16 get_unaligned_le16(const void *p)
{
return le16_to_cpup(p);
@@ -102,60 +116,67 @@ static inline u64 __get_be64_noalign(const u8 *p)
return ((u64)__get_be32_noalign(p) << 32) | __get_be32_noalign(p + 4);
}

-static inline u16 get_unaligned_le16(const void *p)
+static inline u16 load_le16_noalign(const __le16 *p)
{
#ifdef __LITTLE_ENDIAN
- return ((const struct __una_u16 *)p)->x;
+ return ((__force const struct __una_u16 *)p)->x;
#else
- return __get_le16_noalign(p);
+ return __get_le16_noalign((__force const u8 *)p);
#endif
}

-static inline u32 get_unaligned_le32(const void *p)
+static inline u32 load_le32_noalign(const __le32 *p)
{
#ifdef __LITTLE_ENDIAN
- return ((const struct __una_u32 *)p)->x;
+ return ((__force const struct __una_u32 *)p)->x;
#else
- return __get_le32_noalign(p);
+ return __get_le32_noalign((__force const u8 *)p);
#endif
}

-static inline u64 get_unaligned_le64(const void *p)
+static inline u64 load_le64_noalign(const __le64 *p)
{
#ifdef __LITTLE_ENDIAN
- return ((const struct __una_u64 *)p)->x;
+ return ((__force const struct __una_u64 *)p)->x;
#else
- return __get_le64_noalign(p);
+ return __get_le64_noalign((__force const u8 *)p);
#endif
}

-static inline u16 get_unaligned_be16(const void *p)
+static inline u16 load_be16_noalign(const __be16 *p)
{
#ifdef __BIG_ENDIAN
- return ((const struct __una_u16 *)p)->x;
+ return ((__force const struct __una_u16 *)p)->x;
#else
- return __get_be16_noalign(p);
+ return __get_be16_noalign((__force const u8 *)p);
#endif
}

-static inline u32 get_unaligned_be32(const void *p)
+static inline u32 load_be32_noalign(const __be32 *p)
{
#ifdef __BIG_ENDIAN
- return ((const struct __una_u32 *)p)->x;
+ return ((__force const struct __una_u32 *)p)->x;
#else
- return __get_be32_noalign(p);
+ return __get_be32_noalign((__force const u8 *)p);
#endif
}

-static inline u64 get_unaligned_be64(const void *p)
+static inline u64 load_be64_noalign(const __be64 *p)
{
#ifdef __BIG_ENDIAN
- return ((const struct __una_u64 *)p)->x;
+ return ((__force const struct __una_u64 *)p)->x;
#else
- return __get_be64_noalign(p);
+ return __get_be64_noalign((__force const u8 *)p);
#endif
}

+#define get_unaligned_le16(p) load_le16_noalign((void *)(p))
+#define get_unaligned_le32(p) load_le32_noalign((void *)(p))
+#define get_unaligned_le64(p) load_le64_noalign((void *)(p))
+#define get_unaligned_be16(p) load_be16_noalign((void *)(p))
+#define get_unaligned_be32(p) load_be32_noalign((void *)(p))
+#define get_unaligned_be64(p) load_be64_noalign((void *)(p))
+
static inline void __put_le16_noalign(u8 *p, u16 val)
{
*p++ = val;
@@ -192,60 +213,67 @@ static inline void __put_be64_noalign(u8 *p, u64 val)
__put_be32_noalign(p + 4, val);
}

-static inline void put_unaligned_le16(u16 val, void *p)
+static inline void store_le16_noalign(__le16 *p, u16 val)
{
#ifdef __LITTLE_ENDIAN
- ((struct __una_u16 *)p)->x = val;
+ ((__force struct __una_u16 *)p)->x = val;
#else
- __put_le16_noalign(p, val);
+ __put_le16_noalign((__force u8 *)p, val);
#endif
}

-static inline void put_unaligned_le32(u32 val, void *p)
+static inline void store_le32_noalign(__le32 *p, u32 val)
{
#ifdef __LITTLE_ENDIAN
- ((struct __una_u32 *)p)->x = val;
+ ((__force struct __una_u32 *)p)->x = val;
#else
- __put_le32_noalign(p, val);
+ __put_le32_noalign((__force u8 *)p, val);
#endif
}

-static inline void put_unaligned_le64(u64 val, void *p)
+static inline void store_le64_noalign(__le64 *p, u64 val)
{
#ifdef __LITTLE_ENDIAN
- ((struct __una_u64 *)p)->x = val;
+ ((__force struct __una_u64 *)p)->x = val;
#else
- __put_le64_noalign(p, val);
+ __put_le64_noalign((__force u8 *)p, val);
#endif
}

-static inline void put_unaligned_be16(u16 val, void *p)
+static inline void store_be16_noalign(__be16 *p, u16 val)
{
#ifdef __BIG_ENDIAN
- ((struct __una_u16 *)p)->x = val;
+ ((__force struct __una_u16 *)p)->x = val;
#else
- __put_be16_noalign(p, val);
+ __put_be16_noalign((__force u8 *)p, val);
#endif
}

-static inline void put_unaligned_be32(u32 val, void *p)
+static inline void store_be32_noalign(__be32 *p, u32 val)
{
#ifdef __BIG_ENDIAN
- ((struct __una_u32 *)p)->x = val;
+ ((__force struct __una_u32 *)p)->x = val;
#else
- __put_be32_noalign(p, val);
+ __put_be32_noalign((__force u8 *)p, val);
#endif
}

-static inline void put_unaligned_be64(u64 val, void *p)
+static inline void store_be64_noalign(__be64 *p, u64 val)
{
#ifdef __BIG_ENDIAN
- ((struct __una_u64 *)p)->x = val;
+ ((__force struct __una_u64 *)p)->x = val;
#else
- __put_be64_noalign(p, val);
+ __put_be64_noalign((__force u8 *)p, val);
#endif
}

+#define put_unaligned_le16(val, p) store_le16_noalign((void *)(p), (val))
+#define put_unaligned_le32(val, p) store_le32_noalign((void *)(p), (val))
+#define put_unaligned_le64(val, p) store_le64_noalign((void *)(p), (val))
+#define put_unaligned_be16(val, p) store_be16_noalign((void *)(p), (val))
+#define put_unaligned_be32(val, p) store_be32_noalign((void *)(p), (val))
+#define put_unaligned_be64(val, p) store_be64_noalign((void *)(p), (val))
+
#endif /* _UNALIGNED_ACCESS_OK */

/*
diff --git a/include/linux/byteorder.h b/include/linux/byteorder.h
index 29f002d..87a56e5 100644
--- a/include/linux/byteorder.h
+++ b/include/linux/byteorder.h
@@ -292,6 +292,20 @@ static inline __be64 __cpu_to_be64p(const __u64 *p)
# define cpu_to_be32 __cpu_to_be32
# define cpu_to_be64 __cpu_to_be64

+# define load_le16 __le16_to_cpup
+# define load_le32 __le32_to_cpup
+# define load_le64 __le64_to_cpup
+# define load_be16 __be16_to_cpup
+# define load_be32 __be32_to_cpup
+# define load_be64 __be64_to_cpup
+
+# define store_le16(p, val) (*(__le16 *)(p) = cpu_to_le16(val))
+# define store_le32(p, val) (*(__le32 *)(p) = cpu_to_le32(val))
+# define store_le64(p, val) (*(__le64 *)(p) = cpu_to_le64(val))
+# define store_be16(p, val) (*(__be16 *)(p) = cpu_to_be16(val))
+# define store_be32(p, val) (*(__be32 *)(p) = cpu_to_be32(val))
+# define store_be64(p, val) (*(__be64 *)(p) = cpu_to_be64(val))
+
# define le16_to_cpup __le16_to_cpup
# define le32_to_cpup __le32_to_cpup
# define le64_to_cpup __le64_to_cpup
--
1.6.0.4.1013.gc6a01



2008-11-24 19:36:00

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH-mm] kernel: add common endian load/store API

From: Harvey Harrison <[email protected]>
Subject: [PATCH] block: aoe switch to the new endian helpers

Add the necesary casts now that the unaligned helpers are typesafe
and switch to load_* where possible as it is more efficient.

Signed-off-by: Harvey Harrison <[email protected]>
---
Andrew, I thought it might be helpful to show what the new API will
look like as it gets used, this is an example conversion patch.

drivers/block/aoe/aoecmd.c | 30 +++++++++++++++---------------
drivers/block/aoe/aoenet.c | 4 ++--
2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 71ff78c..199494a 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -642,16 +642,16 @@ ataid_complete(struct aoedev *d, struct aoetgt *t, unsigned char *id)
u16 n;

/* word 83: command set supported */
- n = get_unaligned_le16(&id[83 << 1]);
+ n = load_le16_noalign((__le16 *)&id[83 << 1]);

/* word 86: command set/feature enabled */
- n |= get_unaligned_le16(&id[86 << 1]);
+ n |= load_le16_noalign((__le16 *)&id[86 << 1]);

if (n & (1<<10)) { /* bit 10: LBA 48 */
d->flags |= DEVFL_EXT;

/* word 100: number lba48 sectors */
- ssize = get_unaligned_le64(&id[100 << 1]);
+ ssize = load_le16_noalign((__le16 *)&id[100 << 1]);

/* set as in ide-disk.c:init_idedisk_capacity */
d->geo.cylinders = ssize;
@@ -662,12 +662,12 @@ ataid_complete(struct aoedev *d, struct aoetgt *t, unsigned char *id)
d->flags &= ~DEVFL_EXT;

/* number lba28 sectors */
- ssize = get_unaligned_le32(&id[60 << 1]);
+ ssize = load_le32_noalign((__le32 *)&id[60 << 1]);

/* NOTE: obsolete in ATA 6 */
- d->geo.cylinders = get_unaligned_le16(&id[54 << 1]);
- d->geo.heads = get_unaligned_le16(&id[55 << 1]);
- d->geo.sectors = get_unaligned_le16(&id[56 << 1]);
+ d->geo.cylinders = load_le16_noalign((__le16 *)&id[54 << 1]);
+ d->geo.heads = load_le16_noalign((__le16 *)&id[55 << 1]);
+ d->geo.sectors = load_le16_noalign((__le16 *)&id[56 << 1]);
}

if (d->ssize != ssize)
@@ -760,7 +760,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
u16 aoemajor;

hin = (struct aoe_hdr *) skb_mac_header(skb);
- aoemajor = get_unaligned_be16(&hin->major);
+ aoemajor = load_be16_noalign(&hin->major);
d = aoedev_by_aoeaddr(aoemajor, hin->minor);
if (d == NULL) {
snprintf(ebuf, sizeof ebuf, "aoecmd_ata_rsp: ata response "
@@ -772,7 +772,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)

spin_lock_irqsave(&d->lock, flags);

- n = get_unaligned_be32(&hin->tag);
+ n = load_be32_noalign(&hin->tag);
t = gettgt(d, hin->src);
if (t == NULL) {
printk(KERN_INFO "aoe: can't find target e%ld.%d:%012llx\n",
@@ -787,9 +787,9 @@ aoecmd_ata_rsp(struct sk_buff *skb)
snprintf(ebuf, sizeof ebuf,
"%15s e%d.%d tag=%08x@%08lx\n",
"unexpected rsp",
- get_unaligned_be16(&hin->major),
+ load_be16_noalign(&hin->major),
hin->minor,
- get_unaligned_be32(&hin->tag),
+ load_be32_noalign(&hin->tag),
jiffies);
aoechr_error(ebuf);
return;
@@ -854,7 +854,7 @@ aoecmd_ata_rsp(struct sk_buff *skb)
printk(KERN_INFO
"aoe: unrecognized ata command %2.2Xh for %d.%d\n",
ahout->cmdstat,
- get_unaligned_be16(&hin->major),
+ load_be16_noalign(&hin->major),
hin->minor);
}
}
@@ -982,7 +982,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
* Enough people have their dip switches set backwards to
* warrant a loud message for this special case.
*/
- aoemajor = get_unaligned_be16(&h->major);
+ aoemajor = load_be16_noalign(&h->major);
if (aoemajor == 0xfff) {
printk(KERN_ERR "aoe: Warning: shelf address is all ones. "
"Check shelf dip switches.\n");
@@ -996,7 +996,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
return;
}

- n = be16_to_cpu(ch->bufcnt);
+ n = load_be16(&ch->bufcnt);
if (n > aoe_maxout) /* keep it reasonable */
n = aoe_maxout;

@@ -1049,7 +1049,7 @@ aoecmd_cfg_rsp(struct sk_buff *skb)
spin_unlock_irqrestore(&d->lock, flags);
return;
}
- d->fw_ver = be16_to_cpu(ch->fwver);
+ d->fw_ver = load_be16(&ch->fwver);

sl = aoecmd_ata_id(d);

diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 9157d64..7a61d54 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -127,7 +127,7 @@ aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt,
skb_push(skb, ETH_HLEN); /* (1) */

h = (struct aoe_hdr *) skb_mac_header(skb);
- n = get_unaligned_be32(&h->tag);
+ n = load_be32_noalign(&h->tag);
if ((h->verfl & AOEFL_RSP) == 0 || (n & 1<<31))
goto exit;

@@ -139,7 +139,7 @@ aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt,
printk(KERN_ERR
"%s%d.%d@%s; ecode=%d '%s'\n",
"aoe: error packet from ",
- get_unaligned_be16(&h->major),
+ load_be16_noalign(&h->major),
h->minor, skb->dev->name,
h->err, aoe_errlist[n]);
goto exit;
--
1.6.0.4.1013.gc6a01