mirror of https://github.com/schoebel/mars
copy: constify state table subpages
I have seen some "strange" behaviour so infrequently, so I am unsure whether
this is a bugfix at all.
I am unsure, whether this _suspected_ bug would be in gcc, or whether
the bug would be mine.
If it is mine, then I assumed that gcc would produce better machine code
than required by ISO C.
Currently, I don't (yet) have the newest version of the ISO C standard,
so I cannot claim the truth.
Whatever: I found the _suspected_ bug by _dissassembling_ before vs after this
patch had been applied:
.type copy_brick_construct, @function .type copy_brick_construct, @function
copy_brick_construct: copy_brick_construct:
> pushq %r13 #
pushq %r12 # pushq %r12 #
pushq %rbp # | # block/mars/kernel/mars_copy.c: st = brick_block_alloc(0, PAGE_SIZE);
movl $1155, %edx #, <
> pushq %rbp #
pushq %rbx # pushq %rbx #
movl $4096, %esi #, <
movq %rdi, %rbx # brick, brick | movq %rdi, %r12 # brick, brick
> movl $4096, %esi #,
xorl %edi, %edi # xorl %edi, %edi #
call _brick_block_alloc # call _brick_block_alloc #
movq %rax, %rdx #, tmp99 | movq %rax, %rbx #, _res_
movl $1024, %ecx #, tmp102 movl $1024, %ecx #, tmp102
movq %rax, 408(%rbx) # tmp99, brick_11(D)->st <
movq %rdx, %rdi # tmp99, tmp99 <
xorl %eax, %eax # tmp101 xorl %eax, %eax # tmp101
xorl %ebp, %ebp # ivtmp.179 | movq %rbx, %rdi # _res_, _res_
xorl %r12d, %r12d # tmp106 <
rep stosl rep stosl
> xorl %ebp, %ebp # ivtmp.174
> xorl %r13d, %r13d # tmp105
.L26: .L26:
movl $1172, %edx #, | movl $1177, %edx #,
movl $4096, %esi #, movl $4096, %esi #,
xorl %edi, %edi # xorl %edi, %edi #
call _brick_block_alloc # call _brick_block_alloc #
movq %rax, %rdx #, tmp103 movq %rax, %rdx #, tmp103
movq 408(%rbx), %rax # brick_11(D)->st, brick_11(D)->st | movq %rdx, (%rbx,%rbp,8) # tmp103, MEM[base: _res__9, index: ivtmp.174_28, step:
> incq %rbp # ivtmp.174
movl $1024, %ecx #, tmp107 | movq %rax, %rdi # tmp103, _res_
movq %rdx, %rdi # tmp103, tmp103 | movl $1024, %ecx #, tmp106
movq %rdx, (%rax,%rbp) # tmp103, *_4 <
addq $8, %rbp #, ivtmp.179 <
movl %r12d, %eax # tmp106, tmp106 <
cmpq $2048, %rbp #, ivtmp.179 | cmpq $256, %rbp #, ivtmp.174
rep stosl rep stosl
jne .L26 #, jne .L26 #,
leaq 376(%rbx), %rdi #, tmp108 | leaq 376(%r12), %rdi #, tmp107
movq $__key.72209, %rdx #, | movq $__key.72227, %rdx #,
movq $.LC3, %rsi #, movq $.LC3, %rsi #,
call __init_waitqueue_head # call __init_waitqueue_head #
> # block/mars/kernel/mars_copy.c: brick->st = st;
> movq %rbx, 408(%r12) # _res_, brick_10(D)->st
xorl %eax, %eax # xorl %eax, %eax #
popq %rbx # popq %rbx #
popq %rbp # popq %rbp #
popq %r12 # popq %r12 #
> popq %r13 #
ret ret
Hint: the 2-dimenional array indexing looks _suspicious_ to me.
But analysis is not as easy as one might assume.
Please help me:
If this would be really a bug in gcc (I am not sure), it should be fixed
by the upstream of gcc. Please contact me if you know / can show that
(a) if it is _really_ a bug (currently very hard to reproduce via MARS), and
(b) the bug is really in gcc but not mine (unsure for now), and
(c) how to convert this into a _reproducer_ for the gcc team.
I am not sure whether this is a _full_ reproducer, because it might
depend on the arch (amd64) and/or on specific Linux kernel compile options.
I don't have the time for ananlysis of all of these, or maybe even more
thingies to do.
This commit is contained in:
Thomas Schoebel-Theuer
Thomas Schoebel-Theuer
parent
602d1380ec
commit
49c408cac2
|
|
@ -67,8 +67,10 @@
|
|||
register __u64 __index = (index); \
|
||||
register __u64 __page_index = __index / STATES_PER_PAGE; \
|
||||
register __u64 __state_index = __index % STATES_PER_PAGE; \
|
||||
void *const *___st = (brick)->st; \
|
||||
struct copy_state *__st = ___st[__page_index]; \
|
||||
\
|
||||
&((brick)->st[__page_index][__state_index]); \
|
||||
&__st[__state_index]; \
|
||||
})
|
||||
|
||||
///////////////////////// own type definitions ////////////////////////
|
||||
|
|
@ -1346,10 +1348,16 @@ MARS_MAKE_STATICS(copy);
|
|||
static
|
||||
void _free_pages(struct copy_brick *brick)
|
||||
{
|
||||
void **st;
|
||||
unsigned i;
|
||||
|
||||
st = (void **)brick->st;
|
||||
if (!st)
|
||||
return;
|
||||
brick->st = NULL;
|
||||
|
||||
for (i = 0; i < MAX_SUB_TABLES; i++) {
|
||||
struct copy_state *sub_table = brick->st[i];
|
||||
void *sub_table = st[i];
|
||||
|
||||
if (!sub_table) {
|
||||
continue;
|
||||
|
|
@ -1357,15 +1365,16 @@ void _free_pages(struct copy_brick *brick)
|
|||
|
||||
brick_block_free(sub_table, PAGE_SIZE);
|
||||
}
|
||||
brick_block_free(brick->st, PAGE_SIZE);
|
||||
brick_block_free(st, PAGE_SIZE);
|
||||
}
|
||||
|
||||
static int copy_brick_construct(struct copy_brick *brick)
|
||||
{
|
||||
void **st;
|
||||
unsigned i;
|
||||
|
||||
brick->st = brick_block_alloc(0, PAGE_SIZE);
|
||||
memset(brick->st, 0, PAGE_SIZE);
|
||||
st = brick_block_alloc(0, PAGE_SIZE);
|
||||
memset(st, 0, PAGE_SIZE);
|
||||
|
||||
for (i = 0; i < MAX_SUB_TABLES; i++) {
|
||||
struct copy_state *sub_table;
|
||||
|
|
@ -1378,11 +1387,12 @@ static int copy_brick_construct(struct copy_brick *brick)
|
|||
}
|
||||
|
||||
sub_table = brick_block_alloc(0, PAGE_SIZE);
|
||||
brick->st[i] = sub_table;
|
||||
memset(sub_table, 0, PAGE_SIZE);
|
||||
st[i] = sub_table;
|
||||
}
|
||||
|
||||
init_waitqueue_head(&brick->event);
|
||||
brick->st = st;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -113,7 +113,7 @@ struct copy_brick {
|
|||
long long last_jiffies;
|
||||
wait_queue_head_t event;
|
||||
struct task_struct *thread;
|
||||
struct copy_state **st;
|
||||
void *const *st;
|
||||
};
|
||||
|
||||
struct copy_input {
|
||||
|
|
|
|||
Loading…
Reference in New Issue