this_cpu_cmpxchg_double support

+ * cmpxchg_double replaces two adjacent scalars at once. The first two
+ * parameters are per cpu variables which have to be of the same size.
+ * A truth value is returned to indicate success or
+ * failure (since a double register result is difficult to handle).
+ * There is very limited hardware support for these operations. So only certain
+ * sizes may work.

* Christoph Lameter ([email protected]) wrote:
[...]

Quoted 6 lines

+ * cmpxchg_double replaces two adjacent scalars at once. The first two
+ * parameters are per cpu variables which have to be of the same size.
+ * A truth value is returned to indicate success or
+ * failure (since a double register result is difficult to handle).
+ * There is very limited hardware support for these operations. So only certain
+ * sizes may work.

What's the issue with returning the value read by cmpxchg_double in addition to the boolean ? "returning" it per se might be an issue, but you could add 2 more parameters to the macros that take the addresses of these outputs. Returning the values read by cmpxchg instead of just the boolean result helps removing the extra reads from cmpxchg loops, which is why I think it's a shame to just return the boolean result.

Thoughts ?

Introduce this_cpu_cmpxchg_double. this_cpu_cmpxchg_double() allows the comparision between two consecutive words and replaces them if there is a match.

	bool this_cpu_cmpxchg_double(pcp1, pcp2,
		old_word1, old_word2, new_word1, new_word2)

this_cpu_cmpxchg_double does not return the old value (difficult since there are two words) but a boolean indicating if the operation was successful.

The first percpu variable must be double word aligned!

Quoted 7 lines · Christoph Lameter

Introduce this_cpu_cmpxchg_double. this_cpu_cmpxchg_double() allows the comparision between two consecutive words and replaces them if there is a match.

	bool this_cpu_cmpxchg_double(pcp1, pcp2,
		old_word1, old_word2, new_word1, new_word2)

Quoted 5 lines

this_cpu_cmpxchg_double does not return the old value (difficult since there are two words) but a boolean indicating if the operation was successful.

The first percpu variable must be double word aligned!

I really truly hate this interface. The whole notion of passing two pointers where only one is really used, is just painful.

Quoted 8 lines · H. Peter Anvin

Quoted 7 lines · Christoph Lameter

Introduce this_cpu_cmpxchg_double. this_cpu_cmpxchg_double() allows the comparision between two consecutive words and replaces them if there is a match.

	bool this_cpu_cmpxchg_double(pcp1, pcp2,
		old_word1, old_word2, new_word1, new_word2)

Quoted 8 lines

Quoted 5 lines

this_cpu_cmpxchg_double does not return the old value (difficult since there are two words) but a boolean indicating if the operation was successful.

The first percpu variable must be double word aligned!

I really truly hate this interface. The whole notion of passing two pointers where only one is really used, is just painful.

Well the second cpu variable is just there for correctness checking. That way we can verify that the types are compatible for the second word and that the second word was properly placed relative to the first one. It also helps the reader in the source because it shows explicitly which two words are modified by the operation.

If you look at the prior patch series at the use case you will see that s->cpu_slab->tid was not referred to in the cmpxchg operation but implicitly modified. That is bad and was the initial motivation to require both to be specified.

During debugging I had a couple of problems with the way the compiler placed those two fields that only became evident after painful debugging sessions. With the checks possible only because both are specifiec there is an explicit failure if either of the two cpu variables is specified in such a way that the cmpxchg32b cannot work.

I have to admit that I also hate the current interface for two reasons:

a) the way it splits the target address in two.

b) the loss of the value read (the fact that the only current user of this API does not need the value returned seems like a very weak argument to define an API).

I'm probably missing important compiler trickiness here, but why are we not rather relying on forced compiler alignment to declare the target address type ? e.g.:

typedef struct {
        unsigned long v[2];
} __attribute__((aligned(2 * sizeof(long)))) cmpxchg_double_t;

For the usage you are doing within the allocator code, you can use an union (the bitfield sizes are completely arbitrary).

union alloc_cd {
        cmpxchg_double_t cd;
        struct {
                void *ptr;
                unsigned long seq:48;
                unsigned long cpu:16;
        } v;
};

So this_cpu_cmpxchg_double would expect the type "cmpxchg_double_t *" as its first argument.

One way to manage to return the value read would be to pass a pointer to "old_word1-2" as argument rather than the value itself, and return the value read in these locations. Anyway, we won't need the old words for comparison with the return value, because the returned "bool" takes care of telling us if they differ.

Adding the types just for clarity sake (should be handled differently in your macro scheme of course), this_cpu_cmpxchg_double would look like:

typedef struct {
        unsigned long v[2];
} __attribute__((aligned(2 * sizeof(long)))) cmpxchg_double_t;

bool this_cpu_cmpxchg_double(cmpxchg_double_t *pcp,
unsigned long *old_ret_word1, unsigned long *old_ret_word2, unsigned long new_word1, unsigned long new_word2)

Thoughts ?

Quoted 1 line · Mathieu Desnoyers

I have to admit that I also hate the current interface for two reasons:

Quoted 3 lines

b) the loss of the value read (the fact that the only current user of this API does not need the value returned seems like a very weak argument to define an API).

The other user of cmpxchg_double that I have in my tree also does not have the need. Repeatability is not as simple to implement as with a single word cmpxchg.

Hello,

Quoted 3 lines · Christoph Lameter

Quoted 1 line · Mathieu Desnoyers

I have to admit that I also hate the current interface for two reasons:

Call me weird but I like this one than others. It sure is ugly but the operation itself isn't a particularly pretty so it kinda matches. Also, this one is the least error prone and more consistent with other cpu ops.

Quoted 7 lines

Quoted 3 lines

b) the loss of the value read (the fact that the only current user of this API does not need the value returned seems like a very weak argument to define an API).

The other user of cmpxchg_double that I have in my tree also does not have the need. Repeatability is not as simple to implement as with a single word cmpxchg.

Yeah, even in regular cmpxchg, the read value on failure isn't of very high value. The cpu usually has to go retry anyway && likely to have the cacheline already, so it's not gonna cost much.

So, yeah, of the proposed ones, this is my favorite. Peter and Mathieu don't like it. What do others think? Pekka, Eric, Andrew, what do you guys think?

Hello,

Quoted 3 lines · Christoph Lameter

Quoted 1 line · Mathieu Desnoyers

I have to admit that I also hate the current interface for two reasons:

Call me weird but I like this one than others. It sure is ugly but the operation itself isn't a particularly pretty so it kinda matches. Also, this one is the least error prone and more consistent with other cpu ops.

Quoted 4 lines · Tejun Heo

Call me weird but I like this one than others. It sure is ugly but the operation itself isn't a particularly pretty so it kinda matches. Also, this one is the least error prone and more consistent with other cpu ops.

So what are we going to do about this patch? I'd love to merge Christoph's SLUB patches for linux-next now that .38-rc1 is out.

Hello,

Quoted 8 lines · Pekka Enberg

Quoted 4 lines · Tejun Heo

Call me weird but I like this one than others. It sure is ugly but the operation itself isn't a particularly pretty so it kinda matches. Also, this one is the least error prone and more consistent with other cpu ops.

So what are we going to do about this patch? I'd love to merge Christoph's SLUB patches for linux-next now that .38-rc1 is out.

At least you like it, which is good.

I don't think the currently proposed one with two separate parameters is significantly better than other alternatives and vice-versa. They all have slightly different ugliness and error proneness issues.

That said, I still like the double parameter one best, and, unless there are distinctively good reasons to choose another one, I'm gonna commit it to percpu tree in a few days so that merge can proceed. So, if you have something to say, now would be a good time to assert it.

I really object to passing two pointers where one of them has to be a fixed offset to the other. That really doesn't make any sense.

Hello, Peter.

Quoted 2 lines · H. Peter Anvin

I really object to passing two pointers where one of them has to be a fixed offset to the other. That really doesn't make any sense.

Yeah, I hear you, but it really comes down to which ugliness disgusts one the most. That, unfortunately, is inherently very subjective when there's no significantly better choice.

For me, the double parameter thing at least seems to have the advantages of being able to verify the two intended memory locations to be used actually are together and looking ugly reflecting its true nature.

The inherent ugliness stems from the fact that we don't have the built-in data type to properly deal with this. Array of length two might be better fit, but I can see as many downsides with that too.

So, if anyone can give something clearly better for technical reasons, I'll be more than happy to take it, but as it currently stands, it seems we'll have to choose one among uglies and not everyone would be happy about the choice. :-(

Thanks.

Signature · 1 line

tejun

There is a technical argument: any noninline version will have actual code overhead.

Hello, Peter.

Quoted 2 lines · H. Peter Anvin

I really object to passing two pointers where one of them has to be a fixed offset to the other. That really doesn't make any sense.

Yeah, I hear you, but it really comes down to which ugliness disgusts one the most. That, unfortunately, is inherently very subjective when there's no significantly better choice.

For me, the double parameter thing at least seems to have the advantages of being able to verify the two intended memory locations to be used actually are together and looking ugly reflecting its true nature.

The inherent ugliness stems from the fact that we don't have the built-in data type to properly deal with this. Array of length two might be better fit, but I can see as many downsides with that too.

So, if anyone can give something clearly better for technical reasons, I'll be more than happy to take it, but as it currently stands, it seems we'll have to choose one among uglies and not everyone would be happy about the choice. :-(

I'm again probably missing something, but what is "clumsy" about defining a structure like the following to ensure proper alignment of the target pointer (instead of adding a runtime test) ?

struct cmpxchg_double {

#if __BYTE_ORDER == __LITTLE_ENDIAN
        unsigned long low, high;
#else
        unsigned long high, low;

#endif
} __attribute__((packed, aligned(2 * sizeof(unsigned long))));

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

The prototype could then look like:

bool __this_cpu_generic_cmpxchg_double(pcp, oval_low, oval_high, nval_low, nval_high);

With:
struct cmpxchg_double *pcp

I think Christoph's point is that he wants to alias this with a pointer. Well, this can be done cleanly with:

union {
        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

Thoughts ?

Quoted 21 lines · Mathieu Desnoyers

I'm again probably missing something, but what is "clumsy" about defining a structure like the following to ensure proper alignment of the target pointer (instead of adding a runtime test) ?

struct cmpxchg_double {

#if __BYTE_ORDER == __LITTLE_ENDIAN
        unsigned long low, high;
#else
        unsigned long high, low;

#endif
} __attribute__((packed, aligned(2 * sizeof(unsigned long))));

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

The prototype could then look like:

bool __this_cpu_generic_cmpxchg_double(pcp, oval_low, oval_high, nval_low, nval_high);

Quoted 2 lines

With:
struct cmpxchg_double *pcp

That does not conform to the parameter conventions in other this_cpu_ops. The first parameter is a variable because the notion of a pointer is problematic given that percpu operations use a segment prefix to relocate pointers.

You would be implicitly passing a 128 bit argument although the compiler may not need to generate code for that.

Quoted 11 lines

I think Christoph's point is that he wants to alias this with a pointer. Well, this can be done cleanly with:

union {
        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

There is no need for aliases with the existing implementation.

How will the macro check the parameters now?

Quoted 2 lines

Thoughts ?

Could you actually try to write a patch instead running through points that we have discussed earlier?

Quoted 7 lines

Quoted 2 lines

With:
struct cmpxchg_double *pcp

That does not conform to the parameter conventions in other this_cpu_ops. The first parameter is a variable because the notion of a pointer is problematic given that percpu operations use a segment prefix to relocate pointers.

So the first argument could be along the lines of:

struct cmpxchg_double pcp

then.

Quoted 7 lines

Quoted 6 lines

        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

Quoted 3 lines

There is no need for aliases with the existing implementation.

How will the macro check the parameters now?

Well, my last proposal to check __alignof__ within a __builtin_choose_expr check wouldn't need this union actually, which would be much better I think.

Quoted 10 lines · Mathieu Desnoyers

Quoted 7 lines

Quoted 2 lines

With:
struct cmpxchg_double *pcp

That does not conform to the parameter conventions in other this_cpu_ops. The first parameter is a variable because the notion of a pointer is problematic given that percpu operations use a segment prefix to relocate pointers.

So the first argument could be along the lines of:

Quoted 3 lines

struct cmpxchg_double pcp

then.

Ok then you would pass a struct by value? Or use a non-scalar as a variable passed to a this_cpu_op? So far per cpu scalars have been the only variables allowed to be specified in this_cpu operations.

Quoted 10 lines

Quoted 10 lines

Quoted 7 lines

        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

There is no need for aliases with the existing implementation.

Quoted 4 lines

Quoted 1 line

How will the macro check the parameters now?

Well, my last proposal to check __alignof__ within a __builtin_choose_expr check wouldn't need this union actually, which would be much better I think.

The existing implementation has a check for alignment. That is not the problem.

The typechecking would need to be addressed. I.e. if I pass a pointer for old and an ulong for the new value then I'd like to see the compiler complain. Or if the first parameter is a long but the type of the first word is a pointer etc etc.

"The single large 128 bit scalar does not work. Having to define an additional structure it also a bit clumsy. I think its best to get another patchset out that also duplicates the first parameter and makes the percpu variable specs conform to the other this_cpu ops."

I'm again probably missing something, but what is "clumsy" about defining a structure like the following to ensure proper alignment of the target pointer (instead of adding a runtime test) ?

struct cmpxchg_double {

#if __BYTE_ORDER == __LITTLE_ENDIAN
        unsigned long low, high;
#else
        unsigned long high, low;

#endif
} __attribute__((packed, aligned(2 * sizeof(unsigned long))));

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

The prototype could then look like:

bool __this_cpu_generic_cmpxchg_double(pcp, oval_low, oval_high, nval_low, nval_high);

With:
struct cmpxchg_double *pcp

I think Christoph's point is that he wants to alias this with a pointer. Well, this can be done cleanly with:

union {
        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

So by keeping distinct variables for the oval/nal arguments, we let the compiler use registers (instead of the mandatory stack use that would be required if we pass union or structures as oval/nval arguments), but we ensure proper alignment (and drop the unneeded second pointer, as well as the runtime pointer alignment checks) by passing one single pcp pointer of a fixed type with a known alignment.

Hello,

Quoted 14 lines · Mathieu Desnoyers

"The single large 128 bit scalar does not work. Having to define an additional structure it also a bit clumsy. I think its best to get another patchset out that also duplicates the first parameter and makes the percpu variable specs conform to the other this_cpu ops."

I'm again probably missing something, but what is "clumsy" about defining a structure like the following to ensure proper alignment of the target pointer (instead of adding a runtime test) ?

struct cmpxchg_double {

#if __BYTE_ORDER == __LITTLE_ENDIAN
        unsigned long low, high;
#else
        unsigned long high, low;

Quoted 8 lines

#endif
} __attribute__((packed, aligned(2 * sizeof(unsigned long))));

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

Yeah, good point. :-)

Quoted 3 lines

The prototype could then look like:

bool __this_cpu_generic_cmpxchg_double(pcp, oval_low, oval_high,

nval_low, nval_high);

Quoted 5 lines

With:
struct cmpxchg_double *pcp

I think Christoph's point is that he wants to alias this with a

pointer. Well,

Quoted 10 lines

this can be done cleanly with:

union {
        struct cmpxchg_double casdbl;
        struct {
                void *ptr;
                unsigned long cpuid_tid;
        } t;
}

Quoted 7 lines

So by keeping distinct variables for the oval/nal arguments, we let the compiler use registers (instead of the mandatory stack use that would be required if we pass union or structures as oval/nval arguments), but we ensure proper alignment (and drop the unneeded second pointer, as well as the runtime pointer alignment checks) by passing one single pcp pointer of a fixed type with a known alignment.

Actually, we might be able to work around the stack usage regardless of the passed type. We should be able to decompose the structure or ulonglong using macro and then pass the decomposed parameters to a function if necessary.

Hmm, if we do ulonglong, we don't even need a separate interface and can simply use cmpxchg(). If someone can come up with something which uses ulonglong that way, it would be great; then, we wouldn't need to worry about alignment either. Am I missing something?

Thanks.

Signature · 1 line

tejun

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

Quoted 2 lines · H. Peter Anvin

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

But, IIRC, the problem with int128 was with passing it as parameter and return value. We don't have to do that. We'll be just using it as a data storage / container type. Or even that is broken?

Quoted 2 lines

Quoted 2 lines

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

Quoted 3 lines

But, IIRC, the problem with int128 was with passing it as parameter and return value. We don't have to do that. We'll be just using it as a data storage / container type. Or even that is broken?

Well, part of the point was to pass in registers.

No idea on the data storage type.

Quoted 2 lines · H. Peter Anvin

Quoted 2 lines

Quoted 2 lines

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

Quoted 3 lines · Tejun Heo

Quoted 3 lines

But, IIRC, the problem with int128 was with passing it as parameter and return value. We don't have to do that. We'll be just using it as a data storage / container type. Or even that is broken?

Quoted 3 lines · H. Peter Anvin

Well, part of the point was to pass in registers.

No idea on the data storage type.

Ping? The current situation is that we're unable to merge a perfectly good SLUB performance optimization because we can't seem to agree on the this_cpu_cmpxchg_double() API.

Quoted 3 lines

Ping? The current situation is that we're unable to merge a perfectly good SLUB performance optimization because we can't seem to agree on the this_cpu_cmpxchg_double() API.

I thought cl was preparing new version of the patchset. Christoph?

Quoted 5 lines · Tejun Heo

Quoted 3 lines

Ping? The current situation is that we're unable to merge a perfectly good SLUB performance optimization because we can't seem to agree on the this_cpu_cmpxchg_double() API.

I thought cl was preparing new version of the patchset. Christoph?

In order to do that I would have to see a way to improve the patches. None of the suggestions so far seem to be realizable.

Quoted 8 lines · Christoph Lameter

Quoted 5 lines · Tejun Heo

Quoted 3 lines

Ping? The current situation is that we're unable to merge a perfectly good SLUB performance optimization because we can't seem to agree on the this_cpu_cmpxchg_double() API.

I thought cl was preparing new version of the patchset. Christoph?

Quoted 2 lines

In order to do that I would have to see a way to improve the patches. None of the suggestions so far seem to be realizable.

I see. For some reason, I thought you and hpa agreed on something and you were writing up the patches. I'll read the thread again. Let's just pick one. It's not like this is an API which will be used througout the tree.

Quoted 5 lines · Tejun Heo

Quoted 3 lines

Ping? The current situation is that we're unable to merge a perfectly good SLUB performance optimization because we can't seem to agree on the this_cpu_cmpxchg_double() API.

I thought cl was preparing new version of the patchset. Christoph?

In order to do that I would have to see a way to improve the patches. None of the suggestions so far seem to be realizable.

Just respin your patches then.

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

Quoted 1 line · H. Peter Anvin

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

We also would need to be pass the structure by value (well its really a variable but its like passing by value) in order to be similar to the other this_cpu_ops

You'd want either

DEFINE_PERCPU(struct mycustomdoublestruct, percpu_dd)

this_cpu_cmpxchg_double(percpu_dd, oldword1, oldword2, newword1, newword2)

with the problem of type checking

or

this_cpu_cmpxchg_double(percpu_dd, old_dd, new_dd)

with the problem of 128 bit constants/structs passed by value.

Quoted 3 lines

this_cpu_cmpxchg_double(percpu_dd, oldword1, oldword2, newword1, newword2)

with the problem of type checking

What is the problem with type checking here ?

We could use a gcc builtin like the following to check if the alignment of "percpu_dd" meets the double-cas requirements:

#define testmacro(a, b) \
        __builtin_choose_expr(__alignof__(a) >= 2 * sizeof(unsigned long), \
                              ((a).low) = (b), \    /* success */
                              ((a).low) = (void) 0) /* compile-error */

Quoted 2 lines

or

Quoted 3 lines

this_cpu_cmpxchg_double(percpu_dd, old_dd, new_dd)

with the problem of 128 bit constants/structs passed by value.

Yeah, I guess trying to deal with 128-bit value might be a bit tricky. But having something sane and with compile-time-checked alignment for the percpu_dd type is not to be looked over.

Quoted 5 lines · Mathieu Desnoyers

Quoted 3 lines

this_cpu_cmpxchg_double(percpu_dd, oldword1, oldword2, newword1, newword2)

with the problem of type checking

What is the problem with type checking here ?

You need to know the fields in the struct to do the type checking with each of the other parameters.

Quoted 13 lines

We could use a gcc builtin like the following to check if the alignment of "percpu_dd" meets the double-cas requirements:

#define testmacro(a, b) \
        __builtin_choose_expr(__alignof__(a) >= 2 * sizeof(unsigned long), \
                              ((a).low) = (b), \    /* success */
                              ((a).low) = (void) 0) /* compile-error */

Quoted 4 lines

or

this_cpu_cmpxchg_double(percpu_dd, old_dd, new_dd)

Quoted 5 lines

Quoted 1 line

with the problem of 128 bit constants/structs passed by value.

Yeah, I guess trying to deal with 128-bit value might be a bit tricky. But having something sane and with compile-time-checked alignment for the percpu_dd type is not to be looked over.

The existing implementation could be equipped to do a compile time check for the proper alignment if the pointer passed is constant.

"if the pointer passed is constant" -> if you use the actual type of percpu_dd to check the alignment, then you can do an alignment check at compile-time even for a non-const parameter. The requirement imposed on typing will take care to make sure that even a non-const pointer will have the proper alignment.

Quoted 3 lines · * Christoph Lameter ([email protected]) wrote:

Quoted 1 line · H. Peter Anvin

We could do cmpxchg with a structure... the problem with a lon int

type is that Cristoph ran into bugs with __int128 on 64 bits.

Quoted 2 lines

We also would need to be pass the structure by value (well its really

a

Quoted 8 lines

variable but its like passing by value) in order to be similar to the other this_cpu_ops

You'd want either

DEFINE_PERCPU(struct mycustomdoublestruct, percpu_dd)

this_cpu_cmpxchg_double(percpu_dd, oldword1, oldword2, newword1,

newword2)

Quoted 2 lines

with the problem of type checking

What is the problem with type checking here ?

We could use a gcc builtin like the following to check if the alignment of
"percpu_dd" meets the double-cas requirements:

#define testmacro(a, b) \

  __builtin_choose_expr(__alignof__(a) >= 2 * sizeof(unsigned long), \
                             ((a).low) = (b), \    /* success */
                             ((a).low) = (void) 0) /* compile-error */

Quoted 5 lines

or

this_cpu_cmpxchg_double(percpu_dd, old_dd, new_dd)

with the problem of 128 bit constants/structs passed by value.

Yeah, I guess trying to deal with 128-bit value might be a bit tricky. But
having something sane and with compile-time-checked alignment for the percpu_dd
type is not to be looked over.

Thanks,

Mathieu

Signature · 4 lines

Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

What is the problem with passing 128-bit structures by value? I'm quite sure *that* is okay.

Quoted 5 lines

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

Yeah, good point. :-)

* Tejun Heo ([email protected]) wrote:
[...]

Quoted 7 lines

Quoted 5 lines

(note: packed here along with "aligned" does _not_ generate ugly bytewise read/write memory ops like "packed" alone. The use of "packed" is to let the compiler down-align the structure to the value requested, instead of uselessly aligning it on 32-byte if it chooses to.)

Yeah, good point. :-)

For the records, I just noticed that "packed, aligned(8)" can generate unaligned
accesses on sparc64 by removing the padding between a "int" and a following
pointer. So we should not use it.

Thanks,

Mathieu

Signature · 4 lines

Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com