Found two minor bugs while doing development on the ring buffer code.

The first is something that's been there since its creation. If a reader
 reads a page out of the ring buffer before there's any events on it, it
 can get an out of date timestamp for that event. It may be off by a few
 microseconds, more if the first event gets discarded. The fix was to
 only update the reader time stamp when it actually sees an event on
 the page, instead of just reading the timestamp from the page even if
 it has no events on it. That timestamp is still volatile until an event
 is present.
 
 The second bug is more recent. Instead of passing around parameters
 a descriptor was made and the parameters are passed via a single
 descriptor. This simplified the code a bit. But there was one place that
 expected the parameter to be passed by value not reference (which a
 descriptor now does). And it added to the length of the event, which
 may be ignored later, but the length should not have been increased.
 The only real problem with this bug is that it may allocate more than
 was needed for the event.
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1
 
 iQEcBAABAgAGBQJWVdKDAAoJEKKk/i67LK/8dYwH/15HYLvVMbljmXyatXOp+zO9
 VTJAfXuLowMNLWMEg57NjDkcmqKodQfVWEY1gW8dgY3VnUa6KbbWgcX++3ncnXtH
 RwRQ8YhgTCpz9rFmIKesSBuLpu0uE1naqGn9QSF0AaU48ljykDStXBJJs7QGBCB8
 ZDFru17sFT5/BpeE/zcTrDKmUH79YSWbmnQvcp4UxdP3Eq7THvd41bOEoUrvm/zU
 pA2I/+Yz8wUJMA9I4UHEkltoMSV46/QldXbSh+zzWoTdZp92ZAiyuNNO/EtamGa+
 QVS6cU3b9N9C6EGom21PaSheNUpZDrZf5Tj0S0wrg2f/+lWublplRJNzwhSu85E=
 =K6x5
 -----END PGP SIGNATURE-----

Merge tag 'trace-v4.4-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace

Pull tracing fixes from Steven Rostedt:
 "I found two minor bugs while doing development on the ring buffer
  code.

  The first is something that's been there since its creation.  If a
  reader reads a page out of the ring buffer before there's any events
  on it, it can get an out of date timestamp for that event.  It may be
  off by a few microseconds, more if the first event gets discarded.
  The fix was to only update the reader time stamp when it actually sees
  an event on the page, instead of just reading the timestamp from the
  page even if it has no events on it.  That timestamp is still volatile
  until an event is present.

  The second bug is more recent.  Instead of passing around parameters a
  descriptor was made and the parameters are passed via a single
  descriptor.  This simplified the code a bit.  But there was one place
  that expected the parameter to be passed by value not reference (which
  a descriptor now does).  And it added to the length of the event,
  which may be ignored later, but the length should not have been
  increased.  The only real problem with this bug is that it may
  allocate more than was needed for the event"

* tag 'trace-v4.4-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
  ring-buffer: Put back the length if crossed page with add_timestamp
  ring-buffer: Update read stamp with first real commit on page
This commit is contained in:
Linus Torvalds 2015-11-30 15:38:23 -08:00
commit 9e5d25e829

View file

@ -1887,12 +1887,6 @@ rb_event_index(struct ring_buffer_event *event)
return (addr & ~PAGE_MASK) - BUF_PAGE_HDR_SIZE;
}
static void rb_reset_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
{
cpu_buffer->read_stamp = cpu_buffer->reader_page->page->time_stamp;
cpu_buffer->reader_page->read = 0;
}
static void rb_inc_iter(struct ring_buffer_iter *iter)
{
struct ring_buffer_per_cpu *cpu_buffer = iter->cpu_buffer;
@ -2803,8 +2797,11 @@ rb_reserve_next_event(struct ring_buffer *buffer,
event = __rb_reserve_next(cpu_buffer, &info);
if (unlikely(PTR_ERR(event) == -EAGAIN))
if (unlikely(PTR_ERR(event) == -EAGAIN)) {
if (info.add_timestamp)
info.length -= RB_LEN_TIME_EXTEND;
goto again;
}
if (!event)
goto out_fail;
@ -3626,7 +3623,7 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
/* Finally update the reader page to the new head */
cpu_buffer->reader_page = reader;
rb_reset_reader_page(cpu_buffer);
cpu_buffer->reader_page->read = 0;
if (overwrite != cpu_buffer->last_overrun) {
cpu_buffer->lost_events = overwrite - cpu_buffer->last_overrun;
@ -3636,6 +3633,10 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
goto again;
out:
/* Update the read_stamp on the first event */
if (reader && reader->read == 0)
cpu_buffer->read_stamp = reader->page->time_stamp;
arch_spin_unlock(&cpu_buffer->lock);
local_irq_restore(flags);