On Wed, 23 Apr 2014 23:18:49 -0700 Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > Initial md / block boilerplate for the Intel (R) Smart Response > Technology compatibility driver. Supports reading the packed metadata > and parsing it into a cache lookup tree. > > Cc: Dave Jiang <dave.jiang@xxxxxxxxx> > Cc: Artur Paszkiewicz <artur.paszkiewicz@xxxxxxxxx> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> It would really help in reviewing this to have a glossary. There are frames and segments and sectors and pages. I hope sectors are 512 bytes and pages are PAGE_SIZE, which may or may not be 4096. And there are 16 sectors per frame, so I guess space is allocated in the cache in 8K aligned frames ?? There are 64 segments per page so if pages did happen to be 4096 bytes, that makes 64 bytes per segment. What are they? There is a list somewhere of 32byte frame descriptors which is read into a single vmalloced region (why? you keep page pointers, so why not read it into separate pages?) How is this organised? I might be able to work that out from the code, but I'd rather not. Please don't make me guess, I'm not good at it. I guess it didn't help that diff out the header after the code. I got bored before I got there and didn't read all to words, so maybe some answers are in there. They don't really stand out though. You've chosen '8' for the 'level' number. As this is an array which doesn't have redundancy, I'd rather a number <= 0. I think there are places where I assume >=1 has redundancy and understands spares etc. Should conf->count be a kref??? Just a thought, not a requirement. NeilBrown
Attachment:
signature.asc
Description: PGP signature
