On 27/08/2021 15:07, Jan Beulich wrote:
> On 27.08.2021 16:01, Andrew Cooper wrote:
>> As explained in the comments, a progress label wants to be before the
>> function
>> it refers to for the higher level logic to make sense. As it happens, the
>> effects are benign because gnttab_mappings is immediately adjacent to
>> teardown
>> in terms of co-routine exit points.
>>
>> There is and will always be a corner case with 0. Help alleviate this
>> visually (at least slightly) with a BUILD_BUG_ON() to ensure the property
>> which makes this function do anything useful.
>>
>> There is also a visual corner case when changing from PROGRESS() to
>> PROGRESS_VCPU(). The important detail is to check that there is a "return
>> rc;" logically between each PROGRESS*() marker.
>>
>> Fixes: b1ee10be5625 ("gnttab: add preemption check to
>> gnttab_release_mappings()")
>> Signed-off-by: Andrew Cooper <[email protected]>
> Acked-by: Jan Beulich <[email protected]>
Thanks.
>
> Despite the Fixes: tag I don't really view this as requiring backport.
> Then again it would need to go to 4.15 only. Will need to make up my
> mind ...
I'd prefer you to backport it. Nothing good will come of having this
difference between 4.15 and 4.16.
Amongst other things, I'm not willing to bet we've found all the long
running loops during teardown. After all, the gnttab one was staring me
in the face for years doing domain creation/destruction cleanup, before
I actually spotted it.
~Andrew