Re: [PATCH 03/10] driver core: Flow the return code from ->probe() through to sysfs bind

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 08, 2021 at 08:47:19AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jun 07, 2021 at 09:55:45PM -0300, Jason Gunthorpe wrote:
> > Currently really_probe() returns 1 on success and 0 if the probe() call
> > fails. This return code arrangement is designed to be useful for
> > __device_attach_driver() which is walking the device list and trying every
> > driver. 0 means to keep trying.
> > 
> > However, it is not useful for the other places that call through to
> > really_probe() that do actually want to see the probe() return code.
> > 
> > For instance bind_store() would be better to return the actual error code
> > from the driver's probe method, not discarding it and returning -ENODEV.
> 
> Why does that matter?  Why does it need to know this?

Proper return code to userspace are important. Knowing why the driver
probe() fails is certainly helpful for debugging. Is there are reason
to hide them? I think this is an improvement for sysfs bind.

Why this series needs it is because mdev has fixed sys uAPI at this point
that requires carring the return code from device driver probe() to
a mdev sysfs function.

> > -static int really_probe(struct device *dev, struct device_driver *drv)
> > +enum {
> > +	/* Set on output if the -ERR has come from a probe() function */
> > +	PROBEF_DRV_FAILED = 1 << 0,
> > +};
> > +
> > +static int really_probe(struct device *dev, struct device_driver *drv,
> > +			unsigned int *flags)
> 
> Ugh, no, please no functions with random "flags" in them, that way lies
> madness and unmaintainable code for decades to come.

The alternative to this something like this:

static int really_probe(struct device *dev, struct device_driver *drv,
			int *probe_err)

And since we still need the 'do not probe defer' in next patches then
it would have to be this:

static int really_probe(struct device *dev, struct device_driver *drv,
			int *probe_err, bool allow_probe_defer)

And the two new arguments flowed up through several function call
sites.

Do you prefer one of these more?

For your other question PROBEF_ means 'probe flag'.

Jason



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux