Re: [Qemu-devel] [PATCH 05/10] tcg: Prepare TB invalidation for lockless TB lookup

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 20/07/16 01:27, Paolo Bonzini wrote:
>
> ----- Original Message -----
>> From: "Sergey Fedorov" <serge.fdrv@gmail.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>, qemu-devel@nongnu.org
>> Cc: "sergey fedorov" <sergey.fedorov@linaro.org>, "alex bennee" <alex.bennee@linaro.org>
>> Sent: Tuesday, July 19, 2016 9:56:49 PM
>> Subject: Re: [PATCH 05/10] tcg: Prepare TB invalidation for lockless TB lookup
>>
>> On 19/07/16 11:32, Paolo Bonzini wrote:
>> It looks much better now :)
>>
>>> When invalidating a translation block, set an invalid flag into the
>>> TranslationBlock structure first.  It is also necessary to check whether
>>> the target TB is still valid after acquiring 'tb_lock' but before calling
>>> tb_add_jump() since TB lookup is to be performed out of 'tb_lock' in
>>> future. Note that we don't have to check 'last_tb'; an already invalidated
>>> TB will not be executed anyway and it is thus safe to patch it.
>>>
>>> Suggested-by: Sergey Fedorov <serge.fdrv@gmail.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  cpu-exec.c              | 5 +++--
>>>  include/exec/exec-all.h | 2 ++
>>>  translate-all.c         | 3 +++
>>>  3 files changed, 8 insertions(+), 2 deletions(-)
>> (snip)
>>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>>> index acda7b6..bc0bcc5 100644
>>> --- a/include/exec/exec-all.h
>>> +++ b/include/exec/exec-all.h
>>> @@ -213,6 +213,8 @@ struct TranslationBlock {
>>>  #define CF_USE_ICOUNT  0x20000
>>>  #define CF_IGNORE_ICOUNT 0x40000 /* Do not generate icount code */
>>>  
>>> +    uint16_t invalid;
>> Why not "int"?
> There's a hole there, we may want to move something else so I
> used a smaller data type.  Even uint8_t would do.

But could simple "bool" work as well here?

>
> Paolo
>>> +
>>>      void *tc_ptr;    /* pointer to the translated code */
>>>      uint8_t *tc_search;  /* pointer to search data */

Are you sure that the hole is over there, not here?

Kind regards,
Sergey

>>>      /* original tb when cflags has CF_NOCACHE */
>>>
>> Thanks,
>> Sergey
>>