Re: [PATCH] tcp_vu: head_cnt need not be global

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 18 Feb 2025 09:01:30 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> On 18/02/2025 03:50, David Gibson wrote:
> > head_cnt is a global variable which tracks how many entries in head[] are
> > currently used.  The fact that it's global obscures the fact that the
> > lifetime over which it has a meaningful value is quite short: a single
> > call to of tcp_vu_data_from_sock().
> > 
> > Make it a local to tcp_vu_data_from_sock() to make that lifetime clearer.
> > We keep the head[] array global for now - although technically it has the
> > same valid lifetime - because it's large enough we might not want to put
> > it on the stack.  
> 
> We can make the array local but on the heap using a static declaration in the function.

Well, it would still be in the data segment, not actually on the heap
[1], but it would be somewhat hidden in the function.

I'd rather suggest that we keep this stuff visible at the "top level".


[1] I had a doubt for a moment so I tried it:

$ git diff
diff --git a/tcp_vu.c b/tcp_vu.c
index 0622f17..c40b102 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -187,6 +187,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 {
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
+	static char am_i_heap_or_am_i_data[64 << 20];
 	struct msghdr mh_sock = { 0 };
 	uint16_t mss = MSS_GET(conn);
 	int s = conn->sock;
@@ -195,6 +196,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 	int elem_cnt;
 	int i;
 
+	memset(am_i_heap_or_am_i_data, 0, sizeof(am_i_heap_or_am_i_data));
+
 	*iov_cnt = 0;
 
 	hdrlen = tcp_vu_hdrlen(v6);

$ nm -td -S passt | grep am_i_heap_or_am_i_data
0000000202163552 0000000067108864 b am_i_heap_or_am_i_data.2

-- 
@@ -187,6 +187,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 {
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
+	static char am_i_heap_or_am_i_data[64 << 20];
 	struct msghdr mh_sock = { 0 };
 	uint16_t mss = MSS_GET(conn);
 	int s = conn->sock;
@@ -195,6 +196,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c,
 	int elem_cnt;
 	int i;
 
+	memset(am_i_heap_or_am_i_data, 0, sizeof(am_i_heap_or_am_i_data));
+
 	*iov_cnt = 0;
 
 	hdrlen = tcp_vu_hdrlen(v6);

$ nm -td -S passt | grep am_i_heap_or_am_i_data
0000000202163552 0000000067108864 b am_i_heap_or_am_i_data.2

-- 
Stefano