[...] > > > + /* > > > + * Perhaps the reader page had a write that added > > > + * more data. > > > + */ > > > + kbuffer_refresh(kbuf); > > > + > > > + /* Are there still events to read? */ > > > + if (kbuffer_curr_size(kbuf)) > > > + return 1; > > > > It does not seem to be enough, only kbuf->size is updated in kbuffer_refresh() > > while kbuffer_curr_size is next - cur. > > That's the size of the event, not what's on the buffer. Next just points to > the end of the current event. Hmm, or you mean that we read the last event > and something else was added? Yeah, should check if curr == size, then next > would need to be updated. That's a bug in kbuffer_refresh(). Yeah, probably kbuffer_refresh should also update kbuf->next and not only kbuf->size. > > > > > > + > > > + /* See if a new page is ready? */ > > > + if (ioctl(tmap->fd, TRACE_MMAP_IOCTL_GET_READER) < 0) > > > + return -1; > > > > Maybe this ioctl should be called regardless if events are found on the current > > reader page. This would at least update the reader->read field and make sure > > subsequent readers are not getting the same events we already had here? > > If we call the ioctl() before we are finished reading events, the events on > the reader page will be discarded and added back to the writer buffer if > the writer is not on the reader page. Hum, I am a bit confused here. If the writer is not on the reader page, then there's no way a writer could overwrite these events while we access them? > > And there should only be one reader accessing the map. This is not thread > safe. Once you load the subbuffer into kbuf, kbuf handles what was read. We > don't want to use the reader page for that. I had in mind a scenario with two sequential read. In that case only the reader page read could help to "save" what has been read so far. Currently, we have: <event A> ./cpu-map print event A <event B> ./cpu-map print event A print event B > > If something is reading the buffer outside this application, it's fine if > we read the same events. Multiple readers of the same buffer already screw > things up today. That's why I created instances. > > -- Steve > > > > > > > + id = tmap->map->reader.id; > > > + data = tmap->data + tmap->map->subbuf_size * id; > > > + > > > > [...] >
|