Re: [PATCH] firmware: fix async/manual firmware loading

[Luis R. Rodriguez]

On Thu, Nov 10, 2016 at 1:04 PM, Yves-Alexis Perez <corsac@xxxxxxxxxx> wrote:
> On Thu, 2016-11-10 at 11:48 -0800, Luis R. Rodriguez wrote:
>> > I haven't verified that this particular use case actually worked before,
>> > but this code works with lower timeout values (e.g. 60 in the fallback
>> > case), so this looks isolated.
>>
>> This is true, but as I noted the broken aspect was when the timeout
>> was set to the max value.
>>
>> > The bug was clearly introduced in v4.0 by:
>> >
>> > 68ff2a00dbf5 "firmware_loader: handle timeout via
>> > wait_for_completion_interruptible_timeout()"
>> >
>> > So please add a Fixes: and
>> >
>> > Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>>
>> This I agree with, thanks for that, and because of this then:
>>
>> Acked-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
>>
>> And because of this do recommend it for stable. I would still prefer
>> at least a new re-submit with the respected tags and a changed commit
>> log describing the reason for the fix, how the cast is an issue
>> exactly, and how this is a regression.
>
> Hi all,
>
> I'm still slightly confused, but I can certainly re-submit it. I've added the
> CC: stable because I first experienced it on a stable box, but indeed it was
> during coding a kernel driver so it might not be what's expected in
> âregressionâ here.

If you look at the code added on commit 68ff2a00dbf5
("firmware_loader: handle timeout via
wait_for_completion_interruptible_timeout()") by Ming you'll see that
the else statement setting the timeout to MAX_JIFFY_OFFSET for the
non-udev event was only added there, so this commit caused the cast to
fail, as such this should be the commit that introduced the issue.

Then

git describe --contains 68ff2a00dbf5
v4.0-rc1~83^2~1

So this was added as of v4.0, so the regression appeared first as of
v4.0. You could try v3.19 or just revert this commit to verify if this
would have fixed the issue you are reporting to confirm this indeed
was the regression, but upon code examination it seems to be the case.

> I'll try to collect the tag and rewrite the commit log, then re-submit,
> hopefully not messing with git send-email this time.

Thanks!

  Luis