new cache component
Frank Ch. Eigler
fche@redhat.com
Fri Jun 15 08:46:00 GMT 2001
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
More information about the Sid
mailing list