[compiler-rt] [scudo] Fix release to OS logic in secondary cache. (PR #103303)

[Joshua Baehring via llvm-commits]

JoshuaMBa wrote:

> > > > > This is a fix, please mention that in the title and the description. In addition, I think this can be covered by the test, right? If so, please add a test case.
> > > > 
> > > > 
> > > > The main issue with testing this is that the interval at which the issue occurs is somewhat arbitrary. I can try different cache sizes and interval values to determine a point where the error occurs, but I don't think that's a robust way of testing this case. I will try coming up with a better way of testing this outside of the current `CacheOrder` test.
> > > 
> > > 
> > > IIRC, we want to ensure the eviction order is still LRU. If we do a page releasing before the first eviction, we will see the wrong order happens, right?
> > 
> > 
> > Not necessarily. The case I showed was when all cached entries are old and ready to be released. That is when we see a complete inversion of the cache order. If the interval is not large enough, then it's possible that the entries are released in the proper order.
> > For example, if the committed entries are (1 2 3 4 5), from most recent to least recent and only 5 is old enough to be released on a store() call, then the committed list becomes (1 2 3 4) and the decommitted list becomes (5). On another store() call, if only 4 is old enough to be released then the committed list becomes (1 2 3) and the decommitted list becomes (4 5). In this case, the LRU ordering is actually being preserved.
> > However, I have now uploaded a test case that solves this issue by just first disabling releases during the initial store() calls and then reenabling the releases.
> 
> So what I'm saying is doing an explicit release to dump all commited entries to decommited list

Yes that will work as long as we disable the release interval.

https://github.com/llvm/llvm-project/pull/103303