Re: [PATCH v2 01/16] chardev: add helper function to register char devs with a struct device

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Sun, Feb 26, 2017 at 10:21:25AM -0800, Dan Williams wrote:
> > + * cdev_device_add() adds the char device represented by @cdev to the system,
> > + * just as cdev_add does. It then adds @dev to the system using device_add
> > + * The dev_t for the char device will be taken from the struct device which
> > + * needs to be initialized first. This helper function correctly takes a
> > + * reference to the parent device so the parent will not get released until
> > + * all references to the cdev are released.
> > + *
> > + * This function should be used whenever the struct cdev and the
> > + * struct device are members of the same structure whose lifetime is
> > + * managed by the struct device.
> > + */
> 
> Perhaps add a note here that userspace may have invoked file
> operations between cdev_add() and a failing device_add(), so
> additional cleanup beyond put_device() (like mmap invalidation) might
> be needed. That can be a later follow-on patch.

Yes please, that is way too subtle. Suggest:

NOTE: Callers must assume that userspace was able to open the cdev and
can call cdev fops callbacks at any time, even if this function fails.

I would also add a note to cdev_device_del:

NOTE: This guarantees that associated sysfs callbacks are not running
or runnable, however any open cdevs will remain and their fops remain
callable even after this returns.

Since I have seen a lot of confusion on that point as well..

Jason