Re: [PATCH] Optimize sys_times for a single thread process

[Andrew Morton <akpm@osdl.org>]

Christoph Lameter <christoph@lameter.com> wrote:
>
> Avoid taking the tasklist_lock in sys_times if the process is
> single threaded. In a NUMA system taking the tasklist_lock may
> cause a bouncing cacheline if multiple independent processes
> continually call sys_times to measure their performance.
> 
> ...
> +		if (current == next_thread(current)) {
> +			/*
> +			 * Single thread case. We do not need to scan the tasklist
> +			 * and thus can avoid the read_lock(&task_list_lock). We
> +			 * also do not need to take the siglock since we
> +			 * are the only thread in this process
> +			 */
> +			utime = cputime_add(current->signal->utime, current->utime);
> +			stime = cputime_add(current->signal->utime, current->stime);
> +			cutime = current->signal->cutime;
> +			cstime = current->signal->cstime;
> +		} else {

Well, hrm, maybe.  If this task has one sibling thread, and that thread is
in the process of exitting then (current == next_thread(current)) may
become true before that sibling thread has had a chance to dump its process
accounting info into the signal structure.

If that dumping happens prior to the __detach_pid() call then things are
probably OK (modulo memory ordering issues).  Otherwise there's a little
window where the accounting will go wrong.

Have you audited that code to ensure that the desired sequencing occurs in
all cases and that the appropriate barriers are in place?

It all looks a bit fast-and-loose.  If there are significant performance
benefits and these issues are loudly commented (they aren't at present)
then maybe-OK, I guess.
-
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/