> From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> > Sent: Monday, March 10, 2025 9:45 PM > > [...] > > Hi Greg, > > I understand this is deviating from the discussions that we have had > > till now, but I wanted to kindly request your opinion on the following > > approach, which came up in one of our internal discussions. > > > > By moving the sysfs creation logic from hv_uio_probe to hv_uio_open, I > > believe we can address this problem. Here are some of the benefits of > > this approach: > > > > * This approach involves minimal changes and provides a localized > > solution. I prefer the "Approach 1" below, which requires only 1/10 of the changes of "Approach 2". > > * Since the use-case of ring sysfs is specific to uio_hv_generic and > > DPDK, this will give us the flexibility to modify it based on > > requirements. For example, ring_buffer_bin_attr.size should depend on > > the ring buffer's allocated size, which is easier to manage if the > > current code resides in uio_hv_generic. The 'ring' sysfs file is used by DPDK, the user-mode driver for fcopy (tools/hv/hv_fcopy_uio_daemon.c) and other out-of-tree user-mode drivers. Before the hv_uio_generic driver and the user-mode driver load, the hv_vmbus driver doesn't know the correct size of the 'ring'; currently the patch of "Approach 2" hardcodes ring_buffer_bin_attr.size to 4MB, which is incorrect for the Fcopy VMBus device and other VMBus devices. "Approach 1" also uses a hardcoded 4MB, but it can't be easily fixed by adding one line in hv_uio_probe(). With "Approach 2", the fix would be difficult as we would need to pass one more param 'size' from the driver hv_uio_generic to hv_vmbus. The patch of "Approach 2" already passes two params from hv_uio_generic to hv_vmbus: 'ring_sysfs_visible' and 'mmap_ring_buffer', and adds and exports 2 APIs to hv_uio_generic. With Approach 1, we can avoid all the trouble. > > * The use-case of DPDK is such that at any given time, either the > > hv_netvsc kernel driver or the userspace (DPDK) will be controlling this > > HV_NIC device. We do not want to expose the ring buffer to userspace > > when hv_netvsc is using the device. This is where the "awareness" of the > > current user comes into play, and we need a way to control the > > visibility of the "ring" sysfs from uio_hv_generic. > > > > > > Alternatively, I could focus on resolving the race condition you > > mentioned and proceed with refining the patch. This approach addresses > > most of the general practice concerns you highlighted. > > > > Regards, > > Naman > > Hello Greg, > Here are the two approaches that we discussed. > Can you please suggest the approach which looks better to you > for next version. > > **Approach 1: move sysfs creation to hv_uio_open** > [...] > **Approach 2: Move sysfs creation to hyperv drivers** In general, indeed we would want to avoid manipulating sysfs and kobj directly, but here IMO calling sysfs_create_bin_file() in hv_uio_generic is reasonable as hv_uio_generic is the only user of the 'ring' sysfs file, and it has more info about the 'ring' (i.e. the correct size; when the 'ring' is used); I prefer "Approach 1" since the patch is much smaller and cleaner. Greg, can we have your opinions? Thanks, Dexuan
