Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

A tale of an impossible bug: big.LITTLE and caching

mono-project.com

Post reply on HN
Print view View original

101–110 of 116 posts

Re: A tale of an impossible bug: big.LITTLE and caching

#101

Post by oshepherd »

caf wrote: post #89
rodrigokumpera wrote: post #87

Earlier quoted context omitted.

Multi-threaded safety is simply due to JIT controlling the visibility of the newly compiled code. First flush, then make it visible for execution, can't go wrong with that and scheduling won't matter. Things get a lot more complicated when it comes to code patching, but the principle is similar.

I don't think that helps - the point is that the flush might not be effective if the flushing thread gets scheduled away from the core which has the stale I$ before it manages to fully issue the flush. Or is the flush guaranteed to flush all cores caches? That would be a fairly unusual design.

IC IVAU instructions are broadcast to all cores in the same 'inner shareable domain' (all cores running the same OS instance are in the same inner shareable domain)

Re: A tale of an impossible bug: big.LITTLE and caching

#102

Post by oshepherd »

sangnoir wrote: post #98
gcp wrote: post #92

Earlier quoted context omitted.

Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. Going to 128-byte cache lines was fine, but they should have made the lower-power part of the chip match (which in this case they likely couldn't because they licensed that design), or they should have made sure 128-byte lines were reported in all circumstances (which requires a hack similar to the workaround done in the kernel be…

> Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. This is similar to blaming OS/BIOS manufaturers for Y2K breaking "correctly written code" at the turn of the millenium.The code was incorrect in this instance because it assumed the cache size would be the same for all cores, Samsung simply manufactured a SOC that breaks that faulty assumption.

The architecture states that CTR_EL0 reports the _minimum_ ICache line size across all cores.

In this system, that would be 64 bytes.

Re: A tale of an impossible bug: big.LITTLE and caching

#103

Post by gcp »

cesarb wrote: post #99
gcp wrote: post #86

Earlier quoted context omitted.

ARM's own designs (A53, A57, A72, A73) all have 64-byte cache line sizes and avoid the problem entirely. The one at fault appears to be Samsung, who designed M1 Mongoose with 128 byte lines and packed it together with A53 cores in their SoC.

Perhaps a better (and simpler) workaround, then, could be to clamp the reported cache line size to 64 bytes. So even if the Samsung core reports 128-byte cache lines, the code would simply invalidate each line twice, and if it is migrated in the middle of the invalidation loop, it would still work correctly.

This is exactly the fix used in the kernel, yes. The trickiness and complexity comes from the cache line size probe having to be intercepted.

Re: A tale of an impossible bug: big.LITTLE and caching

#104

Post by gcp »

sangnoir wrote: post #98
gcp wrote: post #92

Earlier quoted context omitted.

Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. Going to 128-byte cache lines was fine, but they should have made the lower-power part of the chip match (which in this case they likely couldn't because they licensed that design), or they should have made sure 128-byte lines were reported in all circumstances (which requires a hack similar to the workaround done in the kernel be…

> Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. This is similar to blaming OS/BIOS manufaturers for Y2K breaking "correctly written code" at the turn of the millenium.The code was incorrect in this instance because it assumed the cache size would be the same for all cores, Samsung simply manufactured a SOC that breaks that faulty assumption.

The assumption isn't faulty, it's guaranteed to be right. And it has to be, or there's no way to write functional code for ARM.

Re: A tale of an impossible bug: big.LITTLE and caching

#105

Post by gcp »

pawadu wrote: post #100
gcp wrote: post #92

Earlier quoted context omitted.

Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. Going to 128-byte cache lines was fine, but they should have made the lower-power part of the chip match (which in this case they likely couldn't because they licensed that design), or they should have made sure 128-byte lines were reported in all circumstances (which requires a hack similar to the workaround done in the kernel be…

> Samsung's design breaks correctly written user-land code. Are you sure? From the article, it sounds to me that the real cause of the error is an incorrect assumption made by the GCC people.

Yes, I am sure.

If we forget about the ARM spec explicitly enforcing this anyway, here's some food for thought: how exactly do you suppose the GCC people could write correct code, if they aren't allowed to make that assumption?

Re: A tale of an impossible bug: big.LITTLE and caching

#106

Post by pawadu »

oshepherd wrote: post #102
sangnoir wrote: post #98

Earlier quoted context omitted.

> Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault. This is similar to blaming OS/BIOS manufaturers for Y2K breaking "correctly written code" at the turn of the millenium.The code was incorrect in this instance because it assumed the cache size would be the same for all cores, Samsung simply manufactured a SOC that breaks that faulty assumption.

The architecture states that CTR_EL0 reports the _minimum_ ICache line size across all cores. In this system, that would be 64 bytes.

Can you point me to the manual that does say so? This is all I could find on the subject

> [3:0] IminLine Log 2 of the number of words in the smallest cache line of all the Instruction Caches that the processor controls.

> This values is:

> 0x4

> Smallest Instruction Cache line size is 16 words.

Re: A tale of an impossible bug: big.LITTLE and caching

#107

Post by pawadu »

gcp wrote: post #105
pawadu wrote: post #100

Earlier quoted context omitted.

> Samsung's design breaks correctly written user-land code. Are you sure? From the article, it sounds to me that the real cause of the error is an incorrect assumption made by the GCC people.

Yes, I am sure. If we forget about the ARM spec explicitly enforcing this anyway, here's some food for thought: how exactly do you suppose the GCC people could write correct code, if they aren't allowed to make that assumption?

Okay, I did not know that. But could you point me to the part of the spec that says this? I wasn't able to find this in their public specs.

Re: A tale of an impossible bug: big.LITTLE and caching

#108

Post by gcp »

pawadu wrote: post #107
gcp wrote: post #105

Earlier quoted context omitted.

Yes, I am sure. If we forget about the ARM spec explicitly enforcing this anyway, here's some food for thought: how exactly do you suppose the GCC people could write correct code, if they aren't allowed to make that assumption?

Okay, I did not know that. But could you point me to the part of the spec that says this? I wasn't able to find this in their public specs.

ARM ARM, section B2.2.6

The CTR holds minimum line length values for: - the instruction caches

...this value is the most efficient address stride to use to apply to a sequence of address-based maintenance operations to a range of addresses...

The documentation for the CTR_EL0 reg talks about "caches under this processors control" which you could argue don't include other cores, but if you allow migration between cores, then line size changes underneath running software break the above "most efficient address stride" assumption. So you can't do that.

It boils down to this: If you can't assume that cache line size doesn't change underneath you, then you can't invalidate line by line at all, and would have to go word by word. That's terrible for performance (and a huge waste), which is why the spec says to use the above value for those operations.

Re: A tale of an impossible bug: big.LITTLE and caching

#109

Post by pawadu »

gcp wrote: post #108
pawadu wrote: post #107

Earlier quoted context omitted.

Okay, I did not know that. But could you point me to the part of the spec that says this? I wasn't able to find this in their public specs.

ARM ARM, section B2.2.6 The CTR holds minimum line length values for: - the instruction caches ...this value is the most efficient address stride to use to apply to a sequence of address-based maintenance operations to a range of addresses... The documentation for the CTR_EL0 reg talks about "caches under this processors control" which you could argue don't include other cores, but if you allow migration between core…

I appreciate you digging up the relevant text from the manual, but I don't think you should accuse Samsung of wrongdoing based on such far fetched assumptions.

That document does not explicitly forbids this. In addition, take a look at their sample code which indeed reads some cache configuration registers during each call. The same code is found verbatim in the linux kernel, if you now don't trust the ARM engineers :)

Post reply on HN