linux-kernel - Re: [PATCH 05/18] perf tools: Add ordered_events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Jun 2014 12:02:21 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	David Ahern <dsahern@...il.com>
Cc:	Jiri Olsa <jolsa@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
	linux-kernel@...r.kernel.org,
	Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	Jean Pihet <jean.pihet@...aro.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 05/18] perf tools: Add ordered_events_(get|put) interface

Em Sun, Jun 29, 2014 at 10:50:24AM -0600, David Ahern escreveu:
> On 6/29/14, 10:39 AM, Jiri Olsa wrote:
> >On Fri, Jun 27, 2014 at 05:06:36PM -0600, David Ahern wrote:
> >>On 6/18/14, 8:58 AM, Jiri Olsa wrote:
> >>>+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;
> >>>+}

> >>The _get name does not really correlate with what is happening -- ie.,
> >>allocate a new event and add it to the queue. There is no reference taken
> >>either.

> >ook.. so how about ordered_events_alloc ordered_events_free

ordered_events__new() and ordered_events__delete(), to be consistent
with general naming for constructors and destructors in tools/perf/ :-)

I would also not use "new" as the name of the new instance, as above,
but would rather use 'oe', shortcut for ordered event, or even oevent,
as elsewhere suggested in this thread.

> >>>+static void
> >>>+ordered_event_put(struct ordered_events_queue *q, struct ordered_event *iter)
> >>>+{
> >>>+	list_del(&iter->list);
> >>>+	list_add(&iter->list, &q->cache);
> >>>+	q->nr_events--;
> >>>+}
> >>
> >>Similarly here with the _put. In this case the function is moving the event
> >>from one list to another. And how about something else for the name besides
> >>iter -- oe, or oevent?
> >
> >how about 'event' ?
> 
> Already a heavily used keyword in perf, that's why I was thinking oe or
> oevent -- besides it is a struct ordered_event not an event.

Agreed on all counts.
 
> The bigger thing to me with this patch is the _get/_put names.

Agreed, those are for reference counting.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/