On Mon, Nov 11, 2019 at 04:45:07PM +0800, lantianyu1986@xxxxxxxxx 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
