On Mon, Feb 10, 2020 at 11:56:06AM +0800, Jason Wang wrote:
> +/**
> + * vdpa_register_device - register a vDPA device
> + * Callers must have a succeed call of vdpa_init_device() before.
> + * @vdev: the vdpa device to be registered to vDPA bus
> + *
> + * Returns an error when fail to add to vDPA bus
> + */
> +int vdpa_register_device(struct vdpa_device *vdev)
> +{
> + int err = device_add(&vdev->dev);
> +
> + if (err) {
> + put_device(&vdev->dev);
> + ida_simple_remove(&vdpa_index_ida, vdev->index);
> + }
This is a very dangerous construction, I've seen it lead to driver
bugs. Better to require the driver to always do the put_device on
error unwind
The ida_simple_remove should probably be part of the class release
function to make everything work right
> +/**
> + * vdpa_unregister_driver - unregister a vDPA device driver
> + * @drv: the vdpa device driver to be unregistered
> + */
> +void vdpa_unregister_driver(struct vdpa_driver *drv)
> +{
> + driver_unregister(&drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_unregister_driver);
> +
> +static int vdpa_init(void)
> +{
> + if (bus_register(&vdpa_bus) != 0)
> + panic("virtio bus registration failed");
> + return 0;
> +}
Linus will tell you not to kill the kernel - return the error code and
propagate it up to the module init function.
> +/**
> + * vDPA device - representation of a vDPA device
> + * @dev: underlying device
> + * @dma_dev: the actual device that is performing DMA
> + * @config: the configuration ops for this device.
> + * @index: device index
> + */
> +struct vdpa_device {
> + struct device dev;
> + struct device *dma_dev;
> + const struct vdpa_config_ops *config;
> + int index;
unsigned values shuld be unsigned int
Jason
