On Tue, Jun 08, 2021 at 03:16:51PM +0200, Greg Kroah-Hartman wrote: > On Tue, Jun 08, 2021 at 09:30:23AM -0300, Jason Gunthorpe wrote: > > 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. > > What is mdev and what userspace tool requires such a userspace api to > depend on this? Were you able to see the cover letter? mdev is part of vfio, it is very ugly, but it has a userspace ecosystem now. > Tools doing manual bind/unbind from userspace are crazy, it's always > been a "look at this neat hack!" type of thing. To do it "right" you > should always do it correctly within the kernel. Which is what the later patches do for mdev, but the dual operation of creating the struct device and connecting to its driver have a historical requirement to return the error code from driver probe to the userspace. v1 of this just did a hacky approach inside mdev to achieve this but Dan and CH thought it would be more widely useful so asked for this series to allow the driver core to handle it. This did turn out fairly nice so I tend to agree - but returning the error code is important. > Random boolean flags as parameters are just as bad. Sure, but the function needs different behavior depending on the call site. An alternative is to rework all the call chains to somehow embed the difference directly and I don't have a clear vision how to do that that is any nicer than this. Jason
