Date: Thu, 28 Feb 2002 16:48:50 -0500 (EST) From: Jeff Roberson <jroberson@chesapeake.net> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: Julian Elischer <julian@elischer.org>, <arch@FreeBSD.ORG> Subject: Re: Slab allocator Message-ID: <20020228161626.N529-100000@mail.chesapeake.net> In-Reply-To: <200202282114.g1SLEQW38736@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> > Could you explain the change made (crinit() and crzero() calls) to > kern/init_main.c ? Sure. I picked a few things to add initializers for to give examples of how the object caching coud be used. To do this I had to figure out what state the objects were in when they were freed, and how they were initialized. So any freed state that matches initial state can be cached. When looking at creds I noticed that everyone calls crdup except for init_main. So the extra zeroing was unneccisary. This is what crzero does. crinit sets up the zone so that cred may be allocated. I'd rather do this than "if (cred_zone == NULL) create" all the time. The initial zeroing may be ok since it's going to be brought into cache anyway, so maybe it isn't worth it. > > Does uma_zalloc() automatically zero the returned memory? You replace > MALLOC(..M_ZERO) in two places, example: > > - MALLOC(fp, struct file *, sizeof(struct file), M_FILE, M_WAITOK | M_ZERO > ); > + fp = uma_zalloc(file_zone, M_WAITOK); I don't automatically zero memory on return. I have a common initializer that can be used for memory when it is first brought into the allocator. This was the behavior of vm_zone. I could provide a common zero constructor that zeros memory if that is desired. And again, as with the cred zone, the memory was zeroed and subsequently filled in with real values. Files and creds turned out to be less of a win with the object initializers than I was expecting. One of these originally had an embeded mutex but was later switched to a pool mutex. So the savings are pretty minimal. On some objects where there was a lot of dependable state I actually got noticable performance gains. > > I recommend KASSERTing bucket_ub_ptr is in-bounds in vm/uma_core.c > line 1403 (i.e. us_freecount and ub_ptr are coherent). Agreed. In my current working tree I have a few more bucket assertions. > > I recommend KASSERTing that the uz_free_slab list is non-empty > on line 1381. This is probably a good thing as well. > > On line 82 of vm/uma_int.h you describe embedding a slab header > in a Page. I could not find where in the code this embedding > takes place (I looked for 'slab header' in the code but could > not find anything related to the comment on line 82 of vm/uma_int.h If you look at the zone_ctor there is a rather convoluted process if the OFFPAGE flag is not set. This calculates the offset into the slab for the slab header. This is stored in the uz_pgoff field in the zone, and added to the page address of freed memory. If OFFPAGE is set, or if malloc is used, you always have to do a hash lookup to find the slab structure. If you look in slab_zalloc it either allocates a slab structure from the slabzone or it just points it into the recently allocated memory. > > In anycase, if you are embeddeding the slab header I recommend > adding a magic number of the base of the slab header structure > described in the comment and KASSERT()ing it as a sanity check. I originally had one.. I don't remember why I took it out. > > -- -- > documentation > -- -- > > uma_timeout line 185 uma_core.c. Could you describe what a > 'working set calc.' is? What is a working set? It is described in zone_workingset. Basically this tells us how many free items to keep in the zone if the vm asks us to return memory. The reason is so that you don't free all of your memory and then have an immediate need to reallocate. I will make things a little clearer. > > In general, the comments at the head of the routines do not include > context. For example, take hash_sfind(). 'Find a slab within a zone > that has a matching data field'. What I would like to see is a comment > like this: 'Find a slab within a zone that has a matching data field. > This function is mainly used by the deallocator to locate the slab > which owns an item data pointer.' > > Other examples. Line 266, the comment for hash_expand(), says > what it does 'Expands the hash table for OFFPAGE zones', but does > not say WHY it does it or HOW it does it. To reduce collisions. ;-) > > In anycase, this is general for all the routines. It would be > nice if you did a once-over and clarified the context of the comment > where it seems appropriate. > > -Matt > Matthew Dillon > <dillon@backplane.com> > I definately see where you are going. My comments lack a bit of the big picture information. I will grab some information that I have included in this email, and review my documentation in general. I think I geared it more towards people who were already familiar with the bonwick papers. Thanks for the input! Jeff To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020228161626.N529-100000>