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

From: Harvey Harrison
Date: Mon Mar 02 2009 - 19:38:26 EST


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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/