Re: Regarding bug in drivers/edac/x38_edac.c

Re: Regarding bug in drivers/edac/x38_edac.c

[Borislav Petkov]

On Sun, Aug 16, 2020 at 12:05:57PM +0530, Madhuparna Bhowmik wrote:
> 2. Even if we ignore the 1st point above and probe is called after init
> finishes,

AFAIR and if you don't have any async probing (which I don't believe for
this driver or for any other EDAC driver), the ->probe() function gets
called during pci_register_driver(). From the docs:

Documentation/PCI/pci.rst:

"pci_register_driver() call
==========================

...

Once added, the driver probe routine will be invoked for any unclaimed
PCI devices listed in its (newly updated) pci_ids list."

You could verify that by adding some debugging printks to a driver's
probe function.

I believe it ends up somewhere in the driver core, looks like in

really_probe():

        if (dev->bus->probe) {
                ret = dev->bus->probe(dev);

in drivers/base/dd.c but I could be mistaken.

> In general, calling the probe function from init itself is a bit strange.

Yes, that is ugly - the init function tries what the probe function has
already tried, again.

But I don't think either 1. or 2. of yours happens - it is actually
clumsy but the mci_pdev is simply a check to verify whether the
->probe() succeeded and try it again.

Now, if you have that hardware, you could verify all that but I think
the reality is, I don't think anyone uses that hardware anymore and we
will remove them at some point in the future so you might wanna look at
some other drivers which *actually* are still in use.

:-)

HTH.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Regarding bug in drivers/edac/x38_edac.c

[Madhuparna Bhowmik]

On Sun, Aug 16, 2020 at 09:06:21AM +0200, Borislav Petkov wrote:
> On Sun, Aug 16, 2020 at 12:05:57PM +0530, Madhuparna Bhowmik wrote:
> > 2. Even if we ignore the 1st point above and probe is called after init
> > finishes,
> 
> AFAIR and if you don't have any async probing (which I don't believe for
> this driver or for any other EDAC driver), the ->probe() function gets
> called during pci_register_driver(). From the docs:
> 
> Documentation/PCI/pci.rst:
> 
> "pci_register_driver() call
> ==========================
> 
> ...
> 
> Once added, the driver probe routine will be invoked for any unclaimed
> PCI devices listed in its (newly updated) pci_ids list."
> 
> You could verify that by adding some debugging printks to a driver's
> probe function.
> 
> I believe it ends up somewhere in the driver core, looks like in
> 
> really_probe():
> 
>         if (dev->bus->probe) {
>                 ret = dev->bus->probe(dev);
> 
> in drivers/base/dd.c but I could be mistaken.
> 
> > In general, calling the probe function from init itself is a bit strange.
> 
> Yes, that is ugly - the init function tries what the probe function has
> already tried, again.
> 
> But I don't think either 1. or 2. of yours happens - it is actually
> clumsy but the mci_pdev is simply a check to verify whether the
> ->probe() succeeded and try it again.
>
Thanks a lot for clarifying this.

> Now, if you have that hardware, you could verify all that but I think
> the reality is, I don't think anyone uses that hardware anymore and we
> will remove them at some point in the future so you might wanna look at
> some other drivers which *actually* are still in use.
>
Alright, makes sense.

Regards,
Madhuparna

> :-)
> 
> HTH.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette