Subject: Re: amap_extend does unnecessary malloc/memcpy.
To: None <sommerfeld@orchard.arlington.ma.us>
From: enami tsugutomo <enami@sm.sony.co.jp>
List: tech-kern
Date: 11/20/2001 14:41:44
Bill Sommerfeld <sommerfeld@orchard.arlington.ma.us> writes:
> > + if (size <= MAXALLOCSAVE)
> > + slotalloc = (1 << BUCKETINDX(size)) / sizeof(int);
> > + else
> > + slotalloc = roundup(size, PAGE_SIZE) / sizeof(int);
>
> Rather than spreading implementation details about our kernel malloc
> through the unrelated code elsewhere in the kernel, how about defining
> a malloc_roundup() function or MALLOC_ROUNDUP() macro in malloc.h?
Cool. I prefer this idea. The function may be useful in some other
place.
Here is a revised version.
- We can use malloc_roundup() on initial allocation.
- Since we don't zero clear am_bckptr on initial allocation, clearing
rest of bckptr when extending amap shouldn't be necessary.
enami.
Index: uvm_amap.c
===================================================================
RCS file: /cvsroot/syssrc/sys/uvm/uvm_amap.c,v
retrieving revision 1.37
diff -u -r1.37 uvm_amap.c
--- uvm_amap.c 2001/11/10 07:36:59 1.37
+++ uvm_amap.c 2001/11/20 05:31:45
@@ -167,6 +167,22 @@
}
/*
+ * malloc_roundup: roundup size to the real allocation size.
+ * XXX this will goto malloc.[ch].
+ */
+size_t malloc_roundup __P((size_t));
+size_t
+malloc_roundup(size)
+ size_t size;
+{
+
+ if (size > MAXALLOCSAVE)
+ return (roundup(size, PAGE_SIZE));
+ else
+ return (1 << BUCKETINDX(size));
+}
+
+/*
* amap_alloc1: internal function that allocates an amap, but does not
* init the overlay.
*
@@ -177,12 +193,14 @@
int slots, padslots, waitf;
{
struct vm_amap *amap;
- int totalslots = slots + padslots;
+ int totalslots;
amap = pool_get(&uvm_amap_pool, (waitf == M_WAITOK) ? PR_WAITOK : 0);
if (amap == NULL)
return(NULL);
+ totalslots = malloc_roundup((slots + padslots) * sizeof(int)) /
+ sizeof(int);
simple_lock_init(&amap->am_l);
amap->am_ref = 1;
amap->am_flags = 0;
@@ -241,7 +259,7 @@
amap = amap_alloc1(slots, padslots, waitf);
if (amap)
memset(amap->am_anon, 0,
- (slots + padslots) * sizeof(struct vm_anon *));
+ amap->am_maxslot * sizeof(struct vm_anon *));
UVMHIST_LOG(maphist,"<- done, amap = 0x%x, sz=%d", amap, sz, 0, 0);
return(amap);
@@ -291,13 +309,12 @@
{
struct vm_amap *amap = entry->aref.ar_amap;
int slotoff = entry->aref.ar_pageoff;
- int slotmapped, slotadd, slotneed;
+ int slotmapped, slotadd, slotneed, slotadded, slotalloc;
#ifdef UVM_AMAP_PPREF
int *newppref, *oldppref;
#endif
int *newsl, *newbck, *oldsl, *oldbck;
struct vm_anon **newover, **oldover;
- int slotadded;
UVMHIST_FUNC("amap_extend"); UVMHIST_CALLED(maphist);
UVMHIST_LOG(maphist, " (entry=0x%x, addsize=0x%x)", entry,addsize,0,0);
@@ -366,10 +383,12 @@
*/
amap_unlock(amap); /* unlock in case we sleep in malloc */
+ slotalloc = malloc_roundup(slotneed * sizeof(int)) / sizeof(int);
#ifdef UVM_AMAP_PPREF
newppref = NULL;
if (amap->am_ppref && amap->am_ppref != PPREF_NONE) {
- newppref = malloc(slotneed * sizeof(int), M_UVMAMAP, M_NOWAIT);
+ newppref = malloc(slotalloc * sizeof(int), M_UVMAMAP,
+ M_NOWAIT);
if (newppref == NULL) {
/* give up if malloc fails */
free(amap->am_ppref, M_UVMAMAP);
@@ -377,9 +396,9 @@
}
}
#endif
- newsl = malloc(slotneed * sizeof(int), M_UVMAMAP, M_WAITOK);
- newbck = malloc(slotneed * sizeof(int), M_UVMAMAP, M_WAITOK);
- newover = malloc(slotneed * sizeof(struct vm_anon *),
+ newsl = malloc(slotalloc * sizeof(int), M_UVMAMAP, M_WAITOK);
+ newbck = malloc(slotalloc * sizeof(int), M_UVMAMAP, M_WAITOK);
+ newover = malloc(slotalloc * sizeof(struct vm_anon *),
M_UVMAMAP, M_WAITOK);
amap_lock(amap); /* re-lock! */
KASSERT(amap->am_maxslot < slotneed);
@@ -388,7 +407,7 @@
* now copy everything over to new malloc'd areas...
*/
- slotadded = slotneed - amap->am_nslot;
+ slotadded = slotalloc - amap->am_nslot;
/* do am_slots */
oldsl = amap->am_slots;
@@ -404,7 +423,6 @@
/* do am_bckptr */
oldbck = amap->am_bckptr;
memcpy(newbck, oldbck, sizeof(int) * amap->am_nslot);
- memset(newbck + amap->am_nslot, 0, sizeof(int) * slotadded); /* XXX: needed? */
amap->am_bckptr = newbck;
#ifdef UVM_AMAP_PPREF
@@ -417,13 +435,14 @@
if ((slotoff + slotmapped) < amap->am_nslot)
amap_pp_adjref(amap, slotoff + slotmapped,
(amap->am_nslot - (slotoff + slotmapped)), 1);
- pp_setreflen(newppref, amap->am_nslot, 1, slotadded);
+ pp_setreflen(newppref, amap->am_nslot, 1,
+ slotneed - amap->am_nslot);
}
#endif
/* update master values */
amap->am_nslot = slotneed;
- amap->am_maxslot = slotneed;
+ amap->am_maxslot = slotalloc;
amap_unlock(amap);
free(oldsl, M_UVMAMAP);
@@ -683,6 +702,8 @@
amap->am_slots[amap->am_nused] = lcv;
amap->am_nused++;
}
+ memset(&amap->am_anon[lcv], 0,
+ (amap->am_maxslot - lcv) * sizeof(struct vm_anon *));
/*
* drop our reference to the old amap (srcamap) and unlock.