Re: [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex

[Philippe Mathieu-Daudé]

On 18/11/22 10:18, Richard Henderson wrote:
> Narrow the scope of the lock to the actual read/write,
> moving the cpu_transation_failed call outside the lock.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/cputlb.c | 25 ++++++++-----------------
>   1 file changed, 8 insertions(+), 17 deletions(-)

> @@ -1367,11 +1366,11 @@ static uint64_t io_readx(CPUArchState *env, 
> CPUTLBEntryFull *full,
>           cpu_io_recompile(cpu, retaddr);
>       }
>   
> -    if (!qemu_mutex_iothread_locked()) {
> -        qemu_mutex_lock_iothread();
> -        locked = true;
> +    {
> +        QEMU_IOTHREAD_LOCK_GUARD();
> +        r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
>       }
> -    r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
> +

Example of clearer WITH_QEMU_IOTHREAD_LOCK_GUARD() macro
use suggested earlier:

        WITH_QEMU_IOTHREAD_LOCK_GUARD() {
            r = memory_region_dispatch_read(mr, mr_offset, &val,
                                            op, full->attrs);
        }

>       if (r != MEMTX_OK) {
>           hwaddr physaddr = mr_offset +
>               section->offset_within_address_space -
> @@ -1380,10 +1379,6 @@ static uint64_t io_readx(CPUArchState *env, 
> CPUTLBEntryFull *full,
>           cpu_transaction_failed(cpu, physaddr, addr, memop_size(op), 
> access_type,
>                                  mmu_idx, full->attrs, r, retaddr);
>       }
> -    if (locked) {
> -        qemu_mutex_unlock_iothread();
> -    }
> -
>       return val;
>   }

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>