[PATCH][delayacct] Use better names in schedstats (was Re: [Patch 3/8] cpu delay collection via schedstats)

From: Balbir Singh
Date: Wed May 10 2006 - 06:27:41 EST


On Mon, May 08, 2006 at 02:26:40PM -0700, Andrew Morton wrote:
> Balbir Singh <balbir@xxxxxxxxxx> wrote:
> >
> > +/*
> > + * Expects runqueue lock to be held for atomicity of update
> > + */
> > +static inline void rq_sched_info_arrive(struct runqueue *rq,
> > + unsigned long diff)
> > +{
> > + if (rq) {
> > + rq->rq_sched_info.run_delay += diff;
> > + rq->rq_sched_info.pcnt++;
> > + }
> > +}
> > +
> > +/*
> > + * Expects runqueue lock to be held for atomicity of update
> > + */
> > +static inline void rq_sched_info_depart(struct runqueue *rq,
> > + unsigned long diff)
> > +{
> > + if (rq)
> > + rq->rq_sched_info.cpu_time += diff;
> > +}
>
> The kernel has many different units of time - jiffies, cpu ticks, ns, us,
> ms, etc. So the reader of these functions doesn't have a clue what "diff"
> is.
>
> A good way to remove all doubt in all cases is to include the units in the
> variable's name. Something like delta_jiffies, perhaps.

Hi, Andrew

I have renamed all the "diff" to "delta_jiffies" to make it easier to
read the code as suggested in the review comments.

Balbir Singh,
Linux Technology Center,
IBM Software Labs


Changelog
1. Clean up the usage of the names. Use names with units to make the code
easier to read

Signed-off-by: Balbir Singh <balbir@xxxxxxxxxx>
---

kernel/sched.c | 28 +++++++++++++++-------------
1 files changed, 15 insertions(+), 13 deletions(-)

diff -puN kernel/sched.c~schedstats-use-better-names kernel/sched.c
--- linux-2.6.17-rc3/kernel/sched.c~schedstats-use-better-names 2006-05-10 14:48:54.000000000 +0530
+++ linux-2.6.17-rc3-balbir/kernel/sched.c 2006-05-10 14:56:09.000000000 +0530
@@ -473,10 +473,10 @@ struct file_operations proc_schedstat_op
* Expects runqueue lock to be held for atomicity of update
*/
static inline void rq_sched_info_arrive(struct runqueue *rq,
- unsigned long diff)
+ unsigned long delta_jiffies)
{
if (rq) {
- rq->rq_sched_info.run_delay += diff;
+ rq->rq_sched_info.run_delay += delta_jiffies;
rq->rq_sched_info.pcnt++;
}
}
@@ -485,17 +485,19 @@ static inline void rq_sched_info_arrive(
* Expects runqueue lock to be held for atomicity of update
*/
static inline void rq_sched_info_depart(struct runqueue *rq,
- unsigned long diff)
+ unsigned long delta_jiffies)
{
if (rq)
- rq->rq_sched_info.cpu_time += diff;
+ rq->rq_sched_info.cpu_time += delta_jiffies;
}
# define schedstat_inc(rq, field) do { (rq)->field++; } while (0)
# define schedstat_add(rq, field, amt) do { (rq)->field += (amt); } while (0)
#else /* !CONFIG_SCHEDSTATS */
-static inline void rq_sched_info_arrive(struct runqueue *rq, unsigned long diff)
+static inline void rq_sched_info_arrive(struct runqueue *rq,
+ unsigned long delta_jiffies)
{}
-static inline void rq_sched_info_depart(struct runqueue *rq, unsigned long diff)
+static inline void rq_sched_info_depart(struct runqueue *rq,
+ unsigned long delta_jiffies)
{}
# define schedstat_inc(rq, field) do { } while (0)
# define schedstat_add(rq, field, amt) do { } while (0)
@@ -544,16 +546,16 @@ static inline void sched_info_dequeued(t
*/
static void sched_info_arrive(task_t *t)
{
- unsigned long now = jiffies, diff = 0;
+ unsigned long now = jiffies, delta_jiffies = 0;

if (t->sched_info.last_queued)
- diff = now - t->sched_info.last_queued;
+ delta_jiffies = now - t->sched_info.last_queued;
sched_info_dequeued(t);
- t->sched_info.run_delay += diff;
+ t->sched_info.run_delay += delta_jiffies;
t->sched_info.last_arrival = now;
t->sched_info.pcnt++;

- rq_sched_info_arrive(task_rq(t), diff);
+ rq_sched_info_arrive(task_rq(t), delta_jiffies);
}

/*
@@ -584,10 +586,10 @@ static inline void sched_info_queued(tas
*/
static inline void sched_info_depart(task_t *t)
{
- unsigned long diff = jiffies - t->sched_info.last_arrival;
+ unsigned long delta_jiffies = jiffies - t->sched_info.last_arrival;

- t->sched_info.cpu_time += diff;
- rq_sched_info_depart(task_rq(t), diff);
+ t->sched_info.cpu_time += delta_jiffies;
+ rq_sched_info_depart(task_rq(t), delta_jiffies);
}

/*
_
-
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/