get/put unaligned helpers

get/put unaligned helpers

[Christoph Hellwig]

I just loked into using the new get/put unaligned helpers.  I must
say that I'm really unhappy about the lack of typing there.  We did
put all the sparse infrastructure in place to make sure we do have
strong typechecking for LE/Be types, but using these helpers defeats
that.

Can you please make these properly typed?

Re: get/put unaligned helpers

[Harvey Harrison]

On Wed, 2009-02-11 at 15:35 +0100, Christoph Hellwig wrote:
> I just loked into using the new get/put unaligned helpers.  I must
> say that I'm really unhappy about the lack of typing there.  We did
> put all the sparse infrastructure in place to make sure we do have
> strong typechecking for LE/Be types, but using these helpers defeats
> that.
> 
> Can you please make these properly typed?

Sorry, real life got in the way, I had a series in -mm for a few months
that did the typing and fixed the argument ordering by providing a
load/store API and then moving the existing users across, but that got
delayed while the (aligned) byteswapping patches were made suitable
for mainline.

I'll get that series revived shortly.

Harvey

Re: get/put unaligned helpers

[Boaz Harrosh]

Harvey Harrison wrote:
> On Wed, 2009-02-11 at 15:35 +0100, Christoph Hellwig wrote:
>> I just loked into using the new get/put unaligned helpers.  I must
>> say that I'm really unhappy about the lack of typing there.  We did
>> put all the sparse infrastructure in place to make sure we do have
>> strong typechecking for LE/Be types, but using these helpers defeats
>> that.
>>
>> Can you please make these properly typed?
> 
> Sorry, real life got in the way, I had a series in -mm for a few months
> that did the typing and fixed the argument ordering by providing a
> load/store API and then moving the existing users across, but that got
> delayed while the (aligned) byteswapping patches were made suitable
> for mainline.
> 
> I'll get that series revived shortly.
> 
> Harvey
> 

Speaking of which. When all this is done do you intend to also export
the unaligned.h headers to user-mode just like the aligned Endian helpers?

That could help in a couple of projects, so you have my vote.

Thanks
Boaz

Re: get/put unaligned helpers

[Maciej W. Rozycki]

On Thu, 12 Feb 2009, Boaz Harrosh wrote:

> Speaking of which. When all this is done do you intend to also export
> the unaligned.h headers to user-mode just like the aligned Endian helpers?
> 
> That could help in a couple of projects, so you have my vote.

 Why would you need that?  GCC provides a portable, platform-independent 
way.  It has had it for ten years at the very least.

  Maciej

Re: get/put unaligned helpers

[Boaz Harrosh]

Maciej W. Rozycki wrote:
> On Thu, 12 Feb 2009, Boaz Harrosh wrote:
> 
>> Speaking of which. When all this is done do you intend to also export
>> the unaligned.h headers to user-mode just like the aligned Endian helpers?
>>
>> That could help in a couple of projects, so you have my vote.
> 
>  Why would you need that?  GCC provides a portable, platform-independent 
> way.  It has had it for ten years at the very least.
> 

Do you mean the __built_in_swabXX() and all these guys.

I was under the impression they need to be aligned because otherwise that means
something is done wrong. Because the aligned version on lots of CPUs is one instruction
where unaligned access is better, or must, be emulated (byte accessed).

Assembly wise the two accesses are different and sometimes the compiler has no way to
know, where the programmer can know for sure.

But I like to be educated any day, please explain what to use when.

>   Maciej

Thanks
Boaz

Re: get/put unaligned helpers

[Maciej W. Rozycki]

On Thu, 12 Feb 2009, Boaz Harrosh wrote:

> I was under the impression they need to be aligned because otherwise that means
> something is done wrong. Because the aligned version on lots of CPUs is one instruction
> where unaligned access is better, or must, be emulated (byte accessed).
> 
> Assembly wise the two accesses are different and sometimes the compiler has no way to
> know, where the programmer can know for sure.
> 
> But I like to be educated any day, please explain what to use when.

 The compiler is always told by the programmer what alignment to expect -- 
the language defines the alignment of each data type it may use.  For 
example for integer types the alignment is always equal to the size of the 
type.  Casting a pointer and subsequently accessing data pointed at leads 
to undefined behaviour if it increases the required alignment -- you'll 
get a warning from GCC if you ask it for this class of warnings.  If you 
want to access this data anyway you have to tell the compiler the data is 
not correctly aligned.

 You can use packed structs (or unions) to ask GCC to emit code sequences 
suitable for accessing unaligned data entities of various sizes.  For 
example with the MIPS processor if a 32-bit-wide entity is read via a 
member of a packed type, a sequence consisting of an LWL (load word left) 
and an LWR (load word right) instruction will be generated to perform two 
complementing bus read cycles with byte enables appropriately set to fetch 
the two parts of the entity spanning a 32-bit alignment boundary.  
Normally a single LW (load word) instruction would be used to fetch the 
entity, but that instruction would trap if the address used to access it 
was not aligned.  For platforms where hardware is capable of doing 
unaligned accesses with no special treatment the use of packed types does 
not imply a change in the emitted code.

 Of course letting GCC sort out unaligned accesses itself has a lot of 
advantages, for example other instructions may get scheduled between the 
LWL and LWR mentioned above as GCC sees fit; the two have no special 
scheduling requirements with respect to each other (the reverse is 
actually the case -- care has been taken since the beginning of the 
architecture so that making them adjacent to each other is permitted and 
incurs no performance penalty), but other instructions which surround them 
may and making such a reorder may improve performance.  Such accesses may 
get merged or killed too as any other ones.

 Linux seems to put a lot of infrastructure around it, but in reality it 
is just a couple of lines of common code; I suppose it is there more to 
make people aware of the issue (all the world is not x86) than to invent 
something new.  And also Linux is a single program so it makes sense not 
to scatter duplicates and have local copies of the same code for each 
platform/subsystem/driver/etc.  You'd do the same with any other program, 
but promoting the couple of lines to become a system header sounds like an 
overkill to me.

  Maciej

Re: get/put unaligned helpers

[Stefan Richter]

Boaz Harrosh wrote:
> When all this is done do you intend to also export
> the unaligned.h headers to user-mode just like the aligned Endian helpers?

Neither the aligned nor unaligned endian conversion helpers are
interfaces between kernel and userland, hence should not be exported.
Export of the aligned ones is a mistake of the past.  IIRC there were
different opinions on whether this mistake should be corrected; it seems
it won't.
-- 
Stefan Richter
-=====-==--= --=- -==--
http://arcgraph.de/sr/

Re: get/put unaligned helpers

[Harvey Harrison]

On Thu, 2009-02-12 at 10:50 +0200, Boaz Harrosh wrote:
> Harvey Harrison wrote:

> Speaking of which. When all this is done do you intend to also export
> the unaligned.h headers to user-mode just like the aligned Endian helpers?
> 

No.

Harvey

Re: get/put unaligned helpers

[Harvey Harrison]

On Wed, 2009-02-11 at 15:35 +0100, Christoph Hellwig wrote:
> I just loked into using the new get/put unaligned helpers.  I must
> say that I'm really unhappy about the lack of typing there.  We did
> put all the sparse infrastructure in place to make sure we do have
> strong typechecking for LE/Be types, but using these helpers defeats
> that.
> 
> Can you please make these properly typed?

Christoph, here's a quick summary of comments on the unaligned access bits
I've previously submitted, please let me know if you have any additional ones.

Proposed API:

u16 load_le16_noalign(const __le16 *p)
u32 load_le32_noalign(const __le32 *p)
u64 load_le64_noalign(const __le64 *p)
u16 load_be16_noalign(const __be16 *p)
u32 load_be32_noalign(const __be32 *p)
u64 load_be64_noalign(const __be64 *p)

void store_le16_noalign(__le16 *p, u16 val)
void store_le32_noalign(__le32 *p, u32 val)
void store_le64_noalign(__le64 *p, u64 val)
void store_be16_noalign(__be16 *p, u16 val)
void store_be32_noalign(__be32 *p, u32 val)
void store_be64_noalign(__be64 *p, u64 val)

I'd also like to offer aligned versions like load_le16() store_le16()
to make it symmetric.  Then for arches that have no alignment constraints
the unaligned helpers would degrade to the aligned helpers.  load_le16()
already exists as le16_to_cpup, but store_le16 would be a new API.

Now, from discussions with the SCSI-folks, they really aren't interested in a
typesafe interface as most of the time they are reading/writing into a u8 buffer
at some offset.  It seems a lot of the other places currently in-kernel also
do the same.  So I'm tempted to keep offering a version wrapped around the
typesafe versions that takes a u8 *.

As they were not interested in doing the following:

load_be16_noalign((__be16 *)&buf[offset]);

Perhaps a wrapper could be provided:

load_be16_offset(u8 *buf, int offset)

Thoughts?

Harvey