Simon Wilkinson wrote (Thu Jul 02 2009 12:22:08 GMT+0200 (CEST))
On 2 Jul 2009, at 07:30, Felix Frank wrote:
The problem with that being that it does not really simplify anything.
What's worse, the fetchOps/storeOps would need being determined before
calling afs_CacheFetchProc/CacheStoreProc. As will be seen in the
upcoming patches, that would not be desirable, I think.
I have a few points here,
*) The cache manager already has a cache type indirection mechanism -
afs_cacheOps. Things which vary only by cache type should use this
indirection layer, rather than adding an additional one.
*) Having both a cache type layer, and a protocol/cache type layer seems
very untidy.
*) When you have an indirection layer, you should use it. Having the
ability to have cache or protocol specific initialisation functions, and
then still doing
if (cacheDiskType == AFS_FCACHE_TYPE_UFS ) {
thing
} else {
other thing
}
is just untidy programming.
I can see this happening somehow, and it might even end up being quite
readable. Still, I fear call stacks would compare like this:
Now:
CacheXProc()
=> appropriate rx_Write/_Read
With current proposal:
CachXProc()
=> fetchOp/storeOp (from cache-specific ops structure)
=> appropriate rx calls
With clean seperation:
CacheXProc()
=> fetchOp/storeOp (from generic ops structure)
=> rx-wrapper (from struct afs_cacheOps)
=> appropriate rx calls
That should be manageable, but adds stack overhead.
Cheers
- Felix
_______________________________________________
OpenAFS-devel mailing list
[email protected]
https://lists.openafs.org/mailman/listinfo/openafs-devel
