On Wed, 29 Mar 2023 10:19:44 +0100
Vincent Donnefort [off-list ref] wrote:

quoted

I've been playing with this a bit, and I'm thinking, do we need the
data_pages[] array on the meta page?

I noticed that I'm not even using it.

Currently, we need to do a ioctl every time we finish with the reader page,
and that updates the reader_page in the meta data to point to the next page
to read. When do we need to look at the data_start section?  

This is for non-consuming read, to get all the pages in order.

Yeah, I was trying to see how a non consuming read would work, and was
having issues figuring that out without the tail page being updated.

If we remove this section we would lose this ability ... but we'd also simplify
the code by a good order of magnitude (don't need the update ioctl anymore, no
need to keep those pages in order and everything can fit a 0-order meta-page).
And the non-consuming read doesn't bring much to the user over the pipe version.

This will although impact our hypervisor tracing which will only be able to
expose trace_pipe interfaces. But I don't think it is a problem, all userspace
tools only relying on consuming read anyway.

So if you're happy dropping this support, let's get rid of it.

I don't really want to get rid of it, but perhaps break it up where we
don't have it in the first release, but add it in a second one. That will
also make sure that we can expand the API if necessary (one reason I wanted
the "data_start" in the first place).

Let's drop it for now, but be able to add it later, an have the current
structure be:

struct ring_buffer_meta_page_header {
#if __BITS_PER_LONG == 64
	__u64	entries;
	__u64	overrun;
#else
	__u32	entries;
	__u32	overrun;
#endif
	__u32	pages_touched;
	__u32	meta_page_size;
	__u32	reader_page;	/* page ID for the reader page */
	__u32	nr_data_pages;	/* doesn't take into account the reader_page */
};

BTW, shouldn't the nr_data_pages take into account the reader page? As it
is part of the array we traverse isn't it?

-- Steve