Re: [PATCH v9 5/8] perf record: Toggle overwrite ring buffer for reading

From: Wangnan (F)
Date: Wed Jun 22 2016 - 21:20:01 EST




On 2016/6/22 22:33, Nilay Vaish wrote:
On 22 June 2016 at 04:08, Wang Nan <wangnan0@xxxxxxxxxx> wrote:
@@ -549,17 +573,72 @@ static struct perf_event_header finished_round_event = {
.type = PERF_RECORD_FINISHED_ROUND,
};

-static int record__mmap_read_all(struct record *rec)
+static void
+record__toggle_overwrite_evsels(struct record *rec,
+ enum overwrite_evt_state state)
+{
+ struct perf_evlist *evlist = rec->overwrite_evlist;
+ enum overwrite_evt_state old_state = rec->overwrite_evt_state;
+ enum action {
+ NONE,
+ PAUSE,
+ RESUME,
+ } action = NONE;
+
+ switch (old_state) {
+ case OVERWRITE_EVT_RUNNING:
+ if (state != OVERWRITE_EVT_RUNNING)
+ action = PAUSE;
+ break;
+ case OVERWRITE_EVT_DATA_PENDING:
+ if (state == OVERWRITE_EVT_RUNNING)
+ action = RESUME;
+ break;
+ case OVERWRITE_EVT_EMPTY:
+ if (state == OVERWRITE_EVT_RUNNING)
+ action = RESUME;
+ if (state == OVERWRITE_EVT_DATA_PENDING)
+ state = OVERWRITE_EVT_EMPTY;
else if (state == OVERWRITE_EVT_DATA_PENDING)

You are right, but I believe compiler makes identical binaries even
without 'else'. With no 'else' these two 'if' are aligned.

I'll recheck this patch.

+ break;
+ default:
+ WARN_ONCE(1, "Shouldn't get there\n");
+ }
+
+ rec->overwrite_evt_state = state;
+
+ if (action == NONE)
+ return;
I think the above two lines are not required. The switch below should
be enough.

+
+ if (!evlist)
+ return;
+
+ switch (action) {
+ case PAUSE:
+ perf_evlist__pause(evlist);
+ break;
+ case RESUME:
+ perf_evlist__resume(evlist);
+ break;
+ case NONE:
+ default:
+ break;
+ }
+}
+
--
Nilay