[RFC,v2,10/14] vhost: improve log for memory dumping configuration

Message ID 20231208145950.2184940-11-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Detect superfluous newline in logs |

Commit Message

David Marchand Dec. 8, 2023, 2:59 p.m. UTC
  Add the device name as a prefix of logs associated to madvise() calls.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/iotlb.c      | 18 +++++++++---------
 lib/vhost/vhost.h      |  2 +-
 lib/vhost/vhost_user.c | 26 +++++++++++++-------------
 3 files changed, 23 insertions(+), 23 deletions(-)
  

Comments

Stephen Hemminger Dec. 8, 2023, 5:14 p.m. UTC | #1
On Fri,  8 Dec 2023 15:59:44 +0100
David Marchand <david.marchand@redhat.com> wrote:

> Add the device name as a prefix of logs associated to madvise() calls.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/vhost/iotlb.c      | 18 +++++++++---------
>  lib/vhost/vhost.h      |  2 +-
>  lib/vhost/vhost_user.c | 26 +++++++++++++-------------
>  3 files changed, 23 insertions(+), 23 deletions(-)

The logging part looks good, but looking at the code, the function
mem_set_dump() has some things that should be addressed.

Since it is global (but not exported) function the function name
may potentially clash when used with static linkage. It should
be renamed.

Code is duplication of eal_mem_set_dump(). Would be better
to have one version. Maybe rte_eal_mem_set_dump()?

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
  

Patch

diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c
index 87ac0e5126..10ab77262e 100644
--- a/lib/vhost/iotlb.c
+++ b/lib/vhost/iotlb.c
@@ -54,16 +54,16 @@  vhost_user_iotlb_share_page(struct vhost_iotlb_entry *a, struct vhost_iotlb_entr
 }
 
 static void
-vhost_user_iotlb_set_dump(struct vhost_iotlb_entry *node)
+vhost_user_iotlb_set_dump(struct virtio_net *dev, struct vhost_iotlb_entry *node)
 {
 	uint64_t start;
 
 	start = node->uaddr + node->uoffset;
-	mem_set_dump((void *)(uintptr_t)start, node->size, true, RTE_BIT64(node->page_shift));
+	mem_set_dump(dev, (void *)(uintptr_t)start, node->size, true, RTE_BIT64(node->page_shift));
 }
 
 static void
-vhost_user_iotlb_clear_dump(struct vhost_iotlb_entry *node,
+vhost_user_iotlb_clear_dump(struct virtio_net *dev, struct vhost_iotlb_entry *node,
 		struct vhost_iotlb_entry *prev, struct vhost_iotlb_entry *next)
 {
 	uint64_t start, end;
@@ -80,7 +80,7 @@  vhost_user_iotlb_clear_dump(struct vhost_iotlb_entry *node,
 		end = RTE_ALIGN_FLOOR(end, RTE_BIT64(node->page_shift));
 
 	if (end > start)
-		mem_set_dump((void *)(uintptr_t)start, end - start, false,
+		mem_set_dump(dev, (void *)(uintptr_t)start, end - start, false,
 			RTE_BIT64(node->page_shift));
 }
 
@@ -204,7 +204,7 @@  vhost_user_iotlb_cache_remove_all(struct virtio_net *dev)
 	vhost_user_iotlb_wr_lock_all(dev);
 
 	RTE_TAILQ_FOREACH_SAFE(node, &dev->iotlb_list, next, temp_node) {
-		vhost_user_iotlb_clear_dump(node, NULL, NULL);
+		vhost_user_iotlb_clear_dump(dev, node, NULL, NULL);
 
 		TAILQ_REMOVE(&dev->iotlb_list, node, next);
 		vhost_user_iotlb_remove_notify(dev, node);
@@ -230,7 +230,7 @@  vhost_user_iotlb_cache_random_evict(struct virtio_net *dev)
 		if (!entry_idx) {
 			struct vhost_iotlb_entry *next_node = RTE_TAILQ_NEXT(node, next);
 
-			vhost_user_iotlb_clear_dump(node, prev_node, next_node);
+			vhost_user_iotlb_clear_dump(dev, node, prev_node, next_node);
 
 			TAILQ_REMOVE(&dev->iotlb_list, node, next);
 			vhost_user_iotlb_remove_notify(dev, node);
@@ -285,7 +285,7 @@  vhost_user_iotlb_cache_insert(struct virtio_net *dev, uint64_t iova, uint64_t ua
 			vhost_user_iotlb_pool_put(dev, new_node);
 			goto unlock;
 		} else if (node->iova > new_node->iova) {
-			vhost_user_iotlb_set_dump(new_node);
+			vhost_user_iotlb_set_dump(dev, new_node);
 
 			TAILQ_INSERT_BEFORE(node, new_node, next);
 			dev->iotlb_cache_nr++;
@@ -293,7 +293,7 @@  vhost_user_iotlb_cache_insert(struct virtio_net *dev, uint64_t iova, uint64_t ua
 		}
 	}
 
-	vhost_user_iotlb_set_dump(new_node);
+	vhost_user_iotlb_set_dump(dev, new_node);
 
 	TAILQ_INSERT_TAIL(&dev->iotlb_list, new_node, next);
 	dev->iotlb_cache_nr++;
@@ -322,7 +322,7 @@  vhost_user_iotlb_cache_remove(struct virtio_net *dev, uint64_t iova, uint64_t si
 		if (iova < node->iova + node->size) {
 			struct vhost_iotlb_entry *next_node = RTE_TAILQ_NEXT(node, next);
 
-			vhost_user_iotlb_clear_dump(node, prev_node, next_node);
+			vhost_user_iotlb_clear_dump(dev, node, prev_node, next_node);
 
 			TAILQ_REMOVE(&dev->iotlb_list, node, next);
 			vhost_user_iotlb_remove_notify(dev, node);
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index f8624fba3d..5f24911190 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -1062,6 +1062,6 @@  mbuf_is_consumed(struct rte_mbuf *m)
 	return true;
 }
 
-void mem_set_dump(void *ptr, size_t size, bool enable, uint64_t alignment);
+void mem_set_dump(struct virtio_net *dev, void *ptr, size_t size, bool enable, uint64_t alignment);
 
 #endif /* _VHOST_NET_CDEV_H_ */
diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index e36312181a..413f068bcd 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -763,7 +763,7 @@  hua_to_alignment(struct rte_vhost_memory *mem, void *ptr)
 }
 
 void
-mem_set_dump(void *ptr, size_t size, bool enable, uint64_t pagesz)
+mem_set_dump(struct virtio_net *dev, void *ptr, size_t size, bool enable, uint64_t pagesz)
 {
 #ifdef MADV_DONTDUMP
 	void *start = RTE_PTR_ALIGN_FLOOR(ptr, pagesz);
@@ -771,8 +771,8 @@  mem_set_dump(void *ptr, size_t size, bool enable, uint64_t pagesz)
 	size_t len = end - (uintptr_t)start;
 
 	if (madvise(start, len, enable ? MADV_DODUMP : MADV_DONTDUMP) == -1) {
-		rte_log(RTE_LOG_INFO, vhost_config_log_level,
-			"VHOST_CONFIG: could not set coredump preference (%s).\n", strerror(errno));
+		VHOST_LOG_CONFIG(dev->ifname, INFO,
+			"could not set coredump preference (%s).\n", strerror(errno));
 	}
 #endif
 }
@@ -807,7 +807,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 			return;
 		}
 
-		mem_set_dump(vq->desc_packed, len, true,
+		mem_set_dump(dev, vq->desc_packed, len, true,
 			hua_to_alignment(dev->mem, vq->desc_packed));
 		numa_realloc(&dev, &vq);
 		*pdev = dev;
@@ -824,7 +824,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 			return;
 		}
 
-		mem_set_dump(vq->driver_event, len, true,
+		mem_set_dump(dev, vq->driver_event, len, true,
 			hua_to_alignment(dev->mem, vq->driver_event));
 		len = sizeof(struct vring_packed_desc_event);
 		vq->device_event = (struct vring_packed_desc_event *)
@@ -837,7 +837,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 			return;
 		}
 
-		mem_set_dump(vq->device_event, len, true,
+		mem_set_dump(dev, vq->device_event, len, true,
 			hua_to_alignment(dev->mem, vq->device_event));
 		vq->access_ok = true;
 		return;
@@ -855,7 +855,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 		return;
 	}
 
-	mem_set_dump(vq->desc, len, true, hua_to_alignment(dev->mem, vq->desc));
+	mem_set_dump(dev, vq->desc, len, true, hua_to_alignment(dev->mem, vq->desc));
 	numa_realloc(&dev, &vq);
 	*pdev = dev;
 	*pvq = vq;
@@ -871,7 +871,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 		return;
 	}
 
-	mem_set_dump(vq->avail, len, true, hua_to_alignment(dev->mem, vq->avail));
+	mem_set_dump(dev, vq->avail, len, true, hua_to_alignment(dev->mem, vq->avail));
 	len = sizeof(struct vring_used) +
 		sizeof(struct vring_used_elem) * vq->size;
 	if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX))
@@ -884,7 +884,7 @@  translate_ring_addresses(struct virtio_net **pdev, struct vhost_virtqueue **pvq)
 		return;
 	}
 
-	mem_set_dump(vq->used, len, true, hua_to_alignment(dev->mem, vq->used));
+	mem_set_dump(dev, vq->used, len, true, hua_to_alignment(dev->mem, vq->used));
 
 	if (vq->last_used_idx != vq->used->idx) {
 		VHOST_LOG_CONFIG(dev->ifname, WARNING,
@@ -1274,7 +1274,7 @@  vhost_user_mmap_region(struct virtio_net *dev,
 	region->mmap_addr = mmap_addr;
 	region->mmap_size = mmap_size;
 	region->host_user_addr = (uint64_t)(uintptr_t)mmap_addr + mmap_offset;
-	mem_set_dump(mmap_addr, mmap_size, false, alignment);
+	mem_set_dump(dev, mmap_addr, mmap_size, false, alignment);
 
 	if (dev->async_copy) {
 		if (add_guest_pages(dev, region, alignment) < 0) {
@@ -1580,7 +1580,7 @@  inflight_mem_alloc(struct virtio_net *dev, const char *name, size_t size, int *f
 	}
 
 	alignment = get_blk_size(mfd);
-	mem_set_dump(ptr, size, false, alignment);
+	mem_set_dump(dev, ptr, size, false, alignment);
 	*fd = mfd;
 	return ptr;
 }
@@ -1789,7 +1789,7 @@  vhost_user_set_inflight_fd(struct virtio_net **pdev,
 		dev->inflight_info->fd = -1;
 	}
 
-	mem_set_dump(addr, mmap_size, false, get_blk_size(fd));
+	mem_set_dump(dev, addr, mmap_size, false, get_blk_size(fd));
 	dev->inflight_info->fd = fd;
 	dev->inflight_info->addr = addr;
 	dev->inflight_info->size = mmap_size;
@@ -2343,7 +2343,7 @@  vhost_user_set_log_base(struct virtio_net **pdev,
 	dev->log_addr = (uint64_t)(uintptr_t)addr;
 	dev->log_base = dev->log_addr + off;
 	dev->log_size = size;
-	mem_set_dump(addr, size + off, false, alignment);
+	mem_set_dump(dev, addr, size + off, false, alignment);
 
 	for (i = 0; i < dev->nr_vring; i++) {
 		struct vhost_virtqueue *vq = dev->virtqueue[i];