linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
@ 2025-07-11  6:08 Naman Jain
  2025-07-11  6:15 ` Naman Jain
  0 siblings, 1 reply; 4+ messages in thread
From: Naman Jain @ 2025-07-11  6:08 UTC (permalink / raw)
  To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
	Michael Kelley
  Cc: linux-hyperv, linux-kernel, Olaf Hering, Saurabh Sengar,
	Naman Jain

Size of ring buffer, as defined in uio_hv_generic driver, is no longer
fixed to 16 KB. This creates a problem in fcopy, since this size was
hardcoded. With the change in place to make ring sysfs node actually
reflect the size of underlying ring buffer, it is safe to get the size
of ring sysfs file and use it for ring buffer size in fcopy daemon.
Fix the issue of disparity in ring buffer size, by making it dynamic
in fcopy uio daemon.

Cc: stable@vger.kernel.org
Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
 tools/hv/hv_fcopy_uio_daemon.c | 91 ++++++++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 10 deletions(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d14a2..7d9bcb066d3f 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -35,7 +35,10 @@
 #define WIN8_SRV_MINOR		1
 #define WIN8_SRV_VERSION	(WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
 
-#define FCOPY_UIO		"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
+#define FCOPY_DEVICE_PATH(subdir) \
+	"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/" #subdir
+#define FCOPY_UIO_PATH          FCOPY_DEVICE_PATH(uio)
+#define FCOPY_CHANNELS_PATH     FCOPY_DEVICE_PATH(channels)
 
 #define FCOPY_VER_COUNT		1
 static const int fcopy_versions[] = {
@@ -47,9 +50,62 @@ static const int fw_versions[] = {
 	UTIL_FW_VERSION
 };
 
-#define HV_RING_SIZE		0x4000 /* 16KB ring buffer size */
+static uint32_t get_ring_buffer_size(void)
+{
+	char ring_path[PATH_MAX];
+	DIR *dir;
+	struct dirent *entry;
+	struct stat st;
+	uint32_t ring_size = 0;
+	int retry_count = 0;
+
+	/* Find the channel directory */
+	dir = opendir(FCOPY_CHANNELS_PATH);
+	if (!dir) {
+		usleep(100 * 1000); /* Avoid race with kernel, wait 100ms and retry once */
+		dir = opendir(FCOPY_CHANNELS_PATH);
+		if (!dir) {
+			syslog(LOG_ERR, "Failed to open channels directory: %s", strerror(errno));
+			return 0;
+		}
+	}
+
+retry_once:
+	while ((entry = readdir(dir)) != NULL) {
+		if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
+		    strcmp(entry->d_name, "..") != 0) {
+			snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
+				 FCOPY_CHANNELS_PATH, entry->d_name);
+
+			if (stat(ring_path, &st) == 0) {
+				/*
+				 * stat returns size of Tx, Rx rings combined,
+				 * so take half of it for individual ring size.
+				 */
+				ring_size = (uint32_t)st.st_size / 2;
+				syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
+				       ring_path, ring_size);
+				break;
+			}
+		}
+	}
 
-static unsigned char desc[HV_RING_SIZE];
+	if (!ring_size && retry_count == 0) {
+		retry_count = 1;
+		rewinddir(dir);
+		usleep(100 * 1000); /* Wait 100ms and retry once */
+		goto retry_once;
+	}
+
+	closedir(dir);
+
+	if (!ring_size)
+		syslog(LOG_ERR, "Could not determine ring size");
+
+	return ring_size;
+}
+
+static unsigned char *desc;
 
 static int target_fd;
 static char target_fname[PATH_MAX];
@@ -406,7 +462,7 @@ int main(int argc, char *argv[])
 	int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
 	struct vmbus_br txbr, rxbr;
 	void *ring;
-	uint32_t len = HV_RING_SIZE;
+	uint32_t ring_size, len;
 	char uio_name[NAME_MAX] = {0};
 	char uio_dev_path[PATH_MAX] = {0};
 
@@ -437,7 +493,20 @@ int main(int argc, char *argv[])
 	openlog("HV_UIO_FCOPY", 0, LOG_USER);
 	syslog(LOG_INFO, "starting; pid is:%d", getpid());
 
-	fcopy_get_first_folder(FCOPY_UIO, uio_name);
+	ring_size = get_ring_buffer_size();
+	if (!ring_size) {
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	desc = malloc(ring_size * sizeof(unsigned char));
+	if (!desc) {
+		syslog(LOG_ERR, "malloc failed for desc buffer");
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	fcopy_get_first_folder(FCOPY_UIO_PATH, uio_name);
 	snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
 	fcopy_fd = open(uio_dev_path, O_RDWR);
 
@@ -445,17 +514,17 @@ int main(int argc, char *argv[])
 		syslog(LOG_ERR, "open %s failed; error: %d %s",
 		       uio_dev_path, errno, strerror(errno));
 		ret = fcopy_fd;
-		goto exit;
+		goto free_desc;
 	}
 
-	ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
+	ring = vmbus_uio_map(&fcopy_fd, ring_size);
 	if (!ring) {
 		ret = errno;
 		syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
 		goto close;
 	}
-	vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
-	vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
+	vmbus_br_setup(&txbr, ring, ring_size);
+	vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
 
 	rxbr.vbr->imask = 0;
 
@@ -472,7 +541,7 @@ int main(int argc, char *argv[])
 			goto close;
 		}
 
-		len = HV_RING_SIZE;
+		len = ring_size;
 		ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
 		if (unlikely(ret <= 0)) {
 			/* This indicates a failure to communicate (or worse) */
@@ -492,6 +561,8 @@ int main(int argc, char *argv[])
 	}
 close:
 	close(fcopy_fd);
+free_desc:
+	free(desc);
 exit:
 	return ret;
 }

base-commit: b551c4e2a98a177a06148cf16505643cd2108386
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
  2025-07-11  6:08 [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer Naman Jain
@ 2025-07-11  6:15 ` Naman Jain
  2025-07-12 20:00   ` Long Li
  0 siblings, 1 reply; 4+ messages in thread
From: Naman Jain @ 2025-07-11  6:15 UTC (permalink / raw)
  To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
	Michael Kelley
  Cc: linux-hyperv, linux-kernel, Olaf Hering, Saurabh Sengar



On 7/11/2025 11:38 AM, Naman Jain wrote:
> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
> fixed to 16 KB. This creates a problem in fcopy, since this size was
> hardcoded. With the change in place to make ring sysfs node actually
> reflect the size of underlying ring buffer, it is safe to get the size
> of ring sysfs file and use it for ring buffer size in fcopy daemon.
> Fix the issue of disparity in ring buffer size, by making it dynamic
> in fcopy uio daemon.
> 
> Cc: stable@vger.kernel.org
> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---

Noticed that I missed adding change logs (again). Adding them now.

Changes since v3:
https://lore.kernel.org/all/20250708080319.3904-1-namjain@linux.microsoft.com/
* Added a goto label for freeing desc memory. (Saurabh)
* Avoided declaring device path twice by using FCOPY_DEVICE_PATH(subdir)
(Saurabh)
* Removed extra len variable assignment in main() (Saurabh)
* added Reviewed-by tag from Saurabh

Changes since v2:
https://lore.kernel.org/all/20250701104837.3006-1-namjain@linux.microsoft.com/
* Removed fallback mechanism to default size, to keep fcopy behavior
consistent (Long's suggestion). If ring sysfs file is not present for
some reason, things are already bad and its the right thing for fcopy to
abort.

Changes since v1:
https://lore.kernel.org/all/20250620070618.3097-1-namjain@linux.microsoft.com/

* Removed unnecessary type casting in malloc for desc variable (Olaf)
* Added retry mechanisms to avoid potential race conditions (Michael)
* Moved the logic to fetch ring size to a later part in main (Michael)



>   tools/hv/hv_fcopy_uio_daemon.c | 91 ++++++++++++++++++++++++++++++----
>   1 file changed, 81 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d14a2..7d9bcb066d3f 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -35,7 +35,10 @@
>   #define WIN8_SRV_MINOR		1
>   #define WIN8_SRV_VERSION	(WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
>   
> -#define FCOPY_UIO		"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
> +#define FCOPY_DEVICE_PATH(subdir) \
> +	"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/" #subdir
> +#define FCOPY_UIO_PATH          FCOPY_DEVICE_PATH(uio)
> +#define FCOPY_CHANNELS_PATH     FCOPY_DEVICE_PATH(channels)
>   
>   #define FCOPY_VER_COUNT		1
>   static const int fcopy_versions[] = {
> @@ -47,9 +50,62 @@ static const int fw_versions[] = {
>   	UTIL_FW_VERSION
>   };
>   
> -#define HV_RING_SIZE		0x4000 /* 16KB ring buffer size */
> +static uint32_t get_ring_buffer_size(void)
> +{
> +	char ring_path[PATH_MAX];
> +	DIR *dir;
> +	struct dirent *entry;
> +	struct stat st;
> +	uint32_t ring_size = 0;
> +	int retry_count = 0;
> +
> +	/* Find the channel directory */
> +	dir = opendir(FCOPY_CHANNELS_PATH);
> +	if (!dir) {
> +		usleep(100 * 1000); /* Avoid race with kernel, wait 100ms and retry once */
> +		dir = opendir(FCOPY_CHANNELS_PATH);
> +		if (!dir) {
> +			syslog(LOG_ERR, "Failed to open channels directory: %s", strerror(errno));
> +			return 0;
> +		}
> +	}
> +
> +retry_once:
> +	while ((entry = readdir(dir)) != NULL) {
> +		if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
> +		    strcmp(entry->d_name, "..") != 0) {
> +			snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
> +				 FCOPY_CHANNELS_PATH, entry->d_name);
> +
> +			if (stat(ring_path, &st) == 0) {
> +				/*
> +				 * stat returns size of Tx, Rx rings combined,
> +				 * so take half of it for individual ring size.
> +				 */
> +				ring_size = (uint32_t)st.st_size / 2;
> +				syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
> +				       ring_path, ring_size);
> +				break;
> +			}
> +		}
> +	}
>   
> -static unsigned char desc[HV_RING_SIZE];
> +	if (!ring_size && retry_count == 0) {
> +		retry_count = 1;
> +		rewinddir(dir);
> +		usleep(100 * 1000); /* Wait 100ms and retry once */
> +		goto retry_once;
> +	}
> +
> +	closedir(dir);
> +
> +	if (!ring_size)
> +		syslog(LOG_ERR, "Could not determine ring size");
> +
> +	return ring_size;
> +}
> +
> +static unsigned char *desc;
>   
>   static int target_fd;
>   static char target_fname[PATH_MAX];
> @@ -406,7 +462,7 @@ int main(int argc, char *argv[])
>   	int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
>   	struct vmbus_br txbr, rxbr;
>   	void *ring;
> -	uint32_t len = HV_RING_SIZE;
> +	uint32_t ring_size, len;
>   	char uio_name[NAME_MAX] = {0};
>   	char uio_dev_path[PATH_MAX] = {0};
>   
> @@ -437,7 +493,20 @@ int main(int argc, char *argv[])
>   	openlog("HV_UIO_FCOPY", 0, LOG_USER);
>   	syslog(LOG_INFO, "starting; pid is:%d", getpid());
>   
> -	fcopy_get_first_folder(FCOPY_UIO, uio_name);
> +	ring_size = get_ring_buffer_size();
> +	if (!ring_size) {
> +		ret = -ENODEV;
> +		goto exit;
> +	}
> +
> +	desc = malloc(ring_size * sizeof(unsigned char));
> +	if (!desc) {
> +		syslog(LOG_ERR, "malloc failed for desc buffer");
> +		ret = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	fcopy_get_first_folder(FCOPY_UIO_PATH, uio_name);
>   	snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
>   	fcopy_fd = open(uio_dev_path, O_RDWR);
>   
> @@ -445,17 +514,17 @@ int main(int argc, char *argv[])
>   		syslog(LOG_ERR, "open %s failed; error: %d %s",
>   		       uio_dev_path, errno, strerror(errno));
>   		ret = fcopy_fd;
> -		goto exit;
> +		goto free_desc;
>   	}
>   
> -	ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
> +	ring = vmbus_uio_map(&fcopy_fd, ring_size);
>   	if (!ring) {
>   		ret = errno;
>   		syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
>   		goto close;
>   	}
> -	vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
> -	vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
> +	vmbus_br_setup(&txbr, ring, ring_size);
> +	vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
>   
>   	rxbr.vbr->imask = 0;
>   
> @@ -472,7 +541,7 @@ int main(int argc, char *argv[])
>   			goto close;
>   		}
>   
> -		len = HV_RING_SIZE;
> +		len = ring_size;
>   		ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
>   		if (unlikely(ret <= 0)) {
>   			/* This indicates a failure to communicate (or worse) */
> @@ -492,6 +561,8 @@ int main(int argc, char *argv[])
>   	}
>   close:
>   	close(fcopy_fd);
> +free_desc:
> +	free(desc);
>   exit:
>   	return ret;
>   }
> 
> base-commit: b551c4e2a98a177a06148cf16505643cd2108386


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
  2025-07-11  6:15 ` Naman Jain
@ 2025-07-12 20:00   ` Long Li
  2025-07-15  6:25     ` Wei Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Long Li @ 2025-07-12 20:00 UTC (permalink / raw)
  To: Naman Jain, KY Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley
  Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	Olaf Hering, Saurabh Sengar

> Subject: Re: [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
>
>
>
> On 7/11/2025 11:38 AM, Naman Jain wrote:
> > Size of ring buffer, as defined in uio_hv_generic driver, is no longer
> > fixed to 16 KB. This creates a problem in fcopy, since this size was
> > hardcoded. With the change in place to make ring sysfs node actually
> > reflect the size of underlying ring buffer, it is safe to get the size
> > of ring sysfs file and use it for ring buffer size in fcopy daemon.
> > Fix the issue of disparity in ring buffer size, by making it dynamic
> > in fcopy uio daemon.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
> > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>

Reviewed-by: Long Li <longli@microsoft.com>

> > ---
>
> Noticed that I missed adding change logs (again). Adding them now.
>
> Changes since v3:
> https://lore.kern/
> el.org%2Fall%2F20250708080319.3904-1-
> namjain%40linux.microsoft.com%2F&data=05%7C02%7Clongli%40microsoft.com
> %7C5061a1ac57814cbe542b08ddc042614e%7C72f988bf86f141af91ab2d7cd011
> db47%7C1%7C0%7C638878113544016320%7CUnknown%7CTWFpbGZsb3d8eyJ
> FbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF
> pbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=loiQdbSCzLluXmFECiIrrHbbK9
> fqxJBbLTSAB26%2Bd0k%3D&reserved=0
> * Added a goto label for freeing desc memory. (Saurabh)
> * Avoided declaring device path twice by using FCOPY_DEVICE_PATH(subdir)
> (Saurabh)
> * Removed extra len variable assignment in main() (Saurabh)
> * added Reviewed-by tag from Saurabh
>
> Changes since v2:
> https://lore.kern/
> el.org%2Fall%2F20250701104837.3006-1-
> namjain%40linux.microsoft.com%2F&data=05%7C02%7Clongli%40microsoft.com
> %7C5061a1ac57814cbe542b08ddc042614e%7C72f988bf86f141af91ab2d7cd011
> db47%7C1%7C0%7C638878113544051861%7CUnknown%7CTWFpbGZsb3d8eyJ
> FbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF
> pbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=PZ9GdNL7%2FD6NfxZPhCOX
> 7zQ2DbspDgQs%2BS75PFGAFG4%3D&reserved=0
> * Removed fallback mechanism to default size, to keep fcopy behavior consistent
> (Long's suggestion). If ring sysfs file is not present for some reason, things are
> already bad and its the right thing for fcopy to abort.
>
> Changes since v1:
> https://lore.kern/
> el.org%2Fall%2F20250620070618.3097-1-
> namjain%40linux.microsoft.com%2F&data=05%7C02%7Clongli%40microsoft.com
> %7C5061a1ac57814cbe542b08ddc042614e%7C72f988bf86f141af91ab2d7cd011
> db47%7C1%7C0%7C638878113544063634%7CUnknown%7CTWFpbGZsb3d8eyJ
> FbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF
> pbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=LQsSaTjbam4DPAzMQmy1%
> 2BMB0%2BKb6L7d3xKkM954mpls%3D&reserved=0
>
> * Removed unnecessary type casting in malloc for desc variable (Olaf)
> * Added retry mechanisms to avoid potential race conditions (Michael)
> * Moved the logic to fetch ring size to a later part in main (Michael)
>
>
>
> >   tools/hv/hv_fcopy_uio_daemon.c | 91 ++++++++++++++++++++++++++++++--
> --
> >   1 file changed, 81 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/hv/hv_fcopy_uio_daemon.c
> > b/tools/hv/hv_fcopy_uio_daemon.c index 0198321d14a2..7d9bcb066d3f
> > 100644
> > --- a/tools/hv/hv_fcopy_uio_daemon.c
> > +++ b/tools/hv/hv_fcopy_uio_daemon.c
> > @@ -35,7 +35,10 @@
> >   #define WIN8_SRV_MINOR            1
> >   #define WIN8_SRV_VERSION  (WIN8_SRV_MAJOR << 16 |
> WIN8_SRV_MINOR)
> >
> > -#define FCOPY_UIO          "/sys/bus/vmbus/devices/eb765408-105f-49b6-
> b4aa-c123b64d17d4/uio"
> > +#define FCOPY_DEVICE_PATH(subdir) \
> > +   "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/"
> #subdir
> > +#define FCOPY_UIO_PATH          FCOPY_DEVICE_PATH(uio)
> > +#define FCOPY_CHANNELS_PATH     FCOPY_DEVICE_PATH(channels)
> >
> >   #define FCOPY_VER_COUNT           1
> >   static const int fcopy_versions[] = { @@ -47,9 +50,62 @@ static
> > const int fw_versions[] = {
> >     UTIL_FW_VERSION
> >   };
> >
> > -#define HV_RING_SIZE               0x4000 /* 16KB ring buffer size */
> > +static uint32_t get_ring_buffer_size(void) {
> > +   char ring_path[PATH_MAX];
> > +   DIR *dir;
> > +   struct dirent *entry;
> > +   struct stat st;
> > +   uint32_t ring_size = 0;
> > +   int retry_count = 0;
> > +
> > +   /* Find the channel directory */
> > +   dir = opendir(FCOPY_CHANNELS_PATH);
> > +   if (!dir) {
> > +           usleep(100 * 1000); /* Avoid race with kernel, wait 100ms and
> retry once */
> > +           dir = opendir(FCOPY_CHANNELS_PATH);
> > +           if (!dir) {
> > +                   syslog(LOG_ERR, "Failed to open channels
> directory: %s", strerror(errno));
> > +                   return 0;
> > +           }
> > +   }
> > +
> > +retry_once:
> > +   while ((entry = readdir(dir)) != NULL) {
> > +           if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0
> &&
> > +               strcmp(entry->d_name, "..") != 0) {
> > +                   snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
> > +                            FCOPY_CHANNELS_PATH, entry->d_name);
> > +
> > +                   if (stat(ring_path, &st) == 0) {
> > +                           /*
> > +                            * stat returns size of Tx, Rx rings combined,
> > +                            * so take half of it for individual ring size.
> > +                            */
> > +                           ring_size = (uint32_t)st.st_size / 2;
> > +                           syslog(LOG_INFO, "Ring buffer size from %s: %u
> bytes",
> > +                                  ring_path, ring_size);
> > +                           break;
> > +                   }
> > +           }
> > +   }
> >
> > -static unsigned char desc[HV_RING_SIZE];
> > +   if (!ring_size && retry_count == 0) {
> > +           retry_count = 1;
> > +           rewinddir(dir);
> > +           usleep(100 * 1000); /* Wait 100ms and retry once */
> > +           goto retry_once;
> > +   }
> > +
> > +   closedir(dir);
> > +
> > +   if (!ring_size)
> > +           syslog(LOG_ERR, "Could not determine ring size");
> > +
> > +   return ring_size;
> > +}
> > +
> > +static unsigned char *desc;
> >
> >   static int target_fd;
> >   static char target_fname[PATH_MAX];
> > @@ -406,7 +462,7 @@ int main(int argc, char *argv[])
> >     int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
> >     struct vmbus_br txbr, rxbr;
> >     void *ring;
> > -   uint32_t len = HV_RING_SIZE;
> > +   uint32_t ring_size, len;
> >     char uio_name[NAME_MAX] = {0};
> >     char uio_dev_path[PATH_MAX] = {0};
> >
> > @@ -437,7 +493,20 @@ int main(int argc, char *argv[])
> >     openlog("HV_UIO_FCOPY", 0, LOG_USER);
> >     syslog(LOG_INFO, "starting; pid is:%d", getpid());
> >
> > -   fcopy_get_first_folder(FCOPY_UIO, uio_name);
> > +   ring_size = get_ring_buffer_size();
> > +   if (!ring_size) {
> > +           ret = -ENODEV;
> > +           goto exit;
> > +   }
> > +
> > +   desc = malloc(ring_size * sizeof(unsigned char));
> > +   if (!desc) {
> > +           syslog(LOG_ERR, "malloc failed for desc buffer");
> > +           ret = -ENOMEM;
> > +           goto exit;
> > +   }
> > +
> > +   fcopy_get_first_folder(FCOPY_UIO_PATH, uio_name);
> >     snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
> >     fcopy_fd = open(uio_dev_path, O_RDWR);
> >
> > @@ -445,17 +514,17 @@ int main(int argc, char *argv[])
> >             syslog(LOG_ERR, "open %s failed; error: %d %s",
> >                    uio_dev_path, errno, strerror(errno));
> >             ret = fcopy_fd;
> > -           goto exit;
> > +           goto free_desc;
> >     }
> >
> > -   ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
> > +   ring = vmbus_uio_map(&fcopy_fd, ring_size);
> >     if (!ring) {
> >             ret = errno;
> >             syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret,
> strerror(ret));
> >             goto close;
> >     }
> > -   vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
> > -   vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
> > +   vmbus_br_setup(&txbr, ring, ring_size);
> > +   vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
> >
> >     rxbr.vbr->imask = 0;
> >
> > @@ -472,7 +541,7 @@ int main(int argc, char *argv[])
> >                     goto close;
> >             }
> >
> > -           len = HV_RING_SIZE;
> > +           len = ring_size;
> >             ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
> >             if (unlikely(ret <= 0)) {
> >                     /* This indicates a failure to communicate (or worse) */
> @@
> > -492,6 +561,8 @@ int main(int argc, char *argv[])
> >     }
> >   close:
> >     close(fcopy_fd);
> > +free_desc:
> > +   free(desc);
> >   exit:
> >     return ret;
> >   }
> >
> > base-commit: b551c4e2a98a177a06148cf16505643cd2108386


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
  2025-07-12 20:00   ` Long Li
@ 2025-07-15  6:25     ` Wei Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2025-07-15  6:25 UTC (permalink / raw)
  To: Long Li
  Cc: Naman Jain, KY Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, Olaf Hering, Saurabh Sengar

On Sat, Jul 12, 2025 at 08:00:35PM +0000, Long Li wrote:
> > Subject: Re: [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer
> >
> >
> >
> > On 7/11/2025 11:38 AM, Naman Jain wrote:
> > > Size of ring buffer, as defined in uio_hv_generic driver, is no longer
> > > fixed to 16 KB. This creates a problem in fcopy, since this size was
> > > hardcoded. With the change in place to make ring sysfs node actually
> > > reflect the size of underlying ring buffer, it is safe to get the size
> > > of ring sysfs file and use it for ring buffer size in fcopy daemon.
> > > Fix the issue of disparity in ring buffer size, by making it dynamic
> > > in fcopy uio daemon.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
> > > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> 
> Reviewed-by: Long Li <longli@microsoft.com>

Applied to hyperv-fixes. Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-15  6:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11  6:08 [PATCH v4] tools/hv: fcopy: Fix irregularities with size of ring buffer Naman Jain
2025-07-11  6:15 ` Naman Jain
2025-07-12 20:00   ` Long Li
2025-07-15  6:25     ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).