Hi Andi and Jonathan,
[...]
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > index 619c70c54ef9..4f252f704975 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > @@ -904,9 +904,7 @@ static void active_engine(struct kthread_work *work)
> > arg->result = PTR_ERR(ce[count]);
> > pr_err("[%s] Create context #%ld failed: %d!\n",
> > engine->name, count, arg->result);
> > - if (!count)
> > - return;
> > - while (--count)
> > + while (count--)
>
> This is a good catch, but we still need to decrease count by 1
> before entering the loop, right?
I think Jonathan's change is good, here is why:
count == 0 with current code:
if (!count)
return; -> we skip the loop entirely, which is right,
because we do not have any objects to put
while (--count)
count == 1 with current code:
if (!count)
return;
while (--count) -> count = 0, then the evaluation of expression
inside brackts happens and we skip putting one context (leak)
count == 0 with Jonathan's change:
while(count--) -> the evaluation of expression happens before we
decrement count, so we skip the loop, which is correct, as there
are no contexts allocated
count == 1 with Jonathan's change:
while (count--) -> the evaluation..., then count = 0, so we
enter while loop only once to put the context
So with Jonathan's change we kill two birds with one stone:
1) we still fix the original overflow on count values inside
while loop that I fixed with my patch,
2) and also the overflow, which my patch introduced for
count == 1.
Nice catch Jonathan!
Reviewed-by: Krzysztof Karas <[email protected]>
--
Best Regards,
Krzysztof
