Messages in this thread | | | Subject | Re: [PATCH 05/17] perf tools: Add ordered_events_(get|put) interface | From | Namhyung Kim <> | Date | Fri, 13 Jun 2014 21:05:05 +0900 |
| |
2014-06-13 (금), 00:08 +0200, Jiri Olsa: > +#define MAX_SAMPLE_BUFFER (64 * 1024 / sizeof(struct ordered_event)) > +static struct ordered_event *alloc_event(struct ordered_events_queue *q) > +{ > + struct list_head *cache = &q->cache; > + struct ordered_event *new; > + > + if (!list_empty(cache)) { > + new = list_entry(cache->next, struct ordered_event, list); > + list_del(&new->list); > + } else if (q->buffer) { > + new = q->buffer + q->buffer_idx; > + if (++q->buffer_idx == MAX_SAMPLE_BUFFER) > + q->buffer = NULL; > + } else { > + q->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new)); > + if (!q->buffer) > + return NULL; > + list_add(&q->buffer->list, &q->to_free); > + q->buffer_idx = 2; > + new = q->buffer + 1;
Hmm.. can we add a comment that the first entry is abused to maintain the to_free list?
> + } > + > + return new; > +} > + > +static struct ordered_event* > +ordered_events_get(struct ordered_events_queue *q, u64 timestamp) > +{ > + struct ordered_event *new; > + > + new = alloc_event(q); > + if (new) { > + new->timestamp = timestamp; > + queue_event(q, new); > + } > + > + return new; > +} > + > +static void > +ordered_event_put(struct ordered_events_queue *q, struct ordered_event *iter) > +{ > + list_del(&iter->list); > + list_add(&iter->list, &q->cache);
list_move(&iter->list, &q->cache) ?
> + q->nr_events--; > +}
[SNIP] > @@ -639,29 +681,13 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event, > return -EINVAL; > } > > - if (!list_empty(cache)) { > - new = list_entry(cache->next, struct ordered_event, list); > - list_del(&new->list); > - } else if (q->buffer) { > - new = q->buffer + q->buffer_idx; > - if (++q->buffer_idx == MAX_SAMPLE_BUFFER) > - q->buffer = NULL; > - } else { > - q->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new)); > - if (!q->buffer) > - return -ENOMEM; > - list_add(&q->buffer->list, &q->to_free); > - q->buffer_idx = 2; > - new = q->buffer + 1; > + new = ordered_events_get(q, timestamp); > + if (new) { > + new->file_offset = file_offset; > + new->event = event; > }
What about make it like below:
if (!new) return -ENOMEM;
This way we can share more of the original code.
Thanks, Namhyung
> > - new->timestamp = timestamp; > - new->file_offset = file_offset; > - new->event = event; > - > - __queue_event(new, s); > - > - return 0; > + return new ? 0 : -ENOMEM; > } > > static void callchain__printf(struct perf_sample *sample)
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |