Skip to content

Commit

Permalink
fix off-by-one with slab management
Browse files Browse the repository at this point in the history
data sticking into the highest slab class was unallocated. Thanks to pyry for
the repro case:

perl -e 'use Cache::Memcached;$memd = new Cache::Memcached {
servers=>["127.0.0.1:11212"]};for(20..1000){print "$_\n";$memd->set("fo2$_",
"a"x1024)};'
(in a loop)
with:
./memcached -v -m 32 -p 11212 -f 1.012

This serves as a note to turn this into a test.
  • Loading branch information
dormando committed Apr 20, 2015
1 parent c10feb9 commit a2fc8e9
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 6 deletions.
2 changes: 1 addition & 1 deletion items.c
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,7 @@ enum crawler_result_type lru_crawler_crawl(char *slabs) {
p = strtok_r(NULL, ",", &b)) {

if (!safe_strtoul(p, &sid) || sid < POWER_SMALLEST
|| sid >= MAX_NUMBER_OF_SLAB_CLASSES) {
|| sid >= MAX_NUMBER_OF_SLAB_CLASSES-1) {
pthread_mutex_unlock(&lru_crawler_lock);
return CRAWLER_BADCLASS;
}
Expand Down
2 changes: 1 addition & 1 deletion memcached.c
Original file line number Diff line number Diff line change
Expand Up @@ -2835,7 +2835,7 @@ static void process_stat(conn *c, token_t *tokens, const size_t ntokens) {
return;
}

if (id >= MAX_NUMBER_OF_SLAB_CLASSES) {
if (id >= MAX_NUMBER_OF_SLAB_CLASSES-1) {
out_string(c, "CLIENT_ERROR Illegal slab id");
return;
}
Expand Down
2 changes: 1 addition & 1 deletion memcached.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
#define POWER_LARGEST 255
#define CHUNK_ALIGN_BYTES 8
/* slab class max is a 6-bit number, -1. */
#define MAX_NUMBER_OF_SLAB_CLASSES 63
#define MAX_NUMBER_OF_SLAB_CLASSES (63 + 1)

/** How long an object can reasonably be assumed to be locked before
harvesting it on a low memory condition. Default: disabled. */
Expand Down
5 changes: 2 additions & 3 deletions slabs.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void slabs_init(const size_t limit, const double factor, const bool prealloc) {

memset(slabclass, 0, sizeof(slabclass));

while (++i < MAX_NUMBER_OF_SLAB_CLASSES && size <= settings.item_size_max / factor) {
while (++i < MAX_NUMBER_OF_SLAB_CLASSES-1 && size <= settings.item_size_max / factor) {
/* Make sure items are always n-byte aligned */
if (size % CHUNK_ALIGN_BYTES)
size += CHUNK_ALIGN_BYTES - (size % CHUNK_ALIGN_BYTES);
Expand Down Expand Up @@ -161,7 +161,7 @@ static void slabs_preallocate (const unsigned int maxslabs) {
list. if you really don't want this, you can rebuild without
these three lines. */

for (i = POWER_SMALLEST; i <= MAX_NUMBER_OF_SLAB_CLASSES; i++) {
for (i = POWER_SMALLEST; i < MAX_NUMBER_OF_SLAB_CLASSES; i++) {
if (++prealloc > maxslabs)
return;
if (do_slabs_newslab(i) == 0) {
Expand Down Expand Up @@ -234,7 +234,6 @@ static void *do_slabs_alloc(const size_t size, unsigned int id, unsigned int *to
MEMCACHED_SLABS_ALLOCATE_FAILED(size, 0);
return NULL;
}

p = &slabclass[id];
assert(p->sl_curr == 0 || ((item *)p->slots)->slabs_clsid == 0);

Expand Down

0 comments on commit a2fc8e9

Please sign in to comment.