Re: [PATCH 5/6] hw/vfio/common: Use RCU_READ macros

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 24/1/24 10:25, Manos Pitsidianakis wrote:
> On Wed, 24 Jan 2024 09:42, Philippe Mathieu-Daudé <philmd@linaro.org> 
> wrote:
>> Replace the manual rcu_read_(un)lock calls by the
>> *RCU_READ_LOCK_GUARD macros (See commit ef46ae67ba
>> "docs/style: call out the use of GUARD macros").
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/vfio/common.c | 34 ++++++++++++++++------------------
>> 1 file changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 4aa86f563c..09878a3603 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -308,13 +308,13 @@ static void vfio_iommu_map_notify(IOMMUNotifier 
>> *n, IOMMUTLBEntry *iotlb)
>>         return;
>>     }
>>
>> -    rcu_read_lock();
>> +    RCU_READ_LOCK_GUARD();
>>
>>     if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
>>         bool read_only;
>>
>>         if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
>> -            goto out;
>> +            return;
> 
> Since this is the only early return, we could alternatively do:
> 
> -         if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
> +         if (vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
> 
> remove the goto/return, and wrap the rest of the codeflow in this if's 
> brackets. And then we could use WITH_RCU_READ_LOCK_GUARD instead. That'd 
> increase the code indentation however.

If the maintainer agrees with the style & code churn, I don't
mind respining.

> 
>>         }
>>         /*
>>          * vaddr is only valid until rcu_read_unlock(). But after
>> @@ -343,8 +343,6 @@ static void vfio_iommu_map_notify(IOMMUNotifier 
>> *n, IOMMUTLBEntry *iotlb)
>>             vfio_set_migration_error(ret);
>>         }
>>     }
>> -out:
>> -    rcu_read_unlock();
>> }

> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Thanks!