Re: 2.6.11-rc1-mm1

[Zwane Mwaikambo]

On Fri, 14 Jan 2005, Andrew Morton wrote:

> - Added the Linux Trace Toolkit (and hence relayfs).  Mainly because I
>   haven't yet taken as close a look at LTT as I should have.  Probably neither
>   have you.

Just a few things from a quick look;

- What's with all the ltt_*_bit? Please use the ones provided by the 
kernel.

- i see cpu_has_tsc, can't you use sched_clock?

- ltt_log_event isn't preempt safe

- num_cpus isn't hotplug cpu safe, and you should be using the 
for_each_online_cpu iterators

- code style, you have large hunks of code with blocks of the following 
form, you can save processor cycles by placing an if (incoming_process) 
branch earlier. This code is in _ltt_log_event, which i presume executes 
frequently.

if (event_id == LTT_EV_SCHEDCHANGE)
	incoming_process = (struct task_struct *) ((ltt_schedchange *) event_struct)->in);

if ((trace->tracing_gid == 1) && (current->egid != trace->traced_gid)) {
	if (incoming_process == NULL)
		return 0;
	else if (incoming_process->egid != trace->traced_gid)
		return 0;
}
... [ more of the same ]
if ((trace->tracing_uid == 1) && (current->euid != trace->traced_uid)) {
	if (incoming_process == NULL)
		return 0;
	else if (incoming_process->euid != trace->traced_uid)
		return 0;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/