[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 04/11] tcg: comment on which functions have to
Re: [Qemu-devel] [RFC v2 04/11] tcg: comment on which functions have to be called with tb_lock held
Thu, 5 May 2016 18:42:24 +0300
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2
On 05/05/16 18:25, Sergey Fedorov wrote:
> On 05/05/16 18:03, Alex Bennée wrote:
>> Sergey Fedorov <address@hidden> writes:
>>> On 05/04/16 18:32, Alex Bennée wrote:
>>>> diff --git a/exec.c b/exec.c
>>>> index f46e596..17f390e 100644
>>>> --- a/exec.c
>>>> +++ b/exec.c
>>>> @@ -826,6 +826,7 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int
>>>> CPUBreakpoint *bp;
>>>> + /* TODO: locking (RCU?) */
>>>> bp = g_malloc(sizeof(*bp));
>>>> bp->pc = pc;
>>> This comment is a little inconsistent. We should make access to
>>> breakpoint and watchpoint lists to be thread-safe in all the functions
>>> using them. So if we note this, it should be noted in all such places.
>>> Also, it's probably not a good idea to put such comment just above
>>> g_malloc() invocation, it could be a bit confusing. A bit more details
>>> would also be nice.
>> Good point.
>> I could really do with some tests to exercise the debugging interface. I
>> did some when I wrote the arm kvm GDB stuff (see
>> 261f4d6d3e5445f887e070f047968e756c30cf06) but it is a) not plumbed in
>> and b) not really a stress test which is what you want to be sure your
>> handling is thread safe.
> It seems we can only have a race between TCG helper inserting/removing
> break-/watchpoint and gdbstub. So maybe we could just use separate lists
> for CPU and GDB breakpoints?
However, we would still need to synchronize reads in VCPU threads with
insertions/removals in gdbstub... No much point in splitting lists, we
could just use a spinlock to serialize insertions/removals and RCU to
make reads safe.