Re: [Xen-devel] [PATCH v7 2/3] AMD/IOMMU: allow callers to request allocate_buffer() to skip its memset()

[Jan Beulich <jbeulich@suse.com>]

On 04.10.2019 15:26, Andrew Cooper wrote:
> On 26/09/2019 15:29, Jan Beulich wrote:
>> The command ring buffer doesn't need clearing up front in any event.
>> Subsequently we'll also want to avoid clearing the device tables.
>>
>> While playing with functions signatures replace undue use of fixed width
>> types at the same time, and extend this to deallocate_buffer() as well.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v7: New.
>>
>> --- a/xen/drivers/passthrough/amd/iommu_init.c
>> +++ b/xen/drivers/passthrough/amd/iommu_init.c
>> @@ -994,12 +994,12 @@ static unsigned int __init dt_alloc_size
>>                                               IOMMU_DEV_TABLE_ENTRY_SIZE);
>>  }
>>  
>> -static void __init deallocate_buffer(void *buf, uint32_t sz)
>> +static void __init deallocate_buffer(void *buf, unsigned long sz)
> 
> Probably best to use size_t here, being both shorter, and guaranteed not
> to require modification in the future.

I'd prefer not to without other related code also getting switched
from unsigned long to size_t.

>>  {
>> -    int order = 0;
>>      if ( buf )
>>      {
>> -        order = get_order_from_bytes(sz);
>> +        unsigned int order = get_order_from_bytes(sz);
>> +
>>          __free_amd_iommu_tables(buf, order);
>>      }
>>  }
> 
> How about simply
> 
> if ( buf )
>     __free_amd_iommu_tables(buf, get_order_from_bytes(sz));
> 
> which drops the order variable entirely?

Fine with me; I did actually consider doing so, but then decided
against to stay closer to what the code looked like before.

> Ideally with both of these modifications, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

Thanks.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel