Re: [PATCH] VFIO/VMBUS: Add VFIO VMBUS driver support

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Nov 11, 2019 at 04:45:07PM +0800, lantianyu1986@gmail.com wrote:
> +#define DRIVER_VERSION	"0.0.1"

Never a need for DRIVER_VERSION as your driver just becomes part of the
main kernel tree, so please drop this.  We keep trying to delete these
types of numbers and they keep coming back...

> +static void
> +vfio_vmbus_new_channel(struct vmbus_channel *new_sc)
> +{
> +	struct hv_device *hv_dev = new_sc->primary_channel->device_obj;
> +	struct device *device = &hv_dev->device;
> +	int ret;
> +
> +	/* Create host communication ring */
> +	ret = vmbus_open(new_sc, HV_RING_SIZE, HV_RING_SIZE, NULL, 0,
> +			 vfio_vmbus_channel_cb, new_sc);
> +	if (ret) {
> +		dev_err(device, "vmbus_open subchannel failed: %d\n", ret);
> +		return;
> +	}
> +
> +	/* Disable interrupts on sub channel */
> +	new_sc->inbound.ring_buffer->interrupt_mask = 1;
> +	set_channel_read_mode(new_sc, HV_CALL_ISR);
> +
> +	ret = sysfs_create_bin_file(&new_sc->kobj, &ring_buffer_bin_attr);

No documentation on this new sysfs file?

And by creating it here, userspace is not notified of it, so tools will
not see it :(

> +	if (ret)
> +		dev_notice(&hv_dev->device,
> +			   "sysfs create ring bin file failed; %d\n", ret);

Doesn't the call spit out an error if something happens?

> +	ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
> +	if (ret)
> +		dev_notice(&dev->device,
> +			   "sysfs create ring bin file failed; %d\n", ret);
> +

Again, don't create sysfs files on your own, the bus code should be
doing this for you automatically and in a way that is race-free.

thanks,

greg k-h