Re: [PATCH 4/7] numa,sched: tracepoints for NUMA balancing activenodemask changes

From: Steven Rostedt
Date: Mon Jan 20 2014 - 14:06:02 EST


On Mon, 20 Jan 2014 17:52:05 +0100
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Fri, Jan 17, 2014 at 04:12:06PM -0500, riel@xxxxxxxxxx wrote:
> > From: Rik van Riel <riel@xxxxxxxxxx>
> >

> > +++ b/kernel/sched/fair.c
> > @@ -1300,10 +1300,14 @@ static void update_numa_active_node_mask(struct task_struct *p)
> > faults = numa_group->faults_from[task_faults_idx(nid, 0)] +
> > numa_group->faults_from[task_faults_idx(nid, 1)];
> > if (!node_isset(nid, numa_group->active_nodes)) {
> > - if (faults > max_faults * 4 / 10)
> > + if (faults > max_faults * 4 / 10) {
> > + trace_update_numa_active_nodes_mask(current->pid, numa_group->gid, nid, true, faults, max_faults);
>
> While I think the tracepoint hookery is smart enough to avoid evaluating
> arguments when they're disabled, it might be best to simply pass:
> current and numa_group and do the dereference in fast_assign().

It's really up to gcc to optimize it. But that said, it is more
efficient to just past the pointer and do the dereferencing in the
fast_assign(). At least it keeps any bad optimization in gcc from
infecting the tracepoint caller.

It also makes it easier to get other information if you want to later
extend that tracepoint.

Does this tracepoint always use current? If so, why bother passing it
in?

-- Steve
--
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/