[PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Naman Jain]

Hi,
This patch series aims to address the sysfs creation issue for the ring
buffer by reorganizing the code. Additionally, it updates the ring sysfs
size to accurately reflect the actual ring buffer size, rather than a
fixed static value.

PFB change logs:

Changes since v5:
https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
* Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
  commit msg of both patches.
* Missed to remove check for "primary_channel->device_obj->channels_kset" in
  hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
  now. 
* Changed type for declaring bin_attrs due to changes introduced by
  commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
  merged recently. Did not use bin_attrs_new since another change is in
  the queue to change usage of bin_attrs_new to bin_attrs
  (sysfs: finalize the constification of 'struct bin_attribute').

Changes since v4:
https://lore.kernel.org/all/20250410060847.82407-1-namjain@linux.microsoft.com/
* Added Reviewed-By and Tested-By tags from Michael.
* Fixed syntax by removing extra space (addressed Greg's comment)
* Rebased to latest linux-next tip

Changes since v3:
https://lore.kernel.org/all/20250328052745.1417-1-namjain@linux.microsoft.com/
* Addressed Michael's comments regarding handling of return value of
sysfs_update_group in uio_hv_generic.

Changes since v2:
https://lore.kernel.org/all/20250318061558.3294-1-namjain@linux.microsoft.com/
Addressed Greg's comments:
* Split the original patch into two.
* Updated the commit message to explain the problem scenario.
* Added comments for new APIs in the kerneldoc format.
* Highlighted potential race conditions and explained why sysfs should not be created in the
  driver probe.

* Made minor changes to how the sysfs_update_group return value is handled.

Changes since v1:
https://lore.kernel.org/all/20250225052001.2225-1-namjain@linux.microsoft.com/
* Fixed race condition in setting channel->mmap_ring_buffer by
  introducing a new variable for visibility of sysfs (addressed Greg's
  comments)
* Used binary attribute fields instead of regular ones for initializing attribute_group.
* Make size of ring sysfs dynamic based on actual ring buffer's size.
* Preferred to keep mmap function in uio_hv_generic to give more control over ring's
  mmap functionality, since this is specific to uio_hv_generic driver.
* Remove spurious warning during sysfs creation in uio_hv_generic probe.
* Added comments in a couple of places.

Changes since RFC patch:
https://lore.kernel.org/all/20250214064351.8994-1-namjain@linux.microsoft.com/
* Different approach to solve the problem is proposed (credits to
  Michael Kelley).
* Core logic for sysfs creation moved out of uio_hv_generic, to VMBus
  drivers where rest of the sysfs attributes for a VMBus channel
  are defined. (addressed Greg's comments)
* Used attribute groups instead of sysfs_create* functions, and bundled
  ring attribute with other attributes for the channel sysfs.  

Error logs:

[   35.574120] ------------[ cut here ]------------
[   35.574122] WARNING: CPU: 0 PID: 10 at fs/sysfs/file.c:591 sysfs_create_bin_file+0x81/0x90
[   35.574168] Workqueue: hv_pri_chan vmbus_add_channel_work
[   35.574172] RIP: 0010:sysfs_create_bin_file+0x81/0x90
[   35.574197] Call Trace:
[   35.574199]  <TASK>
[   35.574200]  ? show_regs+0x69/0x80
[   35.574217]  ? __warn+0x8d/0x130
[   35.574220]  ? sysfs_create_bin_file+0x81/0x90
[   35.574222]  ? report_bug+0x182/0x190
[   35.574225]  ? handle_bug+0x5b/0x90
[   35.574244]  ? exc_invalid_op+0x19/0x70
[   35.574247]  ? asm_exc_invalid_op+0x1b/0x20
[   35.574252]  ? sysfs_create_bin_file+0x81/0x90
[   35.574255]  hv_uio_probe+0x1e7/0x410 [uio_hv_generic]
[   35.574271]  vmbus_probe+0x3b/0x90
[   35.574275]  really_probe+0xf4/0x3b0
[   35.574279]  __driver_probe_device+0x8a/0x170
[   35.574282]  driver_probe_device+0x23/0xc0
[   35.574285]  __device_attach_driver+0xb5/0x140
[   35.574288]  ? __pfx___device_attach_driver+0x10/0x10
[   35.574291]  bus_for_each_drv+0x86/0xe0
[   35.574294]  __device_attach+0xc1/0x200
[   35.574297]  device_initial_probe+0x13/0x20
[   35.574315]  bus_probe_device+0x99/0xa0
[   35.574318]  device_add+0x647/0x870
[   35.574320]  ? hrtimer_init+0x28/0x70
[   35.574323]  device_register+0x1b/0x30
[   35.574326]  vmbus_device_register+0x83/0x130
[   35.574328]  vmbus_add_channel_work+0x135/0x1a0
[   35.574331]  process_one_work+0x177/0x340
[   35.574348]  worker_thread+0x2b2/0x3c0
[   35.574350]  kthread+0xe3/0x1f0
[   35.574353]  ? __pfx_worker_thread+0x10/0x10
[   35.574356]  ? __pfx_kthread+0x10/0x10


Naman Jain (2):
  uio_hv_generic: Fix sysfs creation path for ring buffer
  Drivers: hv: Make the sysfs node size for the ring buffer dynamic

 drivers/hv/hyperv_vmbus.h    |   6 ++
 drivers/hv/vmbus_drv.c       | 119 ++++++++++++++++++++++++++++++++++-
 drivers/uio/uio_hv_generic.c |  39 +++++-------
 include/linux/hyperv.h       |   6 ++
 4 files changed, 147 insertions(+), 23 deletions(-)


base-commit: 2c9c612abeb38aab0e87d48496de6fd6daafb00b
-- 
2.34.1

[PATCH v6 1/2] uio_hv_generic: Fix sysfs creation path for ring buffer

[Naman Jain]

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
hv_uio_probe() works fine. However, when the device is removed
and brought back, the channel gets rescinded and the 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 of ring buffer,
from uio_hv_generic to HyperV's VMBus driver, where the 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.

Problematic path:
vmbus_process_offer (A 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
      |                 the primary channel's kobject, which is used to
      |                 create the sysfs file, is not yet initialized)
      |-> kset_create_and_add
      |-> vmbus_add_channel_kobj (initialization of the 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: stable@kernel.org
Suggested-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Suggested-by: Michael Kelley <mhklinux@outlook.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
 drivers/hv/hyperv_vmbus.h    |   6 +++
 drivers/hv/vmbus_drv.c       | 100 ++++++++++++++++++++++++++++++++++-
 drivers/uio/uio_hv_generic.c |  39 ++++++--------
 include/linux/hyperv.h       |   6 +++
 4 files changed, 128 insertions(+), 23 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 29780f3a7478..0b450e53161e 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -477,4 +477,10 @@ static inline int hv_debug_add_dev_dir(struct hv_device *dev)
 
 #endif /* CONFIG_HYPERV_TESTING */
 
+/* Create and remove sysfs entry for memory mapped ring buffers for a channel */
+int hv_create_ring_sysfs(struct vmbus_channel *channel,
+			 int (*hv_mmap_ring_buffer)(struct vmbus_channel *channel,
+						    struct vm_area_struct *vma));
+int hv_remove_ring_sysfs(struct vmbus_channel *channel);
+
 #endif /* _HYPERV_VMBUS_H */
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8d3cff42bdbb..e2827079da89 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1802,6 +1802,27 @@ static ssize_t subchannel_id_show(struct vmbus_channel *channel,
 }
 static VMBUS_CHAN_ATTR_RO(subchannel_id);
 
+static int hv_mmap_ring_buffer_wrapper(struct file *filp, struct kobject *kobj,
+				       const struct bin_attribute *attr,
+				       struct vm_area_struct *vma)
+{
+	struct vmbus_channel *channel = container_of(kobj, struct vmbus_channel, kobj);
+
+	/*
+	 * hv_(create|remove)_ring_sysfs implementation ensures that mmap_ring_buffer
+	 * is not NULL.
+	 */
+	return channel->mmap_ring_buffer(channel, vma);
+}
+
+static struct bin_attribute chan_attr_ring_buffer = {
+	.attr = {
+		.name = "ring",
+		.mode = 0600,
+	},
+	.size = 2 * SZ_2M,
+	.mmap = hv_mmap_ring_buffer_wrapper,
+};
 static struct attribute *vmbus_chan_attrs[] = {
 	&chan_attr_out_mask.attr,
 	&chan_attr_in_mask.attr,
@@ -1821,6 +1842,11 @@ static struct attribute *vmbus_chan_attrs[] = {
 	NULL
 };
 
+static const struct bin_attribute *const vmbus_chan_bin_attrs[] = {
+	&chan_attr_ring_buffer,
+	NULL
+};
+
 /*
  * Channel-level attribute_group callback function. Returns the permission for
  * each attribute, and returns 0 if an attribute is not visible.
@@ -1841,9 +1867,24 @@ static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
 	return attr->mode;
 }
 
+static umode_t vmbus_chan_bin_attr_is_visible(struct kobject *kobj,
+					      const struct bin_attribute *attr, int idx)
+{
+	const struct vmbus_channel *channel =
+		container_of(kobj, struct vmbus_channel, kobj);
+
+	/* Hide ring attribute if channel's ring_sysfs_visible is set to false */
+	if (attr ==  &chan_attr_ring_buffer && !channel->ring_sysfs_visible)
+		return 0;
+
+	return attr->attr.mode;
+}
+
 static const struct attribute_group vmbus_chan_group = {
 	.attrs = vmbus_chan_attrs,
-	.is_visible = vmbus_chan_attr_is_visible
+	.bin_attrs = vmbus_chan_bin_attrs,
+	.is_visible = vmbus_chan_attr_is_visible,
+	.is_bin_visible = vmbus_chan_bin_attr_is_visible,
 };
 
 static const struct kobj_type vmbus_chan_ktype = {
@@ -1851,6 +1892,63 @@ static const struct kobj_type vmbus_chan_ktype = {
 	.release = vmbus_chan_release,
 };
 
+/**
+ * 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;
+
+	channel->mmap_ring_buffer = hv_mmap_ring_buffer;
+	channel->ring_sysfs_visible = true;
+
+	return sysfs_update_group(kobj, &vmbus_chan_group);
+}
+EXPORT_SYMBOL_GPL(hv_create_ring_sysfs);
+
+/**
+ * hv_remove_ring_sysfs() - remove ring sysfs entry corresponding to ring buffers for a channel.
+ * @channel: Pointer to vmbus_channel structure
+ *
+ * Hide "ring" sysfs for a channel by changing its is_visible attribute and updating sysfs group.
+ *
+ * Returns 0 on success or error code on failure.
+ */
+int hv_remove_ring_sysfs(struct vmbus_channel *channel)
+{
+	struct kobject *kobj = &channel->kobj;
+	int ret;
+
+	channel->ring_sysfs_visible = false;
+	ret = sysfs_update_group(kobj, &vmbus_chan_group);
+	channel->mmap_ring_buffer = NULL;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hv_remove_ring_sysfs);
+
 /*
  * vmbus_add_channel_kobj - setup a sub-directory under device/channels
  */
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 5c017860d380..b75eae624e8a 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -151,15 +151,12 @@ static void hv_uio_rescind(struct vmbus_channel *channel)
 	vmbus_device_unregister(channel->device_obj);
 }
 
-/* Sysfs API to allow mmap of the ring buffers
+/* Function used for mmap of ring buffer sysfs interface.
  * The ring buffer is allocated as contiguous memory by vmbus_open
  */
-static int hv_uio_ring_mmap(struct file *filp, struct kobject *kobj,
-			    const struct bin_attribute *attr,
-			    struct vm_area_struct *vma)
+static int
+hv_uio_ring_mmap(struct vmbus_channel *channel, struct vm_area_struct *vma)
 {
-	struct vmbus_channel *channel
-		= container_of(kobj, struct vmbus_channel, kobj);
 	void *ring_buffer = page_address(channel->ringbuffer_page);
 
 	if (channel->state != CHANNEL_OPENED_STATE)
@@ -169,15 +166,6 @@ static int hv_uio_ring_mmap(struct file *filp, struct kobject *kobj,
 			       channel->ringbuffer_pagecount << PAGE_SHIFT);
 }
 
-static const struct bin_attribute ring_buffer_bin_attr = {
-	.attr = {
-		.name = "ring",
-		.mode = 0600,
-	},
-	.size = 2 * SZ_2M,
-	.mmap = hv_uio_ring_mmap,
-};
-
 /* Callback from VMBUS subsystem when new channel created. */
 static void
 hv_uio_new_channel(struct vmbus_channel *new_sc)
@@ -198,8 +186,7 @@ hv_uio_new_channel(struct vmbus_channel *new_sc)
 	/* 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);
+	ret = hv_create_ring_sysfs(new_sc, hv_uio_ring_mmap);
 	if (ret) {
 		dev_err(device, "sysfs create ring bin file failed; %d\n", ret);
 		vmbus_close(new_sc);
@@ -370,10 +357,18 @@ hv_uio_probe(struct hv_device *dev,
 		goto fail_close;
 	}
 
-	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);
+	/*
+	 * This internally calls sysfs_update_group, which returns a non-zero value if it executes
+	 * before sysfs_create_group. This is expected as the 'ring' will be created later in
+	 * vmbus_device_register() -> vmbus_add_channel_kobj(). Thus, no need to check the return
+	 * value and print warning.
+	 *
+	 * Creating/exposing sysfs in driver probe is not encouraged as it can lead to race
+	 * conditions with userspace. For backward compatibility, "ring" sysfs could not be removed
+	 * or decoupled from uio_hv_generic probe. Userspace programs can make use of inotify
+	 * APIs to make sure that ring is created.
+	 */
+	hv_create_ring_sysfs(channel, hv_uio_ring_mmap);
 
 	hv_set_drvdata(dev, pdata);
 
@@ -395,7 +390,7 @@ hv_uio_remove(struct hv_device *dev)
 	if (!pdata)
 		return;
 
-	sysfs_remove_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr);
+	hv_remove_ring_sysfs(dev->channel);
 	uio_unregister_device(&pdata->info);
 	hv_uio_cleanup(dev, pdata);
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 675959fb97ba..d6ffe01962c2 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1002,6 +1002,12 @@ struct vmbus_channel {
 
 	/* The max size of a packet on this channel */
 	u32 max_pkt_size;
+
+	/* function to mmap ring buffer memory to the channel's sysfs ring attribute */
+	int (*mmap_ring_buffer)(struct vmbus_channel *channel, struct vm_area_struct *vma);
+
+	/* boolean to control visibility of sysfs for ring buffer */
+	bool ring_sysfs_visible;
 };
 
 #define lock_requestor(channel, flags)					\
-- 
2.34.1

[PATCH v6 2/2] Drivers: hv: Make the sysfs node size for the ring buffer dynamic

[Naman Jain]

The ring buffer size varies across VMBus channels. The size of sysfs
node for the ring buffer is currently hardcoded to 4 MB. Userspace
clients either use fstat() or hardcode this size for doing mmap().
To address this, make the sysfs node size dynamic to reflect the
actual ring buffer size for each channel. This will ensure that
fstat() on ring sysfs node always returns the correct size of
ring buffer.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
 drivers/hv/vmbus_drv.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e2827079da89..15b085df95c8 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1820,7 +1820,6 @@ static struct bin_attribute chan_attr_ring_buffer = {
 		.name = "ring",
 		.mode = 0600,
 	},
-	.size = 2 * SZ_2M,
 	.mmap = hv_mmap_ring_buffer_wrapper,
 };
 static struct attribute *vmbus_chan_attrs[] = {
@@ -1880,11 +1879,21 @@ static umode_t vmbus_chan_bin_attr_is_visible(struct kobject *kobj,
 	return attr->attr.mode;
 }
 
+static size_t vmbus_chan_bin_size(struct kobject *kobj,
+				  const struct bin_attribute *bin_attr, int a)
+{
+	const struct vmbus_channel *channel =
+		container_of(kobj, struct vmbus_channel, kobj);
+
+	return channel->ringbuffer_pagecount << PAGE_SHIFT;
+}
+
 static const struct attribute_group vmbus_chan_group = {
 	.attrs = vmbus_chan_attrs,
 	.bin_attrs = vmbus_chan_bin_attrs,
 	.is_visible = vmbus_chan_attr_is_visible,
 	.is_bin_visible = vmbus_chan_bin_attr_is_visible,
+	.bin_size = vmbus_chan_bin_size,
 };
 
 static const struct kobj_type vmbus_chan_ktype = {
-- 
2.34.1

RE: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 23, 2025 10:35 PM
> 
> Hi,
> This patch series aims to address the sysfs creation issue for the ring
> buffer by reorganizing the code. Additionally, it updates the ring sysfs
> size to accurately reflect the actual ring buffer size, rather than a
> fixed static value.
> 
> PFB change logs:
> 
> Changes since v5:
> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>   commit msg of both patches.
> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>   hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>   now.

Ah, OK :-) I thought you had decided to leave the test in, and I wasn't going to
argue further, as it didn't hurt anything. But the test is superfluous, so the
code is better without it. It won't mislead a future someone into thinking
that it solves a synchronization problem.

Michael

> * Changed type for declaring bin_attrs due to changes introduced by
>   commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>   merged recently. Did not use bin_attrs_new since another change is in
>   the queue to change usage of bin_attrs_new to bin_attrs
>   (sysfs: finalize the constification of 'struct bin_attribute').
> 

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Naman Jain]

On 4/24/2025 7:18 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, April 23, 2025 10:35 PM
>>
>> Hi,
>> This patch series aims to address the sysfs creation issue for the ring
>> buffer by reorganizing the code. Additionally, it updates the ring sysfs
>> size to accurately reflect the actual ring buffer size, rather than a
>> fixed static value.
>>
>> PFB change logs:
>>
>> Changes since v5:
>> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
>> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>>    commit msg of both patches.
>> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>>    hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>>    now.
> 
> Ah, OK :-) I thought you had decided to leave the test in, and I wasn't going to
> argue further, as it didn't hurt anything. But the test is superfluous, so the
> code is better without it. It won't mislead a future someone into thinking
> that it solves a synchronization problem.
> 
> Michael

I agree :D

Regards,
Naman

> 
>> * Changed type for declaring bin_attrs due to changes introduced by
>>    commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>>    merged recently. Did not use bin_attrs_new since another change is in
>>    the queue to change usage of bin_attrs_new to bin_attrs
>>    (sysfs: finalize the constification of 'struct bin_attribute').
>>

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Greg Kroah-Hartman]

On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
> Hi,
> This patch series aims to address the sysfs creation issue for the ring
> buffer by reorganizing the code. Additionally, it updates the ring sysfs
> size to accurately reflect the actual ring buffer size, rather than a
> fixed static value.
> 
> PFB change logs:
> 
> Changes since v5:
> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>   commit msg of both patches.
> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>   hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>   now. 
> * Changed type for declaring bin_attrs due to changes introduced by
>   commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>   merged recently. Did not use bin_attrs_new since another change is in
>   the queue to change usage of bin_attrs_new to bin_attrs
>   (sysfs: finalize the constification of 'struct bin_attribute').

Please fix up to apply cleanly without build warnings:

drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
 1893 |         .bin_attrs = vmbus_chan_bin_attrs,
      |                      ^~~~~~~~~~~~~~~~~~~~
1 error generated.

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Naman Jain]

On 4/25/2025 7:30 PM, Greg Kroah-Hartman wrote:
> On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
>> Hi,
>> This patch series aims to address the sysfs creation issue for the ring
>> buffer by reorganizing the code. Additionally, it updates the ring sysfs
>> size to accurately reflect the actual ring buffer size, rather than a
>> fixed static value.
>>
>> PFB change logs:
>>
>> Changes since v5:
>> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
>> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>>    commit msg of both patches.
>> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>>    hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>>    now.
>> * Changed type for declaring bin_attrs due to changes introduced by
>>    commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>>    merged recently. Did not use bin_attrs_new since another change is in
>>    the queue to change usage of bin_attrs_new to bin_attrs
>>    (sysfs: finalize the constification of 'struct bin_attribute').
> 
> Please fix up to apply cleanly without build warnings:
> 
> drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>   1893 |         .bin_attrs = vmbus_chan_bin_attrs,
>        |                      ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.

Hi Greg,
I tried reproducing this error but could not see it. Should I rebase the 
change to some other tree or use some specific config option, gcc 
version, compilation flag etc.?

I tried the following:
* Rebased to latest linux-next tip with below base commit:
393d0c54cae31317deaa9043320c5fd9454deabc
* Regular compilation with gcc: make -j8
* extra flags:
   make -j8  EXTRA_CFLAGS="-Wall -O2"
   make -j8 
EXTRA_CFLAGS="-Wincompatible-pointer-types-discards-qualifiers -Werror"
* Tried gcc 11.4, 13.3
* Tried clang/LLVM with version 18.1.3 : make LLVM=1



BTW I had to edit the type for bin_attrs as this change got merged recently:
9bec944506fa ("sysfs: constify attribute_group::bin_attrs")

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 576b8b3c60af..f418aae4f113 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -107,7 +107,7 @@ struct attribute_group {
                                             int);
         struct attribute        **attrs;
         union {
-               struct bin_attribute            **bin_attrs;
+               const struct bin_attribute      *const *bin_attrs;
                 const struct bin_attribute      *const *bin_attrs_new;
         };
  };


Regards,
Naman

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Greg Kroah-Hartman]

On Mon, Apr 28, 2025 at 02:37:22PM +0530, Naman Jain wrote:
> 
> 
> On 4/25/2025 7:30 PM, Greg Kroah-Hartman wrote:
> > On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
> > > Hi,
> > > This patch series aims to address the sysfs creation issue for the ring
> > > buffer by reorganizing the code. Additionally, it updates the ring sysfs
> > > size to accurately reflect the actual ring buffer size, rather than a
> > > fixed static value.
> > > 
> > > PFB change logs:
> > > 
> > > Changes since v5:
> > > https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
> > > * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
> > >    commit msg of both patches.
> > > * Missed to remove check for "primary_channel->device_obj->channels_kset" in
> > >    hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
> > >    now.
> > > * Changed type for declaring bin_attrs due to changes introduced by
> > >    commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
> > >    merged recently. Did not use bin_attrs_new since another change is in
> > >    the queue to change usage of bin_attrs_new to bin_attrs
> > >    (sysfs: finalize the constification of 'struct bin_attribute').
> > 
> > Please fix up to apply cleanly without build warnings:
> > 
> > drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> >   1893 |         .bin_attrs = vmbus_chan_bin_attrs,
> >        |                      ^~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> 
> Hi Greg,
> I tried reproducing this error but could not see it. Should I rebase the
> change to some other tree or use some specific config option, gcc version,
> compilation flag etc.?
> 
> I tried the following:
> * Rebased to latest linux-next tip with below base commit:
> 393d0c54cae31317deaa9043320c5fd9454deabc
> * Regular compilation with gcc: make -j8
> * extra flags:
>   make -j8  EXTRA_CFLAGS="-Wall -O2"
>   make -j8 EXTRA_CFLAGS="-Wincompatible-pointer-types-discards-qualifiers
> -Werror"
> * Tried gcc 11.4, 13.3
> * Tried clang/LLVM with version 18.1.3 : make LLVM=1

I tried this against my char-misc-linus branch (which is pretty much
just 6.15.0-rc4 plus some iio patches), and it fails with that error
above.

> BTW I had to edit the type for bin_attrs as this change got merged recently:
> 9bec944506fa ("sysfs: constify attribute_group::bin_attrs")
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 576b8b3c60af..f418aae4f113 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -107,7 +107,7 @@ struct attribute_group {
>                                             int);
>         struct attribute        **attrs;
>         union {
> -               struct bin_attribute            **bin_attrs;
> +               const struct bin_attribute      *const *bin_attrs;
>                 const struct bin_attribute      *const *bin_attrs_new;
>         };
>  };

That commit is not in my char-misc branches, that's coming from
somewhere else.

thanks,

greg k-h

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Naman Jain]

On 5/1/2025 9:35 PM, Greg Kroah-Hartman wrote:
> On Mon, Apr 28, 2025 at 02:37:22PM +0530, Naman Jain wrote:
>>
>>
>> On 4/25/2025 7:30 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
>>>> Hi,
>>>> This patch series aims to address the sysfs creation issue for the ring
>>>> buffer by reorganizing the code. Additionally, it updates the ring sysfs
>>>> size to accurately reflect the actual ring buffer size, rather than a
>>>> fixed static value.
>>>>
>>>> PFB change logs:
>>>>
>>>> Changes since v5:
>>>> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
>>>> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>>>>     commit msg of both patches.
>>>> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>>>>     hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>>>>     now.
>>>> * Changed type for declaring bin_attrs due to changes introduced by
>>>>     commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>>>>     merged recently. Did not use bin_attrs_new since another change is in
>>>>     the queue to change usage of bin_attrs_new to bin_attrs
>>>>     (sysfs: finalize the constification of 'struct bin_attribute').
>>>
>>> Please fix up to apply cleanly without build warnings:
>>>
>>> drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>>>    1893 |         .bin_attrs = vmbus_chan_bin_attrs,
>>>         |                      ^~~~~~~~~~~~~~~~~~~~
>>> 1 error generated.
>>
>> Hi Greg,
>> I tried reproducing this error but could not see it. Should I rebase the
>> change to some other tree or use some specific config option, gcc version,
>> compilation flag etc.?
>>
>> I tried the following:
>> * Rebased to latest linux-next tip with below base commit:
>> 393d0c54cae31317deaa9043320c5fd9454deabc
>> * Regular compilation with gcc: make -j8
>> * extra flags:
>>    make -j8  EXTRA_CFLAGS="-Wall -O2"
>>    make -j8 EXTRA_CFLAGS="-Wincompatible-pointer-types-discards-qualifiers
>> -Werror"
>> * Tried gcc 11.4, 13.3
>> * Tried clang/LLVM with version 18.1.3 : make LLVM=1
> 
> I tried this against my char-misc-linus branch (which is pretty much
> just 6.15.0-rc4 plus some iio patches), and it fails with that error
> above.
> 
>> BTW I had to edit the type for bin_attrs as this change got merged recently:
>> 9bec944506fa ("sysfs: constify attribute_group::bin_attrs")
>>
>> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
>> index 576b8b3c60af..f418aae4f113 100644
>> --- a/include/linux/sysfs.h
>> +++ b/include/linux/sysfs.h
>> @@ -107,7 +107,7 @@ struct attribute_group {
>>                                              int);
>>          struct attribute        **attrs;
>>          union {
>> -               struct bin_attribute            **bin_attrs;
>> +               const struct bin_attribute      *const *bin_attrs;
>>                  const struct bin_attribute      *const *bin_attrs_new;
>>          };
>>   };
> 
> That commit is not in my char-misc branches, that's coming from
> somewhere else.
> 
> thanks,
> 
> greg k-h

Hi Greg,

I can send a patch based on char-misc/6.15.0-rc4 which does not have 
this patch, but I am worried that it will cause compilation issues when 
your branch is merged with linux-next since this change is already there 
in linux-next. Do you want me to proceed with sending a patch on 6.15.0-rc4?

Here are more details of that patch:

"""
sysfs: constify attribute_group::bin_attrs
All users of this field have been migrated to bin_attrs_new.
It can now be constified.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Link: 
https://lore.kernel.org/r/20250313-sysfs-const-bin_attr-final-v2-2-96284e1e88ce@weissschuh.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

"""

Regards,
Naman

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Greg Kroah-Hartman]

On Fri, May 02, 2025 at 11:31:03AM +0530, Naman Jain wrote:
> 
> 
> On 5/1/2025 9:35 PM, Greg Kroah-Hartman wrote:
> > On Mon, Apr 28, 2025 at 02:37:22PM +0530, Naman Jain wrote:
> > > 
> > > 
> > > On 4/25/2025 7:30 PM, Greg Kroah-Hartman wrote:
> > > > On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
> > > > > Hi,
> > > > > This patch series aims to address the sysfs creation issue for the ring
> > > > > buffer by reorganizing the code. Additionally, it updates the ring sysfs
> > > > > size to accurately reflect the actual ring buffer size, rather than a
> > > > > fixed static value.
> > > > > 
> > > > > PFB change logs:
> > > > > 
> > > > > Changes since v5:
> > > > > https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
> > > > > * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
> > > > >     commit msg of both patches.
> > > > > * Missed to remove check for "primary_channel->device_obj->channels_kset" in
> > > > >     hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
> > > > >     now.
> > > > > * Changed type for declaring bin_attrs due to changes introduced by
> > > > >     commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
> > > > >     merged recently. Did not use bin_attrs_new since another change is in
> > > > >     the queue to change usage of bin_attrs_new to bin_attrs
> > > > >     (sysfs: finalize the constification of 'struct bin_attribute').
> > > > 
> > > > Please fix up to apply cleanly without build warnings:
> > > > 
> > > > drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> > > >    1893 |         .bin_attrs = vmbus_chan_bin_attrs,
> > > >         |                      ^~~~~~~~~~~~~~~~~~~~
> > > > 1 error generated.
> > > 
> > > Hi Greg,
> > > I tried reproducing this error but could not see it. Should I rebase the
> > > change to some other tree or use some specific config option, gcc version,
> > > compilation flag etc.?
> > > 
> > > I tried the following:
> > > * Rebased to latest linux-next tip with below base commit:
> > > 393d0c54cae31317deaa9043320c5fd9454deabc
> > > * Regular compilation with gcc: make -j8
> > > * extra flags:
> > >    make -j8  EXTRA_CFLAGS="-Wall -O2"
> > >    make -j8 EXTRA_CFLAGS="-Wincompatible-pointer-types-discards-qualifiers
> > > -Werror"
> > > * Tried gcc 11.4, 13.3
> > > * Tried clang/LLVM with version 18.1.3 : make LLVM=1
> > 
> > I tried this against my char-misc-linus branch (which is pretty much
> > just 6.15.0-rc4 plus some iio patches), and it fails with that error
> > above.
> > 
> > > BTW I had to edit the type for bin_attrs as this change got merged recently:
> > > 9bec944506fa ("sysfs: constify attribute_group::bin_attrs")
> > > 
> > > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > > index 576b8b3c60af..f418aae4f113 100644
> > > --- a/include/linux/sysfs.h
> > > +++ b/include/linux/sysfs.h
> > > @@ -107,7 +107,7 @@ struct attribute_group {
> > >                                              int);
> > >          struct attribute        **attrs;
> > >          union {
> > > -               struct bin_attribute            **bin_attrs;
> > > +               const struct bin_attribute      *const *bin_attrs;
> > >                  const struct bin_attribute      *const *bin_attrs_new;
> > >          };
> > >   };
> > 
> > That commit is not in my char-misc branches, that's coming from
> > somewhere else.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> I can send a patch based on char-misc/6.15.0-rc4 which does not have this
> patch, but I am worried that it will cause compilation issues when your
> branch is merged with linux-next since this change is already there in
> linux-next. Do you want me to proceed with sending a patch on 6.15.0-rc4?

Yes, because you want this fix in 6.15-final, right?

> Here are more details of that patch:
> 
> """
> sysfs: constify attribute_group::bin_attrs
> All users of this field have been migrated to bin_attrs_new.
> It can now be constified.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> Link: https://lore.kernel.org/r/20250313-sysfs-const-bin_attr-final-v2-2-96284e1e88ce@weissschuh.net
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> """

I know that patch, I will deal with that in linux-next when needed, you
shouldn't be worrying about it.  I'm more concerned as to why your patch
was not being tested against Linus's tree if you expected it to be in
the latest release and backported everywhere as it you asked it to be.

thanks,

greg k-h

Re: [PATCH v6 0/2] uio_hv_generic: Fix ring buffer sysfs creation path

[Naman Jain]

On 5/2/2025 11:43 AM, Greg Kroah-Hartman wrote:
> On Fri, May 02, 2025 at 11:31:03AM +0530, Naman Jain wrote:
>>
>>
>> On 5/1/2025 9:35 PM, Greg Kroah-Hartman wrote:
>>> On Mon, Apr 28, 2025 at 02:37:22PM +0530, Naman Jain wrote:
>>>>
>>>>
>>>> On 4/25/2025 7:30 PM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Apr 24, 2025 at 11:05:22AM +0530, Naman Jain wrote:
>>>>>> Hi,
>>>>>> This patch series aims to address the sysfs creation issue for the ring
>>>>>> buffer by reorganizing the code. Additionally, it updates the ring sysfs
>>>>>> size to accurately reflect the actual ring buffer size, rather than a
>>>>>> fixed static value.
>>>>>>
>>>>>> PFB change logs:
>>>>>>
>>>>>> Changes since v5:
>>>>>> https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
>>>>>> * Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
>>>>>>      commit msg of both patches.
>>>>>> * Missed to remove check for "primary_channel->device_obj->channels_kset" in
>>>>>>      hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
>>>>>>      now.
>>>>>> * Changed type for declaring bin_attrs due to changes introduced by
>>>>>>      commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
>>>>>>      merged recently. Did not use bin_attrs_new since another change is in
>>>>>>      the queue to change usage of bin_attrs_new to bin_attrs
>>>>>>      (sysfs: finalize the constification of 'struct bin_attribute').
>>>>>
>>>>> Please fix up to apply cleanly without build warnings:
>>>>>
>>>>> drivers/hv/vmbus_drv.c:1893:15: error: initializing 'struct bin_attribute **' with an expression of type 'const struct bin_attribute *const[2]' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>>>>>     1893 |         .bin_attrs = vmbus_chan_bin_attrs,
>>>>>          |                      ^~~~~~~~~~~~~~~~~~~~
>>>>> 1 error generated.
>>>>
>>>> Hi Greg,
>>>> I tried reproducing this error but could not see it. Should I rebase the
>>>> change to some other tree or use some specific config option, gcc version,
>>>> compilation flag etc.?
>>>>
>>>> I tried the following:
>>>> * Rebased to latest linux-next tip with below base commit:
>>>> 393d0c54cae31317deaa9043320c5fd9454deabc
>>>> * Regular compilation with gcc: make -j8
>>>> * extra flags:
>>>>     make -j8  EXTRA_CFLAGS="-Wall -O2"
>>>>     make -j8 EXTRA_CFLAGS="-Wincompatible-pointer-types-discards-qualifiers
>>>> -Werror"
>>>> * Tried gcc 11.4, 13.3
>>>> * Tried clang/LLVM with version 18.1.3 : make LLVM=1
>>>
>>> I tried this against my char-misc-linus branch (which is pretty much
>>> just 6.15.0-rc4 plus some iio patches), and it fails with that error
>>> above.
>>>
>>>> BTW I had to edit the type for bin_attrs as this change got merged recently:
>>>> 9bec944506fa ("sysfs: constify attribute_group::bin_attrs")
>>>>
>>>> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
>>>> index 576b8b3c60af..f418aae4f113 100644
>>>> --- a/include/linux/sysfs.h
>>>> +++ b/include/linux/sysfs.h
>>>> @@ -107,7 +107,7 @@ struct attribute_group {
>>>>                                               int);
>>>>           struct attribute        **attrs;
>>>>           union {
>>>> -               struct bin_attribute            **bin_attrs;
>>>> +               const struct bin_attribute      *const *bin_attrs;
>>>>                   const struct bin_attribute      *const *bin_attrs_new;
>>>>           };
>>>>    };
>>>
>>> That commit is not in my char-misc branches, that's coming from
>>> somewhere else.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Hi Greg,
>>
>> I can send a patch based on char-misc/6.15.0-rc4 which does not have this
>> patch, but I am worried that it will cause compilation issues when your
>> branch is merged with linux-next since this change is already there in
>> linux-next. Do you want me to proceed with sending a patch on 6.15.0-rc4?
> 
> Yes, because you want this fix in 6.15-final, right?
> 
>> Here are more details of that patch:
>>
>> """
>> sysfs: constify attribute_group::bin_attrs
>> All users of this field have been migrated to bin_attrs_new.
>> It can now be constified.
>>
>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>> Link: https://lore.kernel.org/r/20250313-sysfs-const-bin_attr-final-v2-2-96284e1e88ce@weissschuh.net
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> """
> 
> I know that patch, I will deal with that in linux-next when needed, you
> shouldn't be worrying about it.  I'm more concerned as to why your patch
> was not being tested against Linus's tree if you expected it to be in
> the latest release and backported everywhere as it you asked it to be.
> 
> thanks,
> 
> greg k-h

Sure, thanks. I'll rebase it to 6.15.0-rc4 and send the next version.

Regards,
Naman