[RFC PATCH kernel] vfio/spapr_tce: Get rid of possible infinite loop

[Michael Ellerman]

Serhii Popovych <spopovyc at redhat.com> writes:
> Alexey Kardashevskiy wrote:
>> As a part of cleanup, the SPAPR TCE IOMMU subdriver releases preregistered
>> memory. If there is a bug in memory release, the loop in
>> tce_iommu_release() becomes infinite; this actually happened to me.
>> 
>> This makes the loop finite and prints a warning on every failure to make
>> the code more bug prone.
>> 
>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>> ---
>>  drivers/vfio/vfio_iommu_spapr_tce.c | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>> index b1a8ab3..ece0651 100644
>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>> @@ -393,13 +394,8 @@ static void tce_iommu_release(void *iommu_data)
>>  		tce_iommu_free_table(container, tbl);
>>  	}
>>  
>> -	while (!list_empty(&container->prereg_list)) {
>> -		struct tce_iommu_prereg *tcemem;
>> -
>> -		tcemem = list_first_entry(&container->prereg_list,
>> -				struct tce_iommu_prereg, next);
>> -		WARN_ON_ONCE(tce_iommu_prereg_free(container, tcemem));
>> -	}
>> +	list_for_each_entry_safe(tcemem, tmtmp, &container->prereg_list, next)
>> +		WARN_ON(tce_iommu_prereg_free(container, tcemem));
>
> I'm not sure that tce_iommu_prereg_free() call under WARN_ON() is good
> idea because WARN_ON() is a preprocessor macro:
>
>   if CONFIG_WARN=n is added by the analogy with CONFIG_BUG=n defining
>   WARN_ON() as empty we will loose call to tce_iommu_prereg_free()
>   leaking resources.

I don't think that's likely to ever happen though, we have a large
number of uses that would need to be checked one-by-one:

  $ git grep "if (WARN_ON(" | wc -l
  2853


So if we ever did add CONFIG_WARN, I think it would still need to
evaluate the condition, just not emit a warning.

cheers