Re: [PATCH v2 1/5] driver core: add a faux bus for use when a simple device/bus is needed

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 12:44:03PM +0100, Danilo Krummrich wrote:
> On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote:
> > Many drivers abuse the platform driver/bus system as it provides a
> > simple way to create and bind a device to a driver-specific set of
> > probe/release functions.  Instead of doing that, and wasting all of the
> > memory associated with a platform device, here is a "faux" bus that
> > can be used instead.
> > 
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  v2: - renamed bus and root device to just "faux" thanks to Thomas
> >      - removed the one-driver-per-device and now just have one driver
> >        entirely thanks to Danilo
> >      - kerneldoc fixups and additions and string handling bounds checks
> >        hanks to Andy
> >      - coding style fix thanks to Jonathan
> >      - tested that the destroy path actually works
> > 
> >  drivers/base/Makefile       |   2 +-
> >  drivers/base/base.h         |   1 +
> >  drivers/base/faux.c         | 196 ++++++++++++++++++++++++++++++++++++
> >  drivers/base/init.c         |   1 +
> >  include/linux/device/faux.h |  31 ++++++
> >  5 files changed, 230 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/base/faux.c
> >  create mode 100644 include/linux/device/faux.h
> 
> I really like it, it's as simply as it can be.
> 
> Please find one nit below, otherwise
> 
> Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> 
> > 
> > +/**
> > + * faux_device_destroy - destroy a faux device
> > + * @faux_dev: faux device to destroy
> > + *
> > + * Unregister and free all memory associated with a faux device that was
> > + * previously created with a call to faux_device_create().
> 
> Can we really claim that this frees all memory? Someone can still have a
> reference to the underlying struct device, right?

That "someone" is the person that had the original device pointer passed
to it, so if that person then calls faux_device_destroy(), yes, that
should all be properly cleaned up.

But even if it isn't, the device is destroyed and gone from sysfs, and
whenever that final final put_device() is called, the memory will then
be freed by the driver core itself.

So all should be ok unless I'm missing something in some codepath here.

> > + */
> > +void faux_device_destroy(struct faux_device *faux_dev)
> > +{
> > +	struct device *dev = &faux_dev->dev;
> > +
> > +	if (IS_ERR_OR_NULL(faux_dev))
> > +		return;
> > +
> > +	device_del(dev);
> > +
> > +	/* The final put_device() will clean up the driver we created for this device. */
> > +	put_device(dev);
> 
> Same here, how do we know it's the final one? I also think the "clean up the
> driver we created for this device" part isn't true any longer.

Oops, missed that comment, will go fix that.

And same response as above, it should all be cleaned up as best as we
can by now, any other external references is up to them be dropped by
that owner.

thanks for the review,

greg k-h