On Tue, 20 Feb 2024 09:50:13 -0500
Mathieu Desnoyers [off-list ref] wrote:

On 2024-02-20 09:19, Steven Rostedt wrote:

quoted

On Mon, 19 Feb 2024 18:20:32 -0500
Steven Rostedt [off-list ref] wrote:
  

quoted

Instead of using local_add_return() to reserve the ring buffer data,
Mathieu Desnoyers suggested using local_cmpxchg(). This would simplify the
reservation with the time keeping code.

Although, it does not get rid of the double time stamps (before_stamp and
write_stamp), using cmpxchg() does get rid of the more complex case when
an interrupting event occurs between getting the timestamps and reserving
the data, as when that happens, it just tries again instead of dealing
with it.

Before we had:

	w = local_read(&tail_page->write);
	/* get time stamps */
	write = local_add_return(length, &tail_page->write);
	if (write - length == w) {
		/* do simple case */
	} else {
		/* do complex case */
	}

By switching the local_add_return() to a local_try_cmpxchg() it can now be:

	 w = local_read(&tail_page->write);
  again:
	/* get time stamps */
	if (!local_try_cmpxchg(&tail_page->write, &w, w + length))
		goto again;

	 /* do simple case */  

Something about this logic is causing __rb_next_reserve() to sometimes
always return -EAGAIN and triggering the:

     RB_WARN_ON(cpu_buffer, ++nr_loops > 1000)

Which disables the ring buffer.

I'm not sure what it is, but until I do, I'm removing the patch from my
queue.  

Try resetting the info->add_timestamp flags to add_ts_default on goto again
within __rb_reserve_next().

I was looking at that too, but I don't know how it will make a difference.

Note, the test that fails is in my test suite, and takes about a half hour
to get there. Running that suite takes up resources (it's my main test
suite for all changes). I'm currently testing other patches so I either
need to figure it out through inspection, or this will need to wait a while.

-- Steve