On Thu, 24 Apr 2014 00:38:01 -0700 Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > On Thu, Apr 24, 2014 at 12:24 AM, NeilBrown <neilb@xxxxxxx> wrote: > > 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. > > No, they don't. Let me throw together a proper cheat sheet. Thanks. > > > You've chosen '8' for the 'level' number. > > Hmm, ok. I use -12 in the mdadm bits, I neglected to go back and fix > up the kernel. > > > 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. > > Doesn't kref == 0 imply object destroyed? It's a count of pending > metadata events. kref means that in a kobject. Elsewhere it means whatever you want. mpb_read_endio would kref_put(&conf->ref, release) where release would get the conf and wake_up(&conf->eventq); It probably isn't a big win.. NeilBrown
Attachment:
signature.asc
Description: PGP signature
