Skip to content

Commit

Permalink
expand NEED_ALIGN for chunked items
Browse files Browse the repository at this point in the history
some whackarse ARM platforms on specific glibc/gcc (new?) versions trip
SIGBUS while reading the header chunk for a split item.

the header chunk is unfortunate magic: It lives in ITEM_data() at a random
offset, is zero sized, and only exists to simplify code around finding the
orignial slab class, and linking/relinking subchunks to an item.

there's no fix to this which isn't a lot of code. I need to refactor chunked
items, and attempted to do so, but couldn't come up with something I liked
quickly enough.

This change pads the first chunk if alignment is necessary, which wastes
bytes and a little CPU, but I'm not going to worry a ton for these obscure
platforms.

this works with rebalancing because in the case of ITEM_CHUNKED header, it
treats the item size as the size of the class it resides in, and memcpy's the
item during recovery.

all other cases were changes from ITEM_data to a new ITEM_schunk() inline
function that is created when NEED_ALIGN is set, else it's equal to ITEM_data
still.
  • Loading branch information
dormando committed Aug 9, 2018
1 parent 89bf7ab commit 93dfe27
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 9 deletions.
9 changes: 8 additions & 1 deletion items.c
Expand Up @@ -285,6 +285,13 @@ item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,
if (settings.use_cas) {
htotal += sizeof(uint64_t);
}
#ifdef NEED_ALIGN
// header chunk needs to be padded on some systems
int remain = htotal % 8;
if (remain != 0) {
htotal += 8 - remain;
}
#endif
hdr_id = slabs_clsid(htotal);
it = do_item_alloc_pull(htotal, hdr_id);
/* setting ITEM_CHUNKED is fine here because we aren't LINKED yet. */
Expand Down Expand Up @@ -336,7 +343,7 @@ item *do_item_alloc(char *key, const size_t nkey, const unsigned int flags,

/* Initialize internal chunk. */
if (it->it_flags & ITEM_CHUNKED) {
item_chunk *chunk = (item_chunk *) ITEM_data(it);
item_chunk *chunk = (item_chunk *) ITEM_schunk(it);

chunk->next = 0;
chunk->prev = 0;
Expand Down
33 changes: 28 additions & 5 deletions memcached.c
Expand Up @@ -1032,7 +1032,7 @@ static int add_iov(conn *c, const void *buf, int len) {

static int add_chunked_item_iovs(conn *c, item *it, int len) {
assert(it->it_flags & ITEM_CHUNKED);
item_chunk *ch = (item_chunk *) ITEM_data(it);
item_chunk *ch = (item_chunk *) ITEM_schunk(it);
while (ch) {
int todo = (len > ch->used) ? ch->used : len;
if (add_iov(c, ch->data, todo) != 0) {
Expand Down Expand Up @@ -2485,7 +2485,15 @@ static void process_bin_update(conn *c) {
}

c->item = it;
#ifdef NEED_ALIGN
if (it->it_flags & ITEM_CHUNKED) {
c->ritem = ITEM_schunk(it);
} else {
c->ritem = ITEM_data(it);
}
#else
c->ritem = ITEM_data(it);
#endif
c->rlbytes = vlen;
conn_set_state(c, conn_nread);
c->substate = bin_read_set_value;
Expand Down Expand Up @@ -2540,7 +2548,15 @@ static void process_bin_append_prepend(conn *c) {
}

c->item = it;
#ifdef NEED_ALIGN
if (it->it_flags & ITEM_CHUNKED) {
c->ritem = ITEM_schunk(it);
} else {
c->ritem = ITEM_data(it);
}
#else
c->ritem = ITEM_data(it);
#endif
c->rlbytes = vlen;
conn_set_state(c, conn_nread);
c->substate = bin_read_set_value;
Expand Down Expand Up @@ -2701,7 +2717,7 @@ static void complete_nread(conn *c) {
/* Destination must always be chunked */
/* This should be part of item.c */
static int _store_item_copy_chunks(item *d_it, item *s_it, const int len) {
item_chunk *dch = (item_chunk *) ITEM_data(d_it);
item_chunk *dch = (item_chunk *) ITEM_schunk(d_it);
/* Advance dch until we find free space */
while (dch->size == dch->used) {
if (dch->next) {
Expand All @@ -2713,7 +2729,7 @@ static int _store_item_copy_chunks(item *d_it, item *s_it, const int len) {

if (s_it->it_flags & ITEM_CHUNKED) {
int remain = len;
item_chunk *sch = (item_chunk *) ITEM_data(s_it);
item_chunk *sch = (item_chunk *) ITEM_schunk(s_it);
int copied = 0;
/* Fills dch's to capacity, not straight copy sch in case data is
* being added or removed (ie append/prepend)
Expand Down Expand Up @@ -3719,7 +3735,7 @@ static inline int _get_extstore(conn *c, item *it, int iovst, int iovcnt) {
if (chunked) {
unsigned int ciovcnt = 1;
size_t remain = new_it->nbytes;
item_chunk *chunk = (item_chunk *) ITEM_data(new_it);
item_chunk *chunk = (item_chunk *) ITEM_schunk(new_it);
io->io.iov = &c->iov[c->iovused];
// fill the header so we can get the full data + crc back.
add_iov(c, new_it, ITEM_ntotal(new_it) - new_it->nbytes);
Expand Down Expand Up @@ -4080,7 +4096,15 @@ static void process_update_command(conn *c, token_t *tokens, const size_t ntoken
ITEM_set_cas(it, req_cas_id);

c->item = it;
#ifdef NEED_ALIGN
if (it->it_flags & ITEM_CHUNKED) {
c->ritem = ITEM_schunk(it);
} else {
c->ritem = ITEM_data(it);
}
#else
c->ritem = ITEM_data(it);
#endif
c->rlbytes = it->nbytes;
c->cmd = comm;
conn_set_state(c, conn_nread);
Expand Down Expand Up @@ -5293,7 +5317,6 @@ static int read_into_chunked_item(conn *c) {

while (c->rlbytes > 0) {
item_chunk *ch = (item_chunk *)c->ritem;
assert(ch->used <= ch->size);
if (ch->size == ch->used) {
// FIXME: ch->next is currently always 0. remove this?
if (ch->next) {
Expand Down
17 changes: 17 additions & 0 deletions memcached.h
Expand Up @@ -524,6 +524,23 @@ typedef struct _strchunk {
uint8_t slabs_clsid; /* Same as above. */
char data[];
} item_chunk;

#ifdef NEED_ALIGN
static inline char *ITEM_schunk(item *it) {
int offset = it->nkey + 1 + it->nsuffix
+ ((it->it_flags & ITEM_CAS) ? sizeof(uint64_t) : 0);
int remain = offset % 8;
if (remain != 0) {
offset += 8 - remain;
}
return ((char *) &(it->data)) + offset;
}
#else
#define ITEM_schunk(item) ((char*) &((item)->data) + (item)->nkey + 1 \
+ (item)->nsuffix \
+ (((item)->it_flags & ITEM_CAS) ? sizeof(uint64_t) : 0))
#endif

#ifdef EXTSTORE
typedef struct {
unsigned int page_version; /* from IO header */
Expand Down
12 changes: 10 additions & 2 deletions slabs.c
Expand Up @@ -380,7 +380,7 @@ static void *do_slabs_alloc(const size_t size, unsigned int id, uint64_t *total_
}

static void do_slabs_free_chunked(item *it, const size_t size) {
item_chunk *chunk = (item_chunk *) ITEM_data(it);
item_chunk *chunk = (item_chunk *) ITEM_schunk(it);
slabclass_t *p;

it->it_flags = ITEM_SLABBED;
Expand All @@ -404,7 +404,15 @@ static void do_slabs_free_chunked(item *it, const size_t size) {
p->slots = it;
p->sl_curr++;
// TODO: macro
#ifdef NEED_ALIGN
int total = it->nkey + 1 + it->nsuffix + sizeof(item) + sizeof(item_chunk);
if (total % 8 != 0) {
total += 8 - (total % 8);
}
p->requested -= total;
#else
p->requested -= it->nkey + 1 + it->nsuffix + sizeof(item) + sizeof(item_chunk);
#endif
if (settings.use_cas) {
p->requested -= sizeof(uint64_t);
}
Expand Down Expand Up @@ -1013,7 +1021,7 @@ static int slab_rebalance_move(void) {
do_item_replace(it, new_it, hv);
/* Need to walk the chunks and repoint head */
if (new_it->it_flags & ITEM_CHUNKED) {
item_chunk *fch = (item_chunk *) ITEM_data(new_it);
item_chunk *fch = (item_chunk *) ITEM_schunk(new_it);
fch->next->prev = fch;
while (fch) {
fch->head = new_it;
Expand Down
2 changes: 1 addition & 1 deletion storage.c
Expand Up @@ -62,7 +62,7 @@ static int storage_write(void *storage, const int clsid, const int item_age) {
// TODO: should be in items.c
if (it->it_flags & ITEM_CHUNKED) {
// Need to loop through the item and copy
item_chunk *sch = (item_chunk *) ITEM_data(it);
item_chunk *sch = (item_chunk *) ITEM_schunk(it);
int remain = orig_ntotal;
int copied = 0;
// copy original header
Expand Down

0 comments on commit 93dfe27

Please sign in to comment.