Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 Feb 2002 13:14:26 -0800 (PST)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        Jeff Roberson <jroberson@chesapeake.net>
Cc:        Julian Elischer <julian@elischer.org>, <arch@FreeBSD.ORG>
Subject:   Re: Slab allocator
Message-ID:  <200202282114.g1SLEQW38736@apollo.backplane.com>
References:   <20020228141318.J31751-100000@mail.chesapeake.net>

next in thread | previous in thread | raw e-mail | index | archive | help

:Ok, both of you have requested more comments.  I have commented what I
:felt were the trickier situations.  Could you give me some examples of
:what seems to be lacking documentation?  I'm perfectly willing to add it,
:but since I wrote the code most things seem perfectly intuitive to me. :-)
:
:Thanks,
:Jeff

    Could you explain the change made (crinit() and crzero() calls) to
    kern/init_main.c ?

    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 recommend KASSERTing bucket_ub_ptr is in-bounds in vm/uma_core.c
    line 1403 (i.e. us_freecount and ub_ptr are coherent).

    I recommend KASSERTing that the uz_free_slab list is non-empty
    on line 1381.

    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

    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.

    --								--
				documentation
    --								--

    uma_timeout line 185 uma_core.c.  Could you describe what a
    'working set calc.' is?  What is a working set?

    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.

    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>

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?200202282114.g1SLEQW38736>