A patch was posted a bit ago by agruen which made a change to
include/linux/types.h changing aligned_u64 to __aligned_u64 and exposing
this new type to userspace.

http://marc.info/?l=linux-kernel&m=128316627912457&w=2

Everyone seemed to agree the patch was a good idea and was correct.  At
the moment this change only really affects network code, but I would
very much like to make use of this change in the notification tree.
Dave Miller did not apply the patch because "Someone has to first add
the types to linux/types.h, and that doesn't go through my tree."

http://marc.info/?l=linux-kernel&m=128634544524035&w=2

I'm a little stuck as to the right path forward.  I normally would have
had no qualms about adding __aligned_u64 to types.h in the notification
tree and pushing it to Linus next go-round and then the net tree could
convert and potentially drop the old aligned_u64 type (but again that
would be outside the net tree).  Since Dave isn't willing to add the
type and I don't want to get called too many bad names, I figured I
should try to find if there is some better way, maintainer, or tree who
should be adding this new type.

Who needs to sign off on a new type in types.h?  Who should add it?
Should I just ram it on in there myself, take any flames that come
along, and then let net finish their cleanups after I've been charred?
Any suggestions on the best course of action would be appreciated.

-Eric

--

[ message continues ]

I just ran into this problem (aligned_u64 not being exposed to user space),

When in doubt send it to Andrew I guess. I see you already did.

-Andi

--

[ message continues ]

On Thu, 14 Oct 2010 15:34:52 -0400

The usual approach here is someone sends it to me and I send it to
Linus ;)

If the change is simple, obviously safe and is needed in two or more
subsystem trees then I'll usually sneak it into mainline late in -rc,
simply to make everyone's life easier.  Of course, you could both agree
to merge the same patch into local trees and I assume that git will
sort it all out.

For this particular patch I'd suggest it be split into two: one adds
the new __aligned_u64 and friends.  The second patch kills off
aligned_u64 and friends.  I'd say the first four-liner would then be
safe for immediate merge and the cleanup patch can go in any old time.



Regarding the patch itself: it uses open-coded
__attribute__((aligned(...))), however we have the __aligned(...)
helpers in compiler.h.

I'm always a bit ambivalent about those helpers (__packed, etc). 
They're not a very kernely thing to do, but the gcc __attribute__
syntax really is a mouthful.

And adding a compiler.h dependency to the shared-with-userspace types.h
may not be practical or safe, dunno.


So if this works, I'd suggest preparing the simple four-liner with the
intention of an immediate merge.

--

[ message continues ]

We tinkered on types.h before, with the change originating in the Netfilter
subtree, and nobody, not even Dave, complained. (See v2.6.24-6165-gc82a5cb)
--

[ message continues ]

I'm doing the separation and will send part 1 to akpm in a bit.  Make it
easy and make sure noone gets upset   :)

-Eric

--

[ message continues ]

On Thu, 14 Oct 2010 23:26:34 +0200 (CEST)

It doesn't matter much at all what tree a change goes through.  What
matters more is that the appropriate people know about and see the
change.

For example, I never even knew that aligned_u64 and friends existed (it
got secretly merged via the netfilter tree, apparently).  So when I
review code (and I review a lot of code), I don't think to nag people

hm, what does that mean.
--

[ message continues ]

commit c82a5cb8b2b2ce15f1fb8add6772921b72da5943
Author: Jan Engelhardt <jengelh@computergmbh.de>
Date:   Thu Jan 31 03:57:36 2008 -0800

    linux/types.h: Use __u64 for aligned_u64

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--

[ message continues ]

I would be interested in knowing whether you - in whichever subsystems
you happen to be active - would even need aligned_u64. Right now,

When these typedefs were introduced, I am sure that any open-coded

Eh, it's a git commit identifier. When people throw around with these
(or their longer, 40-char variants), it is suggested to use `git log
-1 -p v2.6.24-6165-gc82a5cb` to see the details.
Or equivalent ways, such as visiting
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=v2.6.2...
--

[ message continues ]

On Thu, 14 Oct 2010 23:50:34 +0200 (CEST)

Stranger things have happened.

include/linux/net_dropmon.h:net_dm_config_entry can perhaps be
converted btw.

--

[ message continues ]

[ I'm sure you know this, but just for any innocent non-git
by-standers out there ]

Well, technically it's more than just a git ID. It actually has some
descriptive value, and it's generated by "git descibe".

In something like the above, the only part git cares about in this
case is the (shortened) SHA1 hash, ie the "c82a5cb" part. Everything
else is just a mostly human-readable description, which boils down to
"based on v2.6.24, there's 6165 additional commits, and git commit
c82a5cb is where you end up". So the output is designed to (a) give
some helpful information even for intermediate places that aren't
exact tagged releases and (b) also work with other SCM's (that "-g"
part there is for "git" - but scripts/setlocalversion is able to give
somewhat reasonable output for hg and svn too.

Extended background for non-git people: git _internally_ only uses the
160-bit SHA1 (which in its full ASCII form is 40 hex characters). But
because that is so human-unfriendly, there are various human-readable
ways to express it:

 - Just a shortened unique version, usually 7-12 hex characters. 12
hex characters is already unique in practice for pretty much any
reasonable project, and is much easier to write.

   I tend to use the 12-character short version in commit messages,
for example. The full 40-character SHA1 makes it hard to do any sane
line breaks with in the commit message.

 - named tags or branches ("v2.6.36-rc7", or "master" etc)

 - The <random human-readable noise> + "-g" + <shortened SHA1> thing
described above (output by "git describe", and on input git ignores
the human-readable part), where the human-readable part tries to give
a release as a base for it.

 - a "git revision expression", where you can mix and match SHA1's or
named tags/branches, and their parenthood relationsips.

So that "v2.6.24-6165-gc82a5cb" could be expressed as

 - c82a5cb8b2b2ce15f1fb8add6772921b72da5943 (full SHA1)

 - c82a5cb8b2b2 (abbreviated to 12 hex chars)

 - ...
[ message continues ]

I tend to use describe --tags (rev expr) output; unlike 12-char
versions, they cannot become ambiguous at any time. If git also used
the initial portion of regular describes ("v2.6.24-1234-"), it would
be similarly strengthened. Yes, I'm just theoretically projecting


I beg to differ. It tells you that the certain commit was included
for v2.6.25-rc1. That's much more telling than v2.6.24-1234-gabcdef1.
Especially when a branch has not been recently merged, it could be
showing v2.6.22-9876-gxxx or anything further back in time.


Jan
--

[ message continues ]

Hi Linus,


I have had one case I hit where an abbreviated SHA1 (which may have been
only 7 hex chars) was unique in your tree, but not in linux-next :-(  I
think it was unique after one more character, though.   Having this
happen was a pain, because git reacted badly to being given a non-unique
SHA1 to look up.  i.e. it did *not* say the expected "this sha1 is not
unique".   However, now it does, so things are getting better.  It would
probably be nice if it told you the "matching" objects ...

So I would support using (at least) 12 char abbreviations in commit
messages (and any other long lived context).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[ message continues ]

Using aligned_u64 is good practice to avoid problems with the 
32bit/64bit compat layer. I would recommend it to anyone 
adding a new user space interface passing a 64bit value.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.
--

[ message continues ]

What "problems" does it prevent when used for this?
--

[ message continues ]

64-bit values align to 4-byte boundaries on some 32-bit architectures like x86 
and to 8-byte boundaries on 64-bit architetures.  The new __aligned_64 type 
enforces 8-byte alignment and so structs containing __aligned_64 values have 
the same alignment on 32-bit and 64-bit architectures.  No conversions are 
necessary between 32-bit user-space and a 64-bit kernel.

Andreas
--

[ message continues ]

> 64-bit values align to 4-byte boundaries on some 32-bit architectures like x86 
AFAIK it's only on x86, no other architecture made this mistake in their

Rest looks good and could be put into Andrew's comment.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.
--

[ message continues ]

I don't know of any other examples; all the architectures I tried "got it 
right" except x86.

Andreas
--

[ message continues ]

I don't think x86 has done anything wrong. It processes stuff in
32-bit chunks, so why should one waste space for aligning a 64-bit
entity to 8? On sparcv9, a long double also has an alignment
smaller than its size.

If you don't like that, you can always patch up gcc, because
alignment is after all a decision made by the compiler. In that
regard, it might be worth checking out the gcc machine description
files and find out why people specified an alignment of 8 for
uint64_t anything but i?86.
--

[ message continues ]

Well, new interfaces in networking are using netlink, which has its own 
unlucky ideas of alignment.
--

[ message continues ]