Skip to content

Commit 1635686

Browse files
committed
MDEV-23510: arm64 lf_hash alignment of pointers
volatile != atomic. volatile has no memory barrier schemantics, its for mmaped IO so lets allow some optimizer gains and stop pretending it helps with memory atomicity. The MDEV lists a SEGV an assumption is made that an address was partially read. As C packs structs strictly in order and on arm64 the cache line size is 128 bits. A pointer (link - 64 bits), followed by a hashnr (uint32 - 32 bits), leaves the following key (uchar * 64 bits), neither naturally aligned to any pointer and worse, split across a cache line which is the processors view of an atomic reservation of memory. lf_dynarray_lvalue is assumed to return a 64 bit aligned address. As a solution move the 32bit hashnr to the end so we don't get the *key pointer split across two cache lines. Tested by: Krunal Bauskar Reviewer: Marko Mäkelä
1 parent 9e259d5 commit 1635686

File tree

1 file changed

+14
-14
lines changed

1 file changed

+14
-14
lines changed

mysys/lf_hash.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@
3131

3232
/* An element of the list */
3333
typedef struct {
34-
intptr volatile link; /* a pointer to the next element in a list and a flag */
35-
uint32 hashnr; /* reversed hash number, for sorting */
34+
intptr link; /* a pointer to the next element in a list and a flag */
3635
const uchar *key;
3736
size_t keylen;
37+
uint32 hashnr; /* reversed hash number, for sorting */
3838
/*
3939
data is stored here, directly after the keylen.
4040
thus the pointer to data is (void*)(slist_element_ptr+1)
@@ -48,7 +48,7 @@ const int LF_HASH_OVERHEAD= sizeof(LF_SLIST);
4848
in a list) from l_find to l_insert/l_delete
4949
*/
5050
typedef struct {
51-
intptr volatile *prev;
51+
intptr *prev;
5252
LF_SLIST *curr, *next;
5353
} CURSOR;
5454

@@ -85,7 +85,7 @@ typedef struct {
8585
0 - ok
8686
1 - error (callbck returned 1)
8787
*/
88-
static int l_find(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
88+
static int l_find(LF_SLIST **head, CHARSET_INFO *cs, uint32 hashnr,
8989
const uchar *key, uint keylen, CURSOR *cursor, LF_PINS *pins,
9090
my_hash_walk_action callback)
9191
{
@@ -168,7 +168,7 @@ static int l_find(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
168168
it uses pins[0..2], on return all pins are removed.
169169
if there're nodes with the same key value, a new node is added before them.
170170
*/
171-
static LF_SLIST *l_insert(LF_SLIST * volatile *head, CHARSET_INFO *cs,
171+
static LF_SLIST *l_insert(LF_SLIST **head, CHARSET_INFO *cs,
172172
LF_SLIST *node, LF_PINS *pins, uint flags)
173173
{
174174
CURSOR cursor;
@@ -220,7 +220,7 @@ static LF_SLIST *l_insert(LF_SLIST * volatile *head, CHARSET_INFO *cs,
220220
NOTE
221221
it uses pins[0..2], on return all pins are removed.
222222
*/
223-
static int l_delete(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
223+
static int l_delete(LF_SLIST **head, CHARSET_INFO *cs, uint32 hashnr,
224224
const uchar *key, uint keylen, LF_PINS *pins)
225225
{
226226
CURSOR cursor;
@@ -278,7 +278,7 @@ static int l_delete(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr,
278278
it uses pins[0..2], on return the pin[2] keeps the node found
279279
all other pins are removed.
280280
*/
281-
static LF_SLIST *l_search(LF_SLIST * volatile *head, CHARSET_INFO *cs,
281+
static LF_SLIST *l_search(LF_SLIST **head, CHARSET_INFO *cs,
282282
uint32 hashnr, const uchar *key, uint keylen,
283283
LF_PINS *pins)
284284
{
@@ -319,7 +319,7 @@ static inline my_hash_value_type calc_hash(CHARSET_INFO *cs,
319319

320320
#define MAX_LOAD 1.0 /* average number of elements in a bucket */
321321

322-
static int initialize_bucket(LF_HASH *, LF_SLIST * volatile*, uint, LF_PINS *);
322+
static int initialize_bucket(LF_HASH *, LF_SLIST **, uint, LF_PINS *);
323323

324324
static void default_initializer(LF_HASH *hash, void *dst, const void *src)
325325
{
@@ -398,7 +398,7 @@ void lf_hash_destroy(LF_HASH *hash)
398398
int lf_hash_insert(LF_HASH *hash, LF_PINS *pins, const void *data)
399399
{
400400
int csize, bucket, hashnr;
401-
LF_SLIST *node, * volatile *el;
401+
LF_SLIST *node, **el;
402402

403403
node= (LF_SLIST *)lf_alloc_new(pins);
404404
if (unlikely(!node))
@@ -437,7 +437,7 @@ int lf_hash_insert(LF_HASH *hash, LF_PINS *pins, const void *data)
437437
*/
438438
int lf_hash_delete(LF_HASH *hash, LF_PINS *pins, const void *key, uint keylen)
439439
{
440-
LF_SLIST * volatile *el;
440+
LF_SLIST **el;
441441
uint bucket, hashnr;
442442

443443
hashnr= hash->hash_function(hash->charset, (uchar *)key, keylen) & INT_MAX32;
@@ -473,7 +473,7 @@ void *lf_hash_search_using_hash_value(LF_HASH *hash, LF_PINS *pins,
473473
my_hash_value_type hashnr,
474474
const void *key, uint keylen)
475475
{
476-
LF_SLIST * volatile *el, *found;
476+
LF_SLIST **el, *found;
477477
uint bucket;
478478

479479
/* hide OOM errors - if we cannot initialize a bucket, try the previous one */
@@ -507,7 +507,7 @@ int lf_hash_iterate(LF_HASH *hash, LF_PINS *pins,
507507
CURSOR cursor;
508508
uint bucket= 0;
509509
int res;
510-
LF_SLIST * volatile *el;
510+
LF_SLIST **el;
511511

512512
el= lf_dynarray_lvalue(&hash->array, bucket);
513513
if (unlikely(!el))
@@ -539,13 +539,13 @@ static const uchar *dummy_key= (uchar*)"";
539539
0 - ok
540540
-1 - out of memory
541541
*/
542-
static int initialize_bucket(LF_HASH *hash, LF_SLIST * volatile *node,
542+
static int initialize_bucket(LF_HASH *hash, LF_SLIST **node,
543543
uint bucket, LF_PINS *pins)
544544
{
545545
uint parent= my_clear_highest_bit(bucket);
546546
LF_SLIST *dummy= (LF_SLIST *)my_malloc(sizeof(LF_SLIST), MYF(MY_WME));
547547
LF_SLIST **tmp= 0, *cur;
548-
LF_SLIST * volatile *el= lf_dynarray_lvalue(&hash->array, parent);
548+
LF_SLIST **el= lf_dynarray_lvalue(&hash->array, parent);
549549
if (unlikely(!el || !dummy))
550550
return -1;
551551
if (*el == NULL && bucket &&

0 commit comments

Comments
 (0)