Browse Source

Various fixes and debugging help: particularly don't use random().

Rusty Russell 17 years ago
parent
commit
aec5e6e9b1
1 changed files with 44 additions and 24 deletions
  1. 44 24
      ccan/alloc/alloc.c

+ 44 - 24
ccan/alloc/alloc.c

@@ -478,6 +478,7 @@ static unsigned long new_uniform_page(void *pool, unsigned long poolsize,
 	unsigned long page, metalen;
 	unsigned long page, metalen;
 	uint8_t *metadata;
 	uint8_t *metadata;
 
 
+	/* FIXME: Walk metadata looking for an existing uniform page. */
 	page = alloc_get_pages(pool, poolsize, 1, 1);
 	page = alloc_get_pages(pool, poolsize, 1, 1);
 	if (page == 0)
 	if (page == 0)
 		return 0;
 		return 0;
@@ -514,6 +515,7 @@ static unsigned long alloc_sub_page(void *pool, unsigned long poolsize,
 
 
 	usize = suitable_for_uc(size, align);
 	usize = suitable_for_uc(size, align);
 	if (usize) {
 	if (usize) {
+		static int random_entry;
 		/* Look for a uniform page. */
 		/* Look for a uniform page. */
 		for (i = 0; i < UNIFORM_CACHE_NUM; i++) {
 		for (i = 0; i < UNIFORM_CACHE_NUM; i++) {
 			if (uc->size[i] == usize) {
 			if (uc->size[i] == usize) {
@@ -528,7 +530,7 @@ static unsigned long alloc_sub_page(void *pool, unsigned long poolsize,
 		}
 		}
 
 
 		/* OK, try a new uniform page.  Use random discard for now. */
 		/* OK, try a new uniform page.  Use random discard for now. */
-		i = random() % UNIFORM_CACHE_NUM;
+		i = (++random_entry % UNIFORM_CACHE_NUM);
 		maybe_transform_uniform_page(pool, uc->page[i]);
 		maybe_transform_uniform_page(pool, uc->page[i]);
 
 
 		uc->page[i] = new_uniform_page(pool, poolsize, usize);
 		uc->page[i] = new_uniform_page(pool, poolsize, usize);
@@ -591,7 +593,7 @@ static bool uniform_page_is_empty(uint8_t *meta)
 	metalen = uniform_metalen(decode_usize(meta));
 	metalen = uniform_metalen(decode_usize(meta));
 
 
 	/* Skip the header (first two bytes of metadata). */
 	/* Skip the header (first two bytes of metadata). */
-	for (i = 2; i < metalen + 2; i++) {
+	for (i = 2; i < metalen; i++) {
 		BUILD_ASSERT(FREE == 0);
 		BUILD_ASSERT(FREE == 0);
 		if (meta[i])
 		if (meta[i])
 			return false;
 			return false;
@@ -753,7 +755,7 @@ static void bitmap_free(void *pool, unsigned long pagenum, unsigned long off,
 	off++;
 	off++;
 
 
 	set_bit_pair(metadata, off++, FREE);
 	set_bit_pair(metadata, off++, FREE);
-	while (off < SUBPAGE_METAOFF / BITMAP_GRANULARITY
+	while (off <= SUBPAGE_METAOFF / BITMAP_GRANULARITY
 	       && get_bit_pair(metadata, off) == TAKEN)
 	       && get_bit_pair(metadata, off) == TAKEN)
 		set_bit_pair(metadata, off++, FREE);
 		set_bit_pair(metadata, off++, FREE);
 }
 }
@@ -886,6 +888,12 @@ static bool is_metadata_page(void *pool, unsigned long poolsize,
 	return false;
 	return false;
 }
 }
 
 
+/* Useful for gdb breakpoints. */
+static bool check_fail(void)
+{
+	return false;
+}
+
 static bool check_bitmap_metadata(void *pool, unsigned long *mhoff)
 static bool check_bitmap_metadata(void *pool, unsigned long *mhoff)
 {
 {
 	enum alloc_state last_state = FREE;
 	enum alloc_state last_state = FREE;
@@ -898,10 +906,10 @@ static bool check_bitmap_metadata(void *pool, unsigned long *mhoff)
 		state = get_bit_pair((uint8_t *)pool + *mhoff, i+1);
 		state = get_bit_pair((uint8_t *)pool + *mhoff, i+1);
 		switch (state) {
 		switch (state) {
 		case SPECIAL:
 		case SPECIAL:
-			return false;
+			return check_fail();
 		case TAKEN:
 		case TAKEN:
 			if (last_state == FREE)
 			if (last_state == FREE)
-				return false;
+				return check_fail();
 			break;
 			break;
 		default:
 		default:
 			break;
 			break;
@@ -911,6 +919,17 @@ static bool check_bitmap_metadata(void *pool, unsigned long *mhoff)
 	return true;
 	return true;
 }
 }
 
 
+/* We don't know what alignment they asked for, but we can infer worst
+ * case from the size. */
+static unsigned int max_align(unsigned int size)
+{
+	unsigned int align = 1;
+
+	while (size % (align * 2) == 0)
+		align *= 2;
+	return align;
+}
+
 static bool check_uniform_metadata(void *pool, unsigned long *mhoff)
 static bool check_uniform_metadata(void *pool, unsigned long *mhoff)
 {
 {
 	uint8_t *meta = (uint8_t *)pool + *mhoff;
 	uint8_t *meta = (uint8_t *)pool + *mhoff;
@@ -918,8 +937,8 @@ static bool check_uniform_metadata(void *pool, unsigned long *mhoff)
 	struct uniform_cache *uc = pool;
 	struct uniform_cache *uc = pool;
 
 
 	usize = decode_usize(meta);
 	usize = decode_usize(meta);
-	if (usize == 0 || suitable_for_uc(usize, 1) != usize)
-		return false;
+	if (usize == 0 || suitable_for_uc(usize, max_align(usize)) != usize)
+		return check_fail();
 
 
 	/* If it's in uniform cache, make sure that agrees on size. */
 	/* If it's in uniform cache, make sure that agrees on size. */
 	for (i = 0; i < UNIFORM_CACHE_NUM; i++) {
 	for (i = 0; i < UNIFORM_CACHE_NUM; i++) {
@@ -933,7 +952,7 @@ static bool check_uniform_metadata(void *pool, unsigned long *mhoff)
 			continue;
 			continue;
 
 
 		if (usize != uc->size[i])
 		if (usize != uc->size[i])
-			return false;
+			return check_fail();
 	}
 	}
 	return true;
 	return true;
 }
 }
@@ -944,14 +963,14 @@ static bool check_subpage(void *pool, unsigned long poolsize,
 	unsigned long *mhoff = metadata_off(pool, page);
 	unsigned long *mhoff = metadata_off(pool, page);
 
 
 	if (*mhoff + sizeof(struct metaheader) > poolsize)
 	if (*mhoff + sizeof(struct metaheader) > poolsize)
-		return false;
+		return check_fail();
 
 
 	if (*mhoff % ALIGNOF(struct metaheader) != 0)
 	if (*mhoff % ALIGNOF(struct metaheader) != 0)
-		return false;
+		return check_fail();
 
 
 	/* It must point to a metadata page. */
 	/* It must point to a metadata page. */
 	if (!is_metadata_page(pool, poolsize, *mhoff / getpagesize()))
 	if (!is_metadata_page(pool, poolsize, *mhoff / getpagesize()))
-		return false;
+		return check_fail();
 
 
 	/* Header at start of subpage allocation */
 	/* Header at start of subpage allocation */
 	switch (get_bit_pair((uint8_t *)pool + *mhoff, 0)) {
 	switch (get_bit_pair((uint8_t *)pool + *mhoff, 0)) {
@@ -960,7 +979,7 @@ static bool check_subpage(void *pool, unsigned long poolsize,
 	case UNIFORM:
 	case UNIFORM:
 		return check_uniform_metadata(pool, mhoff);
 		return check_uniform_metadata(pool, mhoff);
 	default:
 	default:
-		return false;
+		return check_fail();
 	}
 	}
 
 
 }
 }
@@ -976,7 +995,7 @@ bool alloc_check(void *pool, unsigned long poolsize)
 		return true;
 		return true;
 
 
 	if (get_page_state(pool, 0) != TAKEN_START)
 	if (get_page_state(pool, 0) != TAKEN_START)
-		return false;
+		return check_fail();
 
 
 	/* First check metadata pages. */
 	/* First check metadata pages. */
 	/* Metadata pages will be marked TAKEN. */
 	/* Metadata pages will be marked TAKEN. */
@@ -985,21 +1004,21 @@ bool alloc_check(void *pool, unsigned long poolsize)
 
 
 		start = pool_offset(pool, mh);
 		start = pool_offset(pool, mh);
 		if (start + sizeof(*mh) > poolsize)
 		if (start + sizeof(*mh) > poolsize)
-			return false;
+			return check_fail();
 
 
 		end = pool_offset(pool, (char *)(mh+1)
 		end = pool_offset(pool, (char *)(mh+1)
 				  + get_metalen(pool, poolsize, mh));
 				  + get_metalen(pool, poolsize, mh));
 		if (end > poolsize)
 		if (end > poolsize)
-			return false;
+			return check_fail();
 
 
 		/* Non-first pages should start on a page boundary. */
 		/* Non-first pages should start on a page boundary. */
 		if (mh != first_mheader(pool, poolsize)
 		if (mh != first_mheader(pool, poolsize)
 		    && start % getpagesize() != 0)
 		    && start % getpagesize() != 0)
-			return false;
+			return check_fail();
 
 
 		/* It should end on a page boundary. */
 		/* It should end on a page boundary. */
 		if (end % getpagesize() != 0)
 		if (end % getpagesize() != 0)
-			return false;
+			return check_fail();
 	}
 	}
 
 
 	for (i = 0; i < poolsize / getpagesize(); i++) {
 	for (i = 0; i < poolsize / getpagesize(); i++) {
@@ -1010,20 +1029,20 @@ bool alloc_check(void *pool, unsigned long poolsize)
 		case FREE:
 		case FREE:
 			/* metadata pages are never free. */
 			/* metadata pages are never free. */
 			if (is_metadata)
 			if (is_metadata)
-				return false;
+				return check_fail();
 		case TAKEN_START:
 		case TAKEN_START:
 			break;
 			break;
 		case TAKEN:
 		case TAKEN:
 			/* This should continue a previous block. */
 			/* This should continue a previous block. */
 			if (last_state == FREE)
 			if (last_state == FREE)
-				return false;
+				return check_fail();
 			if (is_metadata != was_metadata)
 			if (is_metadata != was_metadata)
-				return false;
+				return check_fail();
 			break;
 			break;
 		case SPECIAL:
 		case SPECIAL:
 			/* Check metadata pointer etc. */
 			/* Check metadata pointer etc. */
 			if (!check_subpage(pool, poolsize, i))
 			if (!check_subpage(pool, poolsize, i))
-				return false;
+				return check_fail();
 		}
 		}
 		last_state = state;
 		last_state = state;
 		was_metadata = is_metadata;
 		was_metadata = is_metadata;
@@ -1061,8 +1080,9 @@ void alloc_visualize(FILE *out, void *pool, unsigned long poolsize)
 			if (meta[j / 8] & (1 << (j % 8)))
 			if (meta[j / 8] & (1 << (j % 8)))
 				total++;
 				total++;
 
 
-		printf("  %u: %u/%zu (%zu%% density)\n",
-		       uc->size[j], total, SUBPAGE_METAOFF / uc->size[i],
+		printf("  %u: %lu: %u/%zu (%zu%% density)\n",
+		       uc->size[i], uc->page[i], total,
+		       SUBPAGE_METAOFF / uc->size[i],
 		       (total * 100) / (SUBPAGE_METAOFF / uc->size[i]));
 		       (total * 100) / (SUBPAGE_METAOFF / uc->size[i]));
 	}
 	}
 
 
@@ -1095,7 +1115,7 @@ void alloc_visualize(FILE *out, void *pool, unsigned long poolsize)
 				break;
 				break;
 			case BITMAP:
 			case BITMAP:
 				/* Skip over this allocated part. */
 				/* Skip over this allocated part. */
-				len = BITMAP_METALEN * CHAR_BIT;
+				len = BITMAP_METALEN * METADATA_PER_BYTE;
 				bitmapblocks++;
 				bitmapblocks++;
 				bitmaplen += len;
 				bitmaplen += len;
 				break;
 				break;