Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 19 Apr 2003 12:06:36 -0700
From:      Terry Lambert <tlambert2@mindspring.com>
To:        Don Lewis <truckman@FreeBSD.org>
Cc:        hackers@FreeBSD.org
Subject:   Re: Repeated similar panics on -STABLE
Message-ID:  <3EA19E3C.661C9A28@mindspring.com>
References:  <200304190859.h3J8xNXB016406@gw.catspoiler.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Don Lewis wrote:
> Ok, here's a Reader's Digest version of the code in question:
> 
>         if (kbp->kb_next == NULL) {
>                 kbp->kb_last = NULL;
>                 if (size > MAXALLOCSAVE)
>                         allocsize = roundup(size, PAGE_SIZE);
>                 else
>                         allocsize = 1 << indx;
>                 npg = btoc(allocsize);
>                 va = (caddr_t) kmem_malloc(kmem_map, (vm_size_t)ctob(npg), flags
> );
>                 /*
>                  * Just in case we blocked while allocating memory,
>                  * and someone else also allocated memory for this
>                  * bucket, don't assume the list is still empty.
>                  */
>                 savedlist = kbp->kb_next;
>                 kbp->kb_next = cp = va + (npg * PAGE_SIZE) - allocsize;
>                 for (;;) {
>                         freep = (struct freelist *)cp;
>                         if (cp <= va)
>                                 break;
>                         cp -= allocsize;
>                         freep->next = cp;
>                 }
>                 freep->next = savedlist;

Take an interrupt somewhere around here, and have the available
entries removed from the freelist by an interrupt level driver.

Or take a page fault, and have the same thing happen with
page-related metadata coming from the freelist in question.

>                 if (kbp->kb_last == NULL)
>                         kbp->kb_last = (caddr_t)freep;
>         }
>         va = kbp->kb_next;
>         kbp->kb_next = ((struct freelist *)va)->next;

[ ... ]

> This code bothers me, though, because if allocsize ever happened to not
> evenly divide (npg * PAGE_SIZE), the loop termination condition would be
> wrong and the end of the previous page (or more) would end up on the
> free list.  If va were small enough and allocsize were big enough, it
> would even be possible to wrap around.

That actually can't happen.  Basically, the allocators "waste"
the ends of pages.  See the:

                        if (cp <= va)
                                break;
                        cp -= allocsize;

?  The "<= saves you.

-- Terry



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3EA19E3C.661C9A28>