Re: [PATCH] ring-buffer: Simplify reservation with try_cmpxchg() loop

On Sat, 20 Jan 2024 08:47:13 -0500
Steven Rostedt [off-list ref] wrote:

quoted

I would also consider reducing code complexity as a worthwhile metric
in addition to speed. It makes it easier to extend in the future,
easier to understand for reviewers, and subtle bugs are less likely
to creep in.  

Really, it wouldn't make it that much simpler. The addition of the
cmpxchg() of this patch removed the nasty part of the code.

Now let's look at the difference of the two. You still need to save the
current timestamp in one variable. I have to do it in two, so your
algorithm does have that advantage. I currently have (eliminating the
"always add absolute timestamp" switch):


  w = write;
  before = before_stamp;
again:
  after = write_stamp; (equivalent to your last_tsc)
  ts = rdtsc();
  if (!w)
	delta = 0; // event has same ts as subbuf
  else if (before == after)
	delta = ts - after;
  else {
	delta = 0;
	inject_absolute = true;
  }

  before_stamp = ts;

  if (!try_cmpxchg(&write, w, w + length))
	goto again;

  write_stamp = ts;


Now if I were to updated it to use a delta from the last injected
timestamp, where injecting a timestamp only happens on overflow.

#define TS_BITS 27
#define MAX_DELTA ((1 << TS_BITS) - 1)
#define BITMASK (~MAX_DELTA)

  w = write;
again:
  ts = rdtsc();

  delta = ts & MAX_DELTA;

  if (ts - (write_stamp & BITMASK) > MAX_DELTA)
	inject_absolute = true;

  if (!try_cmpxchg(&write, w, w + length))
	goto again;

  write_stamp = ts;

I admit that it does simplify the code a little, but does it do it
enough to be worth the process of deprecating an ABI with a new one?

-- Steve