Re: [TECH TOPIC] Driver probe fails and register succeeds

[Shuah Khan <skhan@linuxfoundation.org>]

On 6/23/22 5:30 PM, Laurent Pinchart wrote:
> Hi Shuah,
> 
> On Thu, Jun 23, 2022 at 05:28:09PM -0600, Shuah Khan wrote:
>> On 6/23/22 5:13 PM, Laurent Pinchart wrote:
>>> On Thu, Jun 23, 2022 at 05:05:30PM -0600, Shuah Khan wrote:
>>>> I have been debugging a driver probe failure and noticed that driver gets
>>>> registered even when driver probe fails. This is not a new behavior. The
>>>> code in question is the same since 2005.
>>>>
>>>> dmesg will say that a driver probe failed with error code and then the very
>>>> next message from interface core that says driver is registered successfully.
>>>> It will create sysfs interfaces.
>>>>
>>>> The probe failure is propagated from the drive probe routine all the way up to
>>>> __driver_attach(). __driver_attach() ignores the error and and returns success.
>>>>
>>>>            __device_driver_lock(dev, dev->parent);
>>>>            driver_probe_device(drv, dev);
>>>>            __device_driver_unlock(dev, dev->parent);
>>>>
>>>>            return 0;
>>>>
>>>> Interface driver register goes on to create sysfs entries as if driver probe
>>>> worked. It handles errors from driver_register() and unwinds the register
>>>> properly, however in this case it doesn't know about the failure.
>>>>
>>>> At this point the driver is defunct with sysfs interfaces. User has to run
>>>> rmmod to get rid of the defunct driver.
>>>>
>>>> Simply returning the error from __driver_attach() didn't work as expected.
>>>> I figured it would fail since not all interface drivers can handle errors
>>>> from driver probe routines.
>>>>
>>>> I propose that we discuss the scenario to find possible solutions to avoid
>>>> defunct drivers.
>>>
>>> This seems to be the expected behaviour to me. The probe failure doesn't
>>> necessarily indicate that the driver is at fault, it means that
>>> something went wrong when associating a particular device with the
>>> driver. It could be that the device is faulty for instance, and that
>>> shouldn't prevent the driver from being registered, especially if
>>> multiple instances of the device can be present in the system, as that
>>> would then prevent any of those instances from working due to one faulty
>>> device.
>>
>> Agreed. This behavior works well in the cases of hardware/device failures
>> that cause probe failure. The case I am seeing is a driver bug that causes
>> probe failure.
> 
> Is there a way for the kernel to determine that the probe failure was
> caused by a buggy driver and not a faulty device ?
> 

That has to be explored.

>>> What other behaviour would you expect ?
>>
>> I am looking to see if we can propagate the error to the interface driver to
>> handle instead of leaving the defunct driver. This isn't an easy problem to
>> solve though. As you mentioned driver probe could fail if device is bad
>> and we want the driver to handle the others.
>>
>> The fact is we will end up with defunct drivers in some cases. If user notices
>> the error they could go clean it up. My main concern is the sysfs interfaces
>> hanging around. The desired behavior would be not leaving defunct drivers with
>> associated sysfs files.
> 
> I don't think the driver is "defunct". It has been loaded successfully,
> and it's fully operational, just not bound to any device.
> 

Not in the case I am debugging. It won't be successfully bound any device.
That is what I meant by defunct. Maybe there is a better word to use.

The driver releases all resources in its probe failure path.

thanks,
-- Shuah