2009-03-03 00:07:47

by Harvey Harrison

[permalink] [raw]
Subject: [PATCH 2/2] unaligned: add load/store_{endian}_noalign API

The existing get/put_unaligned_{endian} API is not typed, the
arguments are in the oposite of the usual order, and the name
get/put usually implies taking/releasing a reference.

Add a typed API with the usual argument ordering and use load/store,
this API is also consistent with the aligned versions.

Signed-off-by: Harvey Harrison <[email protected]>
---
include/linux/unaligned/generic.h | 60 +++++++++++++++++++++++++++++++++++++
1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/include/linux/unaligned/generic.h b/include/linux/unaligned/generic.h
index 02d97ff..1e3e267 100644
--- a/include/linux/unaligned/generic.h
+++ b/include/linux/unaligned/generic.h
@@ -1,6 +1,66 @@
#ifndef _LINUX_UNALIGNED_GENERIC_H
#define _LINUX_UNALIGNED_GENERIC_H

+static inline u16 load_le16_noalign(const __le16 *p)
+{
+ return get_unaligned_le16(p);
+}
+
+static inline u32 load_le32_noalign(const __le32 *p)
+{
+ return get_unaligned_le32(p);
+}
+
+static inline u64 load_le64_noalign(const __le64 *p)
+{
+ return get_unaligned_le64(p);
+}
+
+static inline u16 load_be16_noalign(const __be16 *p)
+{
+ return get_unaligned_be16(p);
+}
+
+static inline u32 load_be32_noalign(const __be32 *p)
+{
+ return get_unaligned_be32(p);
+}
+
+static inline u64 load_be64_noalign(const __be64 *p)
+{
+ return get_unaligned_be64(p);
+}
+
+static inline void store_le16_noalign(__le16 *p, u16 val)
+{
+ return put_unaligned_le16(val, p);
+}
+
+static inline void store_le32_noalign(__le32 *p, u32 val)
+{
+ return put_unaligned_le32(val, p);
+}
+
+static inline void store_le64_noalign(__le64 *p, u64 val)
+{
+ return put_unaligned_le64(val, p);
+}
+
+static inline void store_be16_noalign(__be16 *p, u16 val)
+{
+ return put_unaligned_be16(val, p);
+}
+
+static inline void store_be32_noalign(__be32 *p, u32 val)
+{
+ return put_unaligned_be32(val, p);
+}
+
+static inline void store_be64_noalign(__be64 *p, u64 val)
+{
+ return put_unaligned_be64(val, p);
+}
+
/*
* Cause a link-time error if we try an unaligned access other than
* 1,2,4 or 8 bytes long
--
1.6.2.rc2.289.g2fa25


2009-03-03 00:19:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 2/2] unaligned: add load/store_{endian}_noalign API



On Mon, 2 Mar 2009, Harvey Harrison wrote:
>
> Add a typed API with the usual argument ordering and use load/store,
> this API is also consistent with the aligned versions.

This naming is horrible. It doesn't match the normal pattern we have in
the kernel.

Why not just try to fix the current (well-named) "get_unaligned_le16()"
problems, instead of introducing a new (and badly named) version of them.

So NAK on both of these.

Linus

2009-03-03 00:38:26

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 2/2] unaligned: add load/store_{endian}_noalign API

On Mon, 2009-03-02 at 16:18 -0800, Linus Torvalds wrote:
>
> On Mon, 2 Mar 2009, Harvey Harrison wrote:
> >
> > Add a typed API with the usual argument ordering and use load/store,
> > this API is also consistent with the aligned versions.
>
> This naming is horrible. It doesn't match the normal pattern we have in
> the kernel.
>
> Why not just try to fix the current (well-named) "get_unaligned_le16()"
> problems, instead of introducing a new (and badly named) version of them.
>
> So NAK on both of these.

Well, the current API can be made typesafe, and then the sparse fallout
can be fixed...I introduced the new API to make it opt-in and then
phase out the old API over time to avoid that. As I already had a new
API, 'fixing' the argument order was doable as well. Comments from AKPM
made me drop the get/put name as well, and load/store seemed the most
natural replacement.

The other reason was currently we have:

Aligned:
le16_to_cpup

Unaligned:
get_unaligned_le16
put_unaligned_le16

And although the le16_to_cpup is at worst the same as le16_to_cpu(*p) it
is more efficient on arches like powerpc and sparc that have a load-swap
instruction...but few people use it because of the goofy name. Same goes
for the get/put_unaligned, people don't use them and just open-code the
byteswapping whenever there is going to be unaligned access.

So I was hoping to make it easier/more obvious to use as well.

Aligned:
load_le16
store_le16

Unaligned (which degrades to aligned on arches without alignment restrictions)
load_le16_noalign
store_le16_noalign

Cheers,

Harvey