Grant Grundler | 4 Jun 19:43

Re: Re: [parisc-linux] [PATCH] usb/input/hid-core.c extract() brain damage

On Sat, Jun 04, 2005 at 11:06:52AM -0400, Alan Stern wrote:
> In spite of the total overall number of changes required, wouldn't it be 
> much simpler to have a suite of routines (inlines or macros) like:
> 
> 	get_16, put_16, get_le16, put_le16, get_be16, put_be16
> 	get_32, put_32, get_le32, put_le32, get_be32, put_be32
> 	get_64, put_64, get_le64, put_le64, get_be64, put_be64
> 
> 	in short, {get|put}_{|le|be}{16|32|64}

Sorry, no. The architectures that trap on misaligned accesses have to
handle that in the kernel. And even though the implementations
get simpler, a plethoria of interfaces just clutters up the general
device driver API. For that reason alone I wouldn't want it.

The endian conversion (swap) macros are PITA already.
I'll argue the swap API should be simplified to four macros:
	cpu_to_le(x), cpu_to_be(x), le_to_cpu(x), be_to_cpu(x)

and force the caller to cast to the right size. switch(sizeof(x)) would
then select the right arch specific variant. I'll figure out how to
pitch this to linux-arch...and then see how far it gets batted back. :^)

> for native, little-endian, and big-endian unaligned accesses?  The generic 
> definitions are extremely simple and architecture-specific headers could 
> easily provide optimized versions.
> 
> I realize this doesn't fit in very well with the current state of the API, 
> but IMHO it would be a big improvement.

(Continue reading)

Alan Stern | 5 Jun 00:59
Picon
Favicon

Re: Re: [parisc-linux] [PATCH] usb/input/hid-core.c extract() brain damage

On Sat, 4 Jun 2005, Grant Grundler wrote:

> On Sat, Jun 04, 2005 at 11:06:52AM -0400, Alan Stern wrote:
> > In spite of the total overall number of changes required, wouldn't it be 
> > much simpler to have a suite of routines (inlines or macros) like:
> > 
> > 	get_16, put_16, get_le16, put_le16, get_be16, put_be16
> > 	get_32, put_32, get_le32, put_le32, get_be32, put_be32
> > 	get_64, put_64, get_le64, put_le64, get_be64, put_be64
> > 
> > 	in short, {get|put}_{|le|be}{16|32|64}
> 
> Sorry, no. The architectures that trap on misaligned accesses have to
> handle that in the kernel. And even though the implementations
> get simpler, a plethoria of interfaces just clutters up the general
> device driver API. For that reason alone I wouldn't want it.
> 
> The endian conversion (swap) macros are PITA already.
> I'll argue the swap API should be simplified to four macros:
> 	cpu_to_le(x), cpu_to_be(x), le_to_cpu(x), be_to_cpu(x)
> 
> and force the caller to cast to the right size. switch(sizeof(x)) would
> then select the right arch specific variant. I'll figure out how to
> pitch this to linux-arch...and then see how far it gets batted back. :^)

I don't really follow your argument.  For example, consider the swap API.  
There's nothing to stop you from defining, in your own code, those four 
macros.  Have them perform the appropriate conversion based on the type of 
x.  You don't have to use the full existing API if you don't want to.

(Continue reading)

Grant Grundler | 5 Jun 09:58

Re: Re: [parisc-linux] [PATCH] usb/input/hid-core.c extract() brain damage

On Sat, Jun 04, 2005 at 06:59:53PM -0400, Alan Stern wrote:
> The point I was driving at is that we currently have separate APIs for 
> byte swapping and for unaligned access, and it would make a lot of sense 
> to combine them into a single API.  Knowing that the bytes have to be 
> swapped _and_ that the value isn't aligned correctly should allow us to 
> use more efficient code (in some cases) than a simple unaligned access 
> followed by a byte swap.

Ah ok. I missed that, sorry. *Some* architectures might be able
to provide more efficient implementations if they were combined.
But I don't think parisc is one of those. If I'm lucky, someone
will prove me wrong. :^)
Since many of the arches now use asm-generic/unaligned.h, I
can't say which other ones would benefit.

Just avoiding the trap is already a *huge* improvement. I'm alot
less worried about shaving a few cycles of here and there unless
it's really a critical code path (e.g. TLB miss handler or DMA
mapping support).

> I also don't understand your point about architectures that trap unaligned 
> accesses.

Architectures that trap unaligned accesses, *must* handle them.
If they don't, then networking won't work either.
I believe avoiding the trap is a good thing.

>   Let's put aside for one moment the question of whether it's 
> better to manually load the pieces of an unaligned value versus incurring 
> the overhead of a trap and a kernel fixup -- the arch-specific code is 
(Continue reading)


Gmane