[MDEV-9857] CACHE_LINE_SIZE in innodb should be 128 on POWER Created: 2016-04-01  Updated: 2016-06-30  Resolved: 2016-06-07

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Fix Version/s: 10.2.1

Type: Task Priority: Major
Reporter: Daniel Black Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 0
Labels: contribution, foundation, patch

Issue Links:
Relates
relates to MDEV-10194 CACHE_LINE_SIZE malloc align Closed
relates to MDEV-6533 MySQL Bug#72718 - CACHE_LINE_SIZE in ... Closed
Sprint: 10.2.1-1, 10.2.1-2

 Description   

MDEV-6533 didn't fully contain all CACHE_LINE_SIZE elements.

The forthcoming github pull request will. Also includes patches in Performance Schema.

 Comments   
Comment by Sergey Vojtovich [ 2016-05-03 ]

serg, I merged everything I found reasonable. Could you have another look at this PR?
Merged changes are here: https://github.com/MariaDB/server/commits/bb-10.2-mdev9857

Comment by Sergey Vojtovich [ 2016-06-02 ]

License confirmed. There's also PR#178 now.

Comment by Sergei Golubchik [ 2016-06-06 ]

Looks ok. The only comment is about

 /*
 
   we want sizeof(LF_PINS) to be CPU_LEVEL1_DCACHE_LINESIZE * 2
 
   to avoid false sharing
 
 */
 
 char pad[CPU_LEVEL1_DCACHE_LINESIZE * 2 - sizeof(uint32) * 2
 
                                         - sizeof(LF_PINBOX*)
 
                                         - sizeof(void*)
 
                                         - sizeof(void*) * (LF_PINBOX_PINS + 1

This is not exactly true. We don't want sizeof(LF_PINS) to be CPU_LEVEL1_DCACHE_LINESIZE * 2, we want
sizeof(LF_PINS) % CPU_LEVEL1_DCACHE_LINESIZE == 0, but whether it'll be two cache lines, one cache line, three cache lines or ten cache lines — doesn't matter much. Preferably, as few as possible. With cache line size being 64, I had to use 64*2, because LF_PINS didn't fit in 64 bytes. But with cache line size being 128, LF_PINS will fit in one cache line, there's no need to allocate 256 bytes for it.

It'd be perfect if you could change it to use as few cache lines as possible.

Comment by Daniel Black [ 2016-06-07 ]

PR#178 removed padding as a mechanism for alignment in favour of using compiler alignment combined with malloc alignment to keep these on different cache lines.

Test case to show the alignment of the structure member affects the structure size: https://gist.github.com/grooverdan/19a7c8d8ac0e680ba6fcf4668962b9c4

The original pad calculation that did include a mod - https://github.com/MariaDB/server/pull/169/files#diff-41202b17494e25e891bbfa27df165331R71
As the pad isn't actually referenced in the code, additional size just means wasted memory.

Comment by Sergey Vojtovich [ 2016-06-07 ]

serg, remember our discussion re paddings:

svoj> > > > > +  struct st_locks
 
svoj> > > > > +  {
 
svoj> > > > > +    mysql_rwlock_t lock;
 
svoj> > > > > +    char pad[128];
 
serg> > > >
 
serg> > > > In these cases I usually do: pad[128 - sizeof(mysql_rwlock_t)];
 
svoj> > > Hmm... strange that I didn't define macro for this, I thought I did.
 
svoj> > >
 
svoj> > > Your suggestion guarantees that st_locks will be at least 128 bytes. But these
 
svoj> > > 128 bytes may be split across 2 cache lines. Or malloc returns pointer aligned
 
svoj> > > on cache line size?
 
serg> >
 
serg> > No, it doesn't. As far as I know. But I only care that different locks
 
serg> > go into different cache lines, they don't need to be aligned to 128 byte
 
serg> > boundary.
 
svoj> What about this:
 
svoj> sizeof(mysql_rwlock_t)= 64 (just rough estimation)
 
svoj> sizeof(pad[128 - sizeof(mysql_rwlock_t)])= 64
 
svoj> 
svoj> It gives:
 
svoj>  +-------------------------------+-------------+-------------------------------+
 
svoj>  | rw_lock_t 64b                 | padding 64b | rw_lock_t 64b                 |
 
svoj>  +---------------+---------------+-------------+---------------+---------------+
 
svoj>  Cache line 128b |               Cache line 128b               |Cache line 128b
 
svoj>  ----------------+---------------------------------------------+----------------
 
serg> Okay, indeed. You're right.

Your statetement is correct for aligned allocs. Minimal padding in this particular case is "char pad[CPU_LEVEL1_DCACHE_LINESIZE]". I'll fix it accordingly.

Comment by Daniel Black [ 2016-06-29 ]

If you have aligned allocs and and aligned structure elements do you actually still need padding? (https://github.com/MariaDB/server/pull/185).

Comment by Sergey Vojtovich [ 2016-06-30 ]

No padding for aligned structures is needed.

Generated at Thu Feb 08 07:37:51 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.