uio_hv_generic: Fix ring buffer sysfs creation path

On regular bootup, devices get registered to VMBus first, so when uio_hv_generic driver for a particular device type is probed, the device is already initialized and added, so sysfs creation in uio_hv_generic probe works fine. However, when device is removed and brought back, the channel rescinds and device again gets registered to VMBus. However this time, the uio_hv_generic driver is already registered to probe for that device and in this case sysfs creation is tried before the device's kobject gets initialized completely.

Fix this by moving the core logic of sysfs creation for ring buffer, from uio_hv_generic to HyperV's VMBus driver, where rest of the sysfs attributes for the channels are defined. While doing that, make use of attribute groups and macros, instead of creating sysfs directly, to ensure better error handling and code flow.

Problem path:
vmbus_process_offer (new offer comes for the VMBus device)
vmbus_add_channel_work

    vmbus_device_register
      |-> device_register
      |     |...
      |     |-> hv_uio_probe
      |           |...
      |           |-> sysfs_create_bin_file (leads to a warning as
      |                 primary channel's kobject, which is used to
      |                 create sysfs is not yet initialized)
      |-> kset_create_and_add
      |-> vmbus_add_channel_kobj (initialization of primary channel's
                                  kobject happens later)

Above code flow is sequential and the warning is always reproducible in this path.

Fixes 9ab877a6ccf8 ("uio_hv_generic: make ring buffer attribute for primary channel")

Cc [email protected]

Suggested-by Saurabh Sengar <[email protected]>

Suggested-by Michael Kelley <[email protected]>

Signed-off-by Naman Jain <[email protected]>

---
 drivers/hv/hyperv_vmbus.h    |   6 ++
 drivers/hv/vmbus_drv.c       | 110 ++++++++++++++++++++++++++++++++++-
 drivers/uio/uio_hv_generic.c |  33 +++++------
 include/linux/hyperv.h       |   6 ++
 4 files changed, 134 insertions(+), 21 deletions(-)

drivers/hv/vmbus_drv.c +32 -0

+/**
+ * hv_create_ring_sysfs() - create "ring" sysfs entry corresponding to ring buffers for a channel.
+ * @channel: Pointer to vmbus_channel structure
+ * @hv_mmap_ring_buffer: function pointer for initializing the function to be called on mmap of
+ *                       channel's "ring" sysfs node, which is for the ring buffer of that channel.
+ *                       Function pointer is of below type:
+ *                       int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+ *                                                  struct vm_area_struct *vma))
+ *                       This has a pointer to the channel and a pointer to vm_area_struct,
+ *                       used for mmap, as arguments.
+ *
+ * Sysfs node for ring buffer of a channel is created along with other fields, however its
+ * visibility is disabled by default. Sysfs creation needs to be controlled when the use-case
+ * is running.
+ * For example, HV_NIC device is used either by uio_hv_generic or hv_netvsc at any given point of
+ * time, and "ring" sysfs is needed only when uio_hv_generic is bound to that device. To avoid
+ * exposing the ring buffer by default, this function is reponsible to enable visibility of
+ * ring for userspace to use.
+ * Note: Race conditions can happen with userspace and it is not encouraged to create new
+ * use-cases for this. This was added to maintain backward compatibility, while solving
+ * one of the race conditions in uio_hv_generic while creating sysfs.
+ *
+ * Returns 0 on success or error code on failure.
+ */
+int hv_create_ring_sysfs(struct vmbus_channel *channel,
+			 int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+						    struct vm_area_struct *vma))
+{
+	struct kobject *kobj = &channel->kobj;
+	struct vmbus_channel *primary_channel = channel->primary_channel ?
+		channel->primary_channel : channel;
+

drivers/hv/vmbus_drv.c +4 -0

+
+	return sysfs_update_group(kobj, &vmbus_chan_group);
+}
+EXPORT_SYMBOL_GPL(hv_create_ring_sysfs);

From: Naman Jain <[email protected]> Sent: Thursday, March 27, 2025 10:28 PM

Quoted 47 lines

On regular bootup, devices get registered to VMBus first, so when uio_hv_generic driver for a particular device type is probed, the device is already initialized and added, so sysfs creation in uio_hv_generic probe works fine. However, when device is removed and brought back, the channel rescinds and device again gets registered to VMBus. However this time, the uio_hv_generic driver is already registered to probe for that device and in this case sysfs creation is tried before the device's kobject gets initialized completely.

Fix this by moving the core logic of sysfs creation for ring buffer, from uio_hv_generic to HyperV's VMBus driver, where rest of the sysfs attributes for the channels are defined. While doing that, make use of attribute groups and macros, instead of creating sysfs directly, to ensure better error handling and code flow.

Problem path:
vmbus_process_offer (new offer comes for the VMBus device)
vmbus_add_channel_work

     vmbus_device_register
       |-> device_register
       |     |...
       |     |-> hv_uio_probe
       |           |...
       |           |-> sysfs_create_bin_file (leads to a warning as
       |                 primary channel's kobject, which is used to
       |                 create sysfs is not yet initialized)
       |-> kset_create_and_add
       |-> vmbus_add_channel_kobj (initialization of primary channel's
                                   kobject happens later)

Above code flow is sequential and the warning is always reproducible in this path.

Fixes 9ab877a6ccf8 ("uio_hv_generic: make ring buffer attribute for primary channel")

Cc [email protected]

Suggested-by Saurabh Sengar <[email protected]>

Suggested-by Michael Kelley <[email protected]>

Signed-off-by Naman Jain <[email protected]>

---
  drivers/hv/hyperv_vmbus.h    |   6 ++
  drivers/hv/vmbus_drv.c       | 110 ++++++++++++++++++++++++++++++++++-
  drivers/uio/uio_hv_generic.c |  33 +++++------
  include/linux/hyperv.h       |   6 ++
  4 files changed, 134 insertions(+), 21 deletions(-)

[snip]

Quoted 32 lines

drivers/hv/vmbus_drv.c +32 -0

+/**
+ * hv_create_ring_sysfs() - create "ring" sysfs entry corresponding to ring buffers for a channel.
+ * @channel: Pointer to vmbus_channel structure
+ * @hv_mmap_ring_buffer: function pointer for initializing the function to be called on mmap of
+ *                       channel's "ring" sysfs node, which is for the ring buffer of that channel.
+ *                       Function pointer is of below type:
+ *                       int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+ *                                                  struct vm_area_struct *vma))
+ *                       This has a pointer to the channel and a pointer to vm_area_struct,
+ *                       used for mmap, as arguments.
+ *
+ * Sysfs node for ring buffer of a channel is created along with other fields, however its
+ * visibility is disabled by default. Sysfs creation needs to be controlled when the use-case
+ * is running.
+ * For example, HV_NIC device is used either by uio_hv_generic or hv_netvsc at any given point of
+ * time, and "ring" sysfs is needed only when uio_hv_generic is bound to that device. To avoid
+ * exposing the ring buffer by default, this function is reponsible to enable visibility of
+ * ring for userspace to use.
+ * Note: Race conditions can happen with userspace and it is not encouraged to create new
+ * use-cases for this. This was added to maintain backward compatibility, while solving
+ * one of the race conditions in uio_hv_generic while creating sysfs.
+ *
+ * Returns 0 on success or error code on failure.
+ */
+int hv_create_ring_sysfs(struct vmbus_channel *channel,
+			 int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+						    struct vm_area_struct *vma))
+{
+	struct kobject *kobj = &channel->kobj;
+	struct vmbus_channel *primary_channel = channel->primary_channel ?
+		channel->primary_channel : channel;
+

drivers/hv/vmbus_drv.c static const struct kobj_type vmbus_chan_ktype = { · line 1936

Quoted 10 lines

drivers/hv/vmbus_drv.c +10 -0

+	channel->mmap_ring_buffer = hv_mmap_ring_buffer;
+	channel->ring_sysfs_visible = true;
+
+	/*
+	 * Skip updating the sysfs group if the primary channel is not yet initialized and sysfs
+	 * group is not yet created. In those cases, the 'ring' will be created later in
+	 * vmbus_device_register() -> vmbus_add_channel_kobj().
+	 */
+	if  (!primary_channel->device_obj->channels_kset)
+		return 0;

This test doesn't accomplish what you want. It tests if the "channels" directory has been created, but not if the numbered subdirectory for this channel has been created. sysfs_update_group() operates on the numbered subdirectory and could still fail because it hasn't been created yet.

My recommendation is to not try to do a test, and just let sysfs_update_group() fail in that case (and ignore the error).

Michael

Quoted 4 lines

drivers/hv/vmbus_drv.c +4 -0

+
+	return sysfs_update_group(kobj, &vmbus_chan_group);
+}
+EXPORT_SYMBOL_GPL(hv_create_ring_sysfs);

Quoted 117 lines · Michael Kelley

From: Naman Jain <[email protected]> Sent: Thursday, March 27, 2025 10:28 PM

Quoted 47 lines

On regular bootup, devices get registered to VMBus first, so when uio_hv_generic driver for a particular device type is probed, the device is already initialized and added, so sysfs creation in uio_hv_generic probe works fine. However, when device is removed and brought back, the channel rescinds and device again gets registered to VMBus. However this time, the uio_hv_generic driver is already registered to probe for that device and in this case sysfs creation is tried before the device's kobject gets initialized completely.

Fix this by moving the core logic of sysfs creation for ring buffer, from uio_hv_generic to HyperV's VMBus driver, where rest of the sysfs attributes for the channels are defined. While doing that, make use of attribute groups and macros, instead of creating sysfs directly, to ensure better error handling and code flow.

Problem path:
vmbus_process_offer (new offer comes for the VMBus device) vmbus_add_channel_work
     vmbus_device_register

       |-> device_register
       |     |...
       |     |-> hv_uio_probe
       |           |...
       |           |-> sysfs_create_bin_file (leads to a warning as
       |                 primary channel's kobject, which is used to
       |                 create sysfs is not yet initialized)
       |-> kset_create_and_add
       |-> vmbus_add_channel_kobj (initialization of primary channel's

                                   kobject happens later)

Above code flow is sequential and the warning is always reproducible in this path.

Fixes 9ab877a6ccf8 ("uio_hv_generic: make ring buffer attribute for primary channel")

Cc [email protected]

Suggested-by Saurabh Sengar <[email protected]>

Suggested-by Michael Kelley <[email protected]>

Signed-off-by Naman Jain <[email protected]>

---
  drivers/hv/hyperv_vmbus.h    |   6 ++ drivers/hv/vmbus_drv.c       | 110 ++++++++++++++++++++++++++++++++++- drivers/uio/uio_hv_generic.c |  33 +++++------ include/linux/hyperv.h       |   6 ++

  4 files changed, 134 insertions(+), 21 deletions(-)

[snip]

Quoted 50 lines

+/**
+ * hv_create_ring_sysfs() - create "ring" sysfs entry corresponding to ring buffers for a channel.
+ * @channel: Pointer to vmbus_channel structure
+ * @hv_mmap_ring_buffer: function pointer for initializing the function to be called on mmap of
+ *                       channel's "ring" sysfs node, which is for the ring buffer of that channel.
+ *                       Function pointer is of below type:
+ *                       int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+ *                                                  struct vm_area_struct *vma))
+ *                       This has a pointer to the channel and a pointer to vm_area_struct,
+ *                       used for mmap, as arguments.
+ *
+ * Sysfs node for ring buffer of a channel is created along with other fields, however its
+ * visibility is disabled by default. Sysfs creation needs to be controlled when the use-case
+ * is running.
+ * For example, HV_NIC device is used either by uio_hv_generic or hv_netvsc at any given point of
+ * time, and "ring" sysfs is needed only when uio_hv_generic is bound to that device. To avoid
+ * exposing the ring buffer by default, this function is reponsible to enable visibility of
+ * ring for userspace to use.
+ * Note: Race conditions can happen with userspace and it is not encouraged to create new
+ * use-cases for this. This was added to maintain backward compatibility, while solving
+ * one of the race conditions in uio_hv_generic while creating sysfs.
+ *
+ * Returns 0 on success or error code on failure.
+ */

drivers/hv/vmbus_drv.c +13 -0

+int hv_create_ring_sysfs(struct vmbus_channel *channel,
+             int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+                            struct vm_area_struct *vma))
+{
+    struct kobject *kobj = &channel->kobj;
+    struct vmbus_channel *primary_channel = channel->primary_channel ?
+        channel->primary_channel : channel;
+
+    channel->mmap_ring_buffer = hv_mmap_ring_buffer;
+    channel->ring_sysfs_visible = true;
+
+    /*
+     * Skip updating the sysfs group if the primary channel is not 

Quoted 7 lines

yet initialized and sysfs
+     * group is not yet created. In those cases, the 'ring' will be created later in
+     * vmbus_device_register() -> vmbus_add_channel_kobj(). +     */
+    if  (!primary_channel->device_obj->channels_kset)
+        return 0;

This test doesn't accomplish what you want. It tests if the "channels" directory has been created, but not if the numbered subdirectory for this channel has been created. sysfs_update_group() operates on the numbered subdirectory and could still fail because it hasn't been created yet.

My recommendation is to not try to do a test, and just let sysfs_update_group() fail in that case (and ignore the error).

Quoted 2 lines

Michael

Thanks Michael. Will remove it.

Regards,
Naman

Quoted 5 lines

Quoted 5 lines

drivers/hv/vmbus_drv.c +4 -0

+
+    return sysfs_update_group(kobj, &vmbus_chan_group);
+}
+EXPORT_SYMBOL_GPL(hv_create_ring_sysfs);