Re: new cache component

[fche at redhat dot com (Frank Ch. Eigler)]

bje wrote:

: I have just committed a new memory cache component.  [...]

Good stuff.  I like the cache_line/cache_set abstractions.  A few nits
with respect to the code:

- The nomenclature of cache component type names is perhaps
  too complicated.  Some of the parameters specified in the
  type name could (should?) be treated as attributes instead.
  (However, then one would have to address the issue of their
  run-time changeability.)  In any case, the type name parsing
  routine in CacheCreate() could benefit from sidutil::tokenize(),
  or even better, from using a fixed list of type-names and 
  parameters, like the flash memory components do.

- In a couple of places, sidutil::parse_attribute() is called
  but its return value is not checked.  Naughty!

- Your use of various static objects is fine w.r.t. C++ standards, 
  but is possibly fragile on platforms with crappy C++ shared library
  implementations.  You should check the code on non-Linux hosts; may
  need to go for heap allocation instead. :-(

- Consider adding TODOs for future support for bus snooping, leading
  to cache coherency protocols

- Consider adding per-cache-line statistics (to detect if accesses
  are not distributed nicely amongst the lines).


- FChE