Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Dmitry Osipenko]

20.01.2022 00:31, Andy Shevchenko пишет:
> On Wed, Jan 19, 2022 at 11:22 PM Dmitry Osipenko <digetx@xxxxxxxxx> wrote:
>>
>> 19.01.2022 20:49, Andy Shevchenko пишет:
>>> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@xxxxxxxxx> wrote:
>>>>
>>>> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
>>>> the CLM blob is released in the device remove path.
>>>
>>> ...
>>>
>>>>         if (ret) {
>>>
>>>>                 brcmf_err(bus, "Failed to get RAM info\n");
>>>> +               release_firmware(fw);
>>>> +               brcmf_fw_nvram_free(nvram);
>>>
>>> Can we first undo the things and only after print a message?
>>
>> Having message first usually is more preferred because at minimum you'll
>> get the message if "undoing the things" crashes, i.e. will be more
>> obvious what happened.
> 
> If "undo the things" crashes, I would rather like to see that crash
> report, while serial UART at 9600 will continue flushing the message
> and then hang without any pointers to what the heck happened. Not
> here, but in general, messages are also good to be out of the locks.

The hang is actually a better example. It's the most annoying when there
is a silent hang and no error messages.