schedstats: sched_info_switch() invocation without checking (prev != next) before

schedstats: sched_info_switch() invocation without checking (prev != next) before

[Mauricio Lin]

Hi all,

I am using the schedstats idea for tracking some schedule info and
after checking the code I have a question about sched_info_switch()
function.

According to the comments as follows:

/*
 * Called when tasks are switched involuntarily due, typically, to expiring
 * their time slice.  (This may also be called when switching to or from
 * the idle task.)  We are only called when prev != next.
 */
static inline void sched_info_switch(task_t *prev, task_t *next)
{
...
}

So after reading the comments, sched_info_switch() should be called
when the current task and next task are different (prev != next), but
this function is called in the schedule() function before checking if
the current task and next task are the same or not. See below a
snippet of schedule() code:

=========================
switch_tasks:
...
sched_info_switch(prev, next);
	if (likely(prev != next)) {
		next->timestamp = now;
		rq->nr_switches++;
		rq->curr = next;
		++*switch_count;

		prepare_task_switch(rq, next);
		prev = context_switch(rq, prev, next);
		barrier();
		/*
		 * this_rq must be evaluated again because prev may have moved
		 * CPUs since it called schedule(), thus the 'rq' on its stack
		 * frame will be invalid.
		 */
		finish_task_switch(this_rq(), prev);
	} else
		spin_unlock_irq(&rq->lock);

=========================

Look that sched_info_switch() is being invoked before verifying if the
prev and next tasks are different or not. IMHO the more logical place
to put sched_info_switch() function is inside the if (likely(prev !=
next) { } block according to the comments mentioned previously.

Any comments?

BR,

Mauricio Lin.

Re: schedstats: sched_info_switch() invocation without checking (prev != next) before

[Balbir Singh]

> 
> Look that sched_info_switch() is being invoked before verifying if the
> prev and next tasks are different or not. IMHO the more logical place
> to put sched_info_switch() function is inside the if (likely(prev !=
> next) { } block according to the comments mentioned previously.
> 
> Any comments?
> 

Yes, I think your analysis seems correct. There is an advantages
to calling sched_info_switch() before checking for prev != next in
the if (likely()).

if prev == next, sched_info_switch() updates the task and runqueue statistics
information (that helps keeping it up to date). This might be useful
for SCHED_FIFO.

-- 
					<---	Balbir