On Fri, May 9, 2025 at 2:58 PM Song Liu song@kernel.org wrote:
On Fri, May 9, 2025 at 2:43 PM T.J. Mercier tjmercier@google.com wrote:
[...]
Personally, I would prefer we just merge all the logic of create_udmabuf() and create_sys_heap_dmabuf() into create_test_buffers().
That's a lot of different stuff to put in one place. How about returning file descriptors from the buffer create functions while having them clean up after themselves:
I do like this version better. Some nitpicks though.
-static int memfd, udmabuf; +static int udmabuf;
About this, and ...
static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] = "udmabuf_test_buffer_for_iter"; static size_t udmabuf_test_buffer_size; static int sysheap_dmabuf; static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] = "sysheap_test_buffer_for_iter"; static size_t sysheap_test_buffer_size;
-static int create_udmabuf(int map_fd) +static int create_udmabuf(void) { struct udmabuf_create create;
int dev_udmabuf;bool f = false;
int dev_udmabuf, memfd, udmabuf;.. here.
It is not ideal to have a global udmabuf and a local udmabuf. If we want the global version, let's rename the local one.
Ok let me change up the name of the aliasing variable to local_udmabuf.
[...]
static int create_test_buffers(int map_fd) {
int ret;
bool f = false;udmabuf = create_udmabuf();sysheap_dmabuf = create_sys_heap_dmabuf();
ret = create_udmabuf(map_fd);if (ret)return ret;
if (udmabuf < 0 || sysheap_dmabuf < 0)return -1;We also need destroy_test_buffers() on the error path here, or at the caller.
The caller does currently check to decide if it should bother running the tests or not, and calls destroy_test_buffers() if not.
return create_sys_heap_dmabuf(map_fd);
return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name,&f, BPF_ANY) ||
bpf_map_update_elem(map_fd, sysheap_test_buffer_name,&f, BPF_ANY); }
static void destroy_test_buffers(void) { close(udmabuf);
close(memfd); close(sysheap_dmabuf);For the two global fds, let's reset them to -1 right after close().
Thanks, Song
Will do, thanks.