[PATCH -tip] sched: Clean unused fields from struct rq

[PATCH -tip] sched: Clean unused fields from struct rq

[Luis Henriques]

Since they are used on in statistics and are always set to zero, the following
fields from struct rq have been removed: yld_exp_empty, yld_act_empty and
yld_both_empty.

Sched Debug version has also been incremented.

Signed-off-by: Luis Henriques <henrix@sapo.pt>
---
 kernel/sched.c       |    3 ---
 kernel/sched_debug.c |    5 +----
 kernel/sched_stats.h |    5 ++---
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 4000304..289eac2 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -646,9 +646,6 @@ struct rq {
 	/* could above be rq->cfs_rq.exec_clock + rq->rt_rq.rt_runtime ? */
 
 	/* sys_sched_yield() stats */
-	unsigned int yld_exp_empty;
-	unsigned int yld_act_empty;
-	unsigned int yld_both_empty;
 	unsigned int yld_count;
 
 	/* schedule() stats */
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index 2b1260f..58cdc5c 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -287,9 +287,6 @@ static void print_cpu(struct seq_file *m, int cpu)
 #ifdef CONFIG_SCHEDSTATS
 #define P(n) SEQ_printf(m, "  .%-30s: %d\n", #n, rq->n);
 
-	P(yld_exp_empty);
-	P(yld_act_empty);
-	P(yld_both_empty);
 	P(yld_count);
 
 	P(sched_switch);
@@ -314,7 +311,7 @@ static int sched_debug_show(struct seq_file *m, void *v)
 	u64 now = ktime_to_ns(ktime_get());
 	int cpu;
 
-	SEQ_printf(m, "Sched Debug Version: v0.08, %s %.*s\n",
+	SEQ_printf(m, "Sched Debug Version: v0.09, %s %.*s\n",
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
diff --git a/kernel/sched_stats.h b/kernel/sched_stats.h
index a8f93dd..2e7287a 100644
--- a/kernel/sched_stats.h
+++ b/kernel/sched_stats.h
@@ -26,9 +26,8 @@ static int show_schedstat(struct seq_file *seq, void *v)
 
 		/* runqueue-specific stats */
 		seq_printf(seq,
-		    "cpu%d %u %u %u %u %u %u %u %u %u %llu %llu %lu",
-		    cpu, rq->yld_both_empty,
-		    rq->yld_act_empty, rq->yld_exp_empty, rq->yld_count,
+		    "cpu%d %u %u %u %u %u %u %llu %llu %lu",
+		    cpu, rq->yld_count,
 		    rq->sched_switch, rq->sched_count, rq->sched_goidle,
 		    rq->ttwu_count, rq->ttwu_local,
 		    rq->rq_cpu_time,
-- 
1.6.2


-- 
Luis Henriques

Re: [PATCH -tip] sched: Clean unused fields from struct rq

[Gregory Haskins]

[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]

Hi Luis,

Luis Henriques wrote:
> Since they are used on in statistics and are always set to zero, the following
> fields from struct rq have been removed: yld_exp_empty, yld_act_empty and
> yld_both_empty.
>
> Sched Debug version has also been incremented.
>
> Signed-off-by: Luis Henriques <henrix@sapo.pt>
> ---
>  kernel/sched.c       |    3 ---
>  kernel/sched_debug.c |    5 +----
>  kernel/sched_stats.h |    5 ++---
>  3 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 4000304..289eac2 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -646,9 +646,6 @@ struct rq {
>  	/* could above be rq->cfs_rq.exec_clock + rq->rt_rq.rt_runtime ? */
>  
>  	/* sys_sched_yield() stats */
> -	unsigned int yld_exp_empty;
> -	unsigned int yld_act_empty;
> -	unsigned int yld_both_empty;
>  	unsigned int yld_count;
>  
>  	/* schedule() stats */
> diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
> index 2b1260f..58cdc5c 100644
> --- a/kernel/sched_debug.c
> +++ b/kernel/sched_debug.c
> @@ -287,9 +287,6 @@ static void print_cpu(struct seq_file *m, int cpu)
>  #ifdef CONFIG_SCHEDSTATS
>  #define P(n) SEQ_printf(m, "  .%-30s: %d\n", #n, rq->n);
>  
> -	P(yld_exp_empty);
> -	P(yld_act_empty);
> -	P(yld_both_empty);
>  	P(yld_count);
>  
>  	P(sched_switch);
> @@ -314,7 +311,7 @@ static int sched_debug_show(struct seq_file *m, void *v)
>  	u64 now = ktime_to_ns(ktime_get());
>  	int cpu;
>  
> -	SEQ_printf(m, "Sched Debug Version: v0.08, %s %.*s\n",
> +	SEQ_printf(m, "Sched Debug Version: v0.09, %s %.*s\n",
>   

I actually key off the SCHEDSTAT_VERSION (currently 14) at the top of
kernel/sched_stats.h.  Cursory inspection of the existing code and your
patch says that this should actually be the value that needs updating
(vs. vSched-Debug).  However, note that I am not sure if there are also
users of sched-debug that need to see that version bump as well in order
to remain compatible.

If you are really motivated, you can test your patches against my
schedtop tool to verify sane behavior (for bumping SCHEDSTAT_VERSION to
15, I would expect it to gracefully report that it cannot handle this
version and exit).  You can find both source and rpms here:

http://rt.wiki.kernel.org/index.php/Schedtop_utility

Thanks!
-Greg



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH -tip] sched: Clean unused fields from struct rq

[Luis Henriques]

Hi Greg,

On Wed, Mar 18, 2009 at 05:00:06PM -0400, Gregory Haskins wrote:
> I actually key off the SCHEDSTAT_VERSION (currently 14) at the top of
> kernel/sched_stats.h.  Cursory inspection of the existing code and your
> patch says that this should actually be the value that needs updating
> (vs. vSched-Debug).  However, note that I am not sure if there are also
> users of sched-debug that need to see that version bump as well in order
> to remain compatible.

Hmm... so, what's the relation between SCHEDSTAT_VERSION and the "Sched Debug"
version I update in my patch?  Assuming there is a relation... :-)

I believe that I actually need to update both versioning mechanims but I think
this is something odd -- either there is one mechanism to handle version or a
comment should clarify why there are two.

> If you are really motivated, you can test your patches against my
> schedtop tool to verify sane behavior (for bumping SCHEDSTAT_VERSION to
> 15, I would expect it to gracefully report that it cannot handle this
> version and exit).  You can find both source and rpms here:

Sure I will give it a try -- I'll let you know about the result.

Thanks!
-- 
Luis Henriques

Re: [PATCH -tip] sched: Clean unused fields from struct rq

[Luis Henriques]

Hi,
On Wed, Mar 18, 2009 at 05:00:06PM -0400, Gregory Haskins wrote:
> I actually key off the SCHEDSTAT_VERSION (currently 14) at the top of
> kernel/sched_stats.h.  Cursory inspection of the existing code and your
> patch says that this should actually be the value that needs updating
> (vs. vSched-Debug).  However, note that I am not sure if there are also
> users of sched-debug that need to see that version bump as well in order
> to remain compatible.

Just realised something while taking a look at you're schedtop: you are
referring to sched_stats, which shows data in /proc/schedstat, while my patch
actually targets sched_debug, which shows data in /proc/sched_debug.  And I
guess this is why we have 2 different versioning mechanisms:  they refer to
different things :-)

So, if this is correct, my last patch actually updates the correct version,
right?

Regards,
-- 
Luis Henriques

Re: [PATCH -tip] sched: Clean unused fields from struct rq

[Luis Henriques]

On Wed, Mar 18, 2009 at 09:52:24PM +0000, Luis Henriques wrote:
> Hi,
> On Wed, Mar 18, 2009 at 05:00:06PM -0400, Gregory Haskins wrote:
> > I actually key off the SCHEDSTAT_VERSION (currently 14) at the top of
> > kernel/sched_stats.h.  Cursory inspection of the existing code and your
> > patch says that this should actually be the value that needs updating
> > (vs. vSched-Debug).  However, note that I am not sure if there are also
> > users of sched-debug that need to see that version bump as well in order
> > to remain compatible.
> 
> Just realised something while taking a look at you're schedtop: you are
> referring to sched_stats, which shows data in /proc/schedstat, while my patch
> actually targets sched_debug, which shows data in /proc/sched_debug.  And I
> guess this is why we have 2 different versioning mechanisms:  they refer to
> different things :-)
> 
> So, if this is correct, my last patch actually updates the correct version,
> right?

Gah!  Wrong again!

My patch touches both sched_debug and sched_stats, so I believe I need to change
both versions.

-- 
Luis Henriques