Re: [Patch 04/18] include/linux/logfs.h
On Tue, 2007-06-05 at 20:49 +0200, Segher Boessenkool wrote:
It would be better if GCC had a 'nopadding' attribute which gave us
what we need without the _extra_ implications about alignment.
That's impossible; removing the padding from a struct
_will_ make accesses to its members unaligned (think
about arrays of that struct).
It _might_ make accesses to _some_ of its members unaligned.
It _will_ make accesses to _at least one_ of the members
unaligned, in the second array element.
It _might_, but not necessarily.
Won't a simple struct { uint16_t } get padded to a size of 4 bytes on
ARM? Even if I'm misremembering that, I certainly can't guarantee that
such a thing will _never_ happen on any newly-invented ABI. If you had
'nopadding' instead of 'packed', then there's no need to emit code to
handle unaligned loads.
Likewise, with a struct which looks like this:
{ uint32_t, uint16_t, uint16_t, uint32_t, uint32_t }
I cannot _guarantee_ that there will never be an architecture on which
we'll end up using 32 bits of space for each uint16_t. I might _guess_
that and hope, but that's precisely the kind of moronic empirical
behaviour which caused Linux to have so many problems with newer
compilers in the past. If it isn't _guaranteed_ then I shouldn't be
assuming it.
Thus, I want a way to tell the compiler not to insert _any_ padding. But
without the compiler making extra inferences about the whole thing being
found at arbitrary alignments.
And if I had something like this (which is admittedly contrived, but
hardware people _do_ do stupid things to us):
{ uint32_t, uint8_t, uint16_t, uint8_t, uint32_t, uint32_t }
With the 'packed' attribute the compiler would assume arbitrary
alignment of all the 32-bit integers. But in reality it's only necessary
for the uint16_t in the middle. A 'nopadding' attribute would deal with
that correctly.
--
dwmw2
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
David Woodhouse [EMAIL PROTECTED] writes:
Won't a simple struct { uint16_t } get padded to a size of 4 bytes on
ARM? Even if I'm misremembering that, I certainly can't guarantee that
such a thing will _never_ happen on any newly-invented ABI. If you had
'nopadding' instead of 'packed', then there's no need to emit code to
handle unaligned loads.
You can add aligned(N) to increase the alignment again.
Andreas.
--
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Wednesday 06 June 2007, David Woodhouse wrote:
And if I had something like this (which is admittedly contrived, but
hardware people _do_ do stupid things to us):
{ uint32_t, uint8_t, uint16_t, uint8_t, uint32_t, uint32_t }
With the 'packed' attribute the compiler would assume arbitrary
alignment of all the 32-bit integers. But in reality it's only necessary
for the uint16_t in the middle. A 'nopadding' attribute would deal with
that correctly.
I would argue that a newly invented 'nopadding' attribute should reject
such a structure as invalid, because it should not let members be
unaligned. Unfortunately, this also gets tricky if you consider
struct {
uint32_t a;
uint64_t b;
uint32_t c;
};
which does have an unaligned member by default in i386, but not on
any modern platform.
Arnd
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
It would be better if GCC had a 'nopadding' attribute which gave us what we need without the _extra_ implications about alignment. That's impossible; removing the padding from a struct _will_ make accesses to its members unaligned (think about arrays of that struct). Segher - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Tue, 2007-06-05 at 17:49 +0200, Segher Boessenkool wrote: It would be better if GCC had a 'nopadding' attribute which gave us what we need without the _extra_ implications about alignment. That's impossible; removing the padding from a struct _will_ make accesses to its members unaligned (think about arrays of that struct). It _might_ make accesses to _some_ of its members unaligned. That's why I said 'without the __EXTRA__ implications about alignment'. Obviously the lack of padding has its own implications, but we don't necessarily need to assume that the struct may be at arbitrary locations. -- dwmw2 - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
It would be better if GCC had a 'nopadding' attribute which gave us what we need without the _extra_ implications about alignment. That's impossible; removing the padding from a struct _will_ make accesses to its members unaligned (think about arrays of that struct). It _might_ make accesses to _some_ of its members unaligned. It _will_ make accesses to _at least one_ of the members unaligned, in the second array element. That's why I said 'without the __EXTRA__ implications about alignment'. Obviously the lack of padding has its own implications, but we don't necessarily need to assume that the struct may be at arbitrary locations. The compiler does though, if it can't prove otherwise. What would nopadding buy us, anyway? Segher - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
Segher Boessenkool wrote: It would be better if GCC had a 'nopadding' attribute which gave us what we need without the _extra_ implications about alignment. That's impossible; removing the padding from a struct _will_ make accesses to its members unaligned (think about arrays of that struct). And many platforms happily support unaligned CPU access in hardware at a price in performance, while other support it in software at great cost in performance. None of that maps into impossible, Some i/o hardware may not support at all and require some bounce buffering, at cost in memory and CPU. None of that equates with impossible. It is readily argued that it could mean inadvisable on some architectures, slow as government assistance and ugly as the north end of a south-bound hedgehog, but it's not impossible. Do NOT take this to mean I think it would be a good thing in a Linux kernel, or that it should be added to gcc, but in some use like embedded applications where memory use is an important cost driver, people are probably doing it already by hand to pack struct arrays into minimal bytes. It's neither impossible nor totally useless. -- bill davidsen [EMAIL PROTECTED] CTO TMR Associates, Inc Doing interesting things with small computers since 1979 - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Sun, 3 June 2007 23:42:25 +0200, Arnd Bergmann wrote:
On Sunday 03 June 2007, Jörn Engel wrote:
+struct logfs_je_spillout {
+ __be64 so_segment[0];
+}__packed;
All the on-disk data structures you define in this file have naturally
aligned members, so the __packed attribute is not needed.
Amen. It is purely paranoia and I don't even know who is out to get me.
However, I think it causes gcc to generate larger and slower code
on some architectures, because now it has to assume that the data
structure itself has no more than byte alignment.
I'd simply remove all instances of __packed therefore. In order
to verify that you got it right in all cases, build with
'-Wpadded -Wpacked'.
Fine with me. Will do.
Jörn
--
Ninety percent of everything is crap.
-- Sturgeon's Law
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Mon, 2007-06-04 at 11:12 +0200, Jörn Engel wrote:
On Sun, 3 June 2007 23:42:25 +0200, Arnd Bergmann wrote:
On Sunday 03 June 2007, Jörn Engel wrote:
+struct logfs_je_spillout {
+ __be64 so_segment[0];
+}__packed;
All the on-disk data structures you define in this file have naturally
aligned members, so the __packed attribute is not needed.
Amen. It is purely paranoia and I don't even know who is out to get me.
You can _never_ know who is out to get you, or what architecture we'll
be ported to next week.
The advice don't tell the compiler what you want unless you _know_
it'll do the wrong thing otherwise runs counter to everything we've
learned, slowly and painfully, over the last few years.
We should never rely on compiler behaviour which is undocumented and
unrequired. Even if you know that the ABI forces it to continue to do
the right thing on the platforms you _currently_ care about, it might
not do it on new platforms (or existing platforms you didn't manage to
test).
It would be better if GCC had a 'nopadding' attribute which gave us what
we need without the _extra_ implications about alignment. In the absence
of that, though, you should at _least_ have a check on the size of the
structure if you're doing to drop the packed attribute.
--
dwmw2
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Mon, 4 June 2007 14:38:23 +0100, David Woodhouse wrote:
On Mon, 2007-06-04 at 11:12 +0200, Jörn Engel wrote:
On Sun, 3 June 2007 23:42:25 +0200, Arnd Bergmann wrote:
On Sunday 03 June 2007, Jörn Engel wrote:
+struct logfs_je_spillout {
+ __be64 so_segment[0];
+}__packed;
All the on-disk data structures you define in this file have naturally
aligned members, so the __packed attribute is not needed.
Amen. It is purely paranoia and I don't even know who is out to get me.
You can _never_ know who is out to get you, or what architecture we'll
be ported to next week.
The advice don't tell the compiler what you want unless you _know_
it'll do the wrong thing otherwise runs counter to everything we've
learned, slowly and painfully, over the last few years.
We should never rely on compiler behaviour which is undocumented and
unrequired. Even if you know that the ABI forces it to continue to do
the right thing on the platforms you _currently_ care about, it might
not do it on new platforms (or existing platforms you didn't manage to
test).
It would be better if GCC had a 'nopadding' attribute which gave us what
we need without the _extra_ implications about alignment. In the absence
of that, though, you should at _least_ have a check on the size of the
structure if you're doing to drop the packed attribute.
Adding a size check is simple enough. But given all this I'll put it to
the very end of my todo list. There are many other optimizations
remaining.
Jörn
--
Joern's library part 14:
http://www.sandpile.org/
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch 04/18] include/linux/logfs.h
On Sunday 03 June 2007, Jörn Engel wrote:
+struct logfs_je_spillout {
+ __be64 so_segment[0];
+}__packed;
All the on-disk data structures you define in this file have naturally
aligned members, so the __packed attribute is not needed.
However, I think it causes gcc to generate larger and slower code
on some architectures, because now it has to assume that the data
structure itself has no more than byte alignment.
I'd simply remove all instances of __packed therefore. In order
to verify that you got it right in all cases, build with
'-Wpadded -Wpacked'.
Arnd
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
