sched: Update scheduler stat documentation.

PATCH In review Rakib Mullick 2 participants 7 messages Jan 12, 2012 – Jan 17, 2012

Subsystems (2)

Documentation Scheduler

lore

Share & tools

lore

Rakib Mullick

author

2012-01-12 17:28 UTC #0 lore raw

PATCH 1/1 sched: Update scheduler stat documentation.

/proc/schedstat's second field reflect to # of time context switch happened not # of times switched to the expired queue.

Signed-off-by Rakib Mullick <[email protected]>

---

Documentation/scheduler/sched-stats.txt +2 -1

index 1cd5d51..7d8e12c 100644
@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
      1) # of times sched_yield() was called
 
 Next three are schedule() statistics:
-     2) # of times we switched to the expired queue and reused it
+     2) # of times context switch happened. This field is not utilized these
+	days but kept for holding userspace tools integrity.
      3) # of times schedule() was called
      4) # of times schedule() left the processor idle

Ingo Molnar

2012-01-16 07:56 UTC #1 lore raw

Quoted 10 lines · Rakib Mullick

/proc/schedstat's second field reflect to # of time context switch happened not # of times switched to the expired queue.
Signed-off-by Rakib Mullick <[email protected]>
---
Documentation/scheduler/sched-stats.txt +0 -0
index 1cd5d51..7d8e12c 100644

Documentation/scheduler/sched-stats.txt First field is a sched_yield() statistic: · line 42

Quoted 6 lines

Documentation/scheduler/sched-stats.txt +2 -1

@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
      1) # of times sched_yield() was called
 
 Next three are schedule() statistics:
-     2) # of times we switched to the expired queue and reused it
+     2) # of times context switch happened. This field is not utilized these
+	days but kept for holding userspace tools integrity.

if it's not used by anything but kept for the ABI then we should not put context switch number in there, but always keep it zero, right?

Thanks,

Ingo

Rakib Mullick

author

2012-01-16 08:22 UTC #2 lore raw

Quoted 15 lines · Ingo Molnar

Quoted 12 lines · Rakib Mullick
/proc/schedstat's second field reflect to # of time context switch happened not # of times switched to the expired queue.
Signed-off-by Rakib Mullick <[email protected]>
---
Documentation/scheduler/sched-stats.txt +0 -0
index 1cd5d51..7d8e12c 100644
@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
1) # of times sched_yield() was called

Quoted 9 lines

Quoted 4 lines
Next three are schedule() statistics:
- 2) # of times we switched to the expired queue and reused it
+ 2) # of times context switch happened. This field is not utilized these
+ days but kept for holding userspace tools integrity.
if it's not used by anything but kept for the ABI then we should not put context switch number in there, but always keep it zero, right?

Yes, quite right. It is a bit confusing. Anyone might have think that context switch numbers are 0. So, instead of saying "# of times context switch happened", we can say, "This field has been kept for ABI integrity". Right?

Thanks,
Rakib

Ingo Molnar

2012-01-16 08:36 UTC #3 lore raw

Quoted 26 lines · Rakib Mullick

Quoted 15 lines · Ingo Molnar
Quoted 12 lines · Rakib Mullick
/proc/schedstat's second field reflect to # of time context switch happened not # of times switched to the expired queue.
Signed-off-by Rakib Mullick <[email protected]>
---
Documentation/scheduler/sched-stats.txt +0 -0
index 1cd5d51..7d8e12c 100644
@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
1) # of times sched_yield() was called
Quoted 9 lines
Quoted 4 lines
Next three are schedule() statistics:
- 2) # of times we switched to the expired queue and reused it
+ 2) # of times context switch happened. This field is not utilized these
+ days but kept for holding userspace tools integrity.
if it's not used by anything but kept for the ABI then we should not put context switch number in there, but always keep it zero, right?

Quoted 4 lines

Yes, quite right. It is a bit confusing. Anyone might have think that context switch numbers are 0. So, instead of saying "# of times context switch happened", we can say, "This field has been kept for ABI integrity". Right?

Yes - but we should also change it to export a value of zero. The field is a legacy 'array expirations' field, and as such it should be zero.

Thanks,

Ingo

Rakib Mullick

author

2012-01-16 16:51 UTC #4 lore raw

Quoted 6 lines · Ingo Molnar

Yes - but we should also change it to export a value of zero. The field is a legacy 'array expirations' field, and as such it should be zero.

Ok, thanks for suggestions. Please, look at the following patch, it's been roughly created to address your suggestion. If it looks okay, then I can send formal patch (perhaps two separate patches?). In this way, I think we can also remove rq->sched_switch field (not done in this patch)?

(quoted diff) +0 -0

b/Documentation/scheduler/sched-stats.txt
index 1cd5d51..cc2d107 100644

Documentation/scheduler/sched-stats.txt +2 -1

@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
      1) # of times sched_yield() was called
 
 Next three are schedule() statistics:
-     2) # of times we switched to the expired queue and reused it
+     2) This field is a legacy array expiration field used in O(1) scheduler.
+	But still kept for ABI integrity.
      3) # of times schedule() was called
      4) # of times schedule() left the processor idle

kernel/sched/stats.c +2 -1

index 2a581ba..01a52d9 100644
@@ -25,6 +25,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
 	seq_printf(seq, "timestamp %lu\n", jiffies);
 	for_each_online_cpu(cpu) {
 		struct rq *rq = cpu_rq(cpu);
+		int rq_sched_switch = 0;
 #ifdef CONFIG_SMP
 		struct sched_domain *sd;
 		int dcount = 0;

@@ -34,7 +35,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
 		seq_printf(seq,
 		    "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_sched_switch, rq->sched_count, rq->sched_goidle,
 		    rq->ttwu_count, rq->ttwu_local,
 		    rq->rq_cpu_time,
 		    rq->rq_sched_info.run_delay, rq->rq_sched_info.pcount);

Ingo Molnar

2012-01-17 09:51 UTC #5 lore raw

Quoted 20 lines · Rakib Mullick

Quoted 6 lines · Ingo Molnar
Yes - but we should also change it to export a value of zero. The field is a legacy 'array expirations' field, and as such it should be zero.
Ok, thanks for suggestions. Please, look at the following patch, it's been roughly created to address your suggestion. If it looks okay, then I can send formal patch (perhaps two separate patches?). In this way, I think we can also remove rq->sched_switch field (not done in this patch)?
Documentation/scheduler/sched-stats.txt +0 -0
index 1cd5d51..cc2d107 100644

Documentation/scheduler/sched-stats.txt First field is a sched_yield() statistic: · line 42

Quoted 6 lines

Documentation/scheduler/sched-stats.txt +2 -1

@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
      1) # of times sched_yield() was called
 
 Next three are schedule() statistics:
-     2) # of times we switched to the expired queue and reused it
+     2) This field is a legacy array expiration field used in O(1) scheduler.
+	But still kept for ABI integrity.

I'd formulate it like this:

Quoted 10 lines

Documentation/scheduler/sched-stats.txt +4 -0

+     2) This field is a legacy array expiration count field
+        used in the O(1) scheduler. We kept it for ABI
+        compatibility, but it is always set to zero.
 
 
+		int rq_sched_switch = 0;
 #ifdef CONFIG_SMP
 		struct sched_domain *sd;
 		int dcount = 0;

Documentation/scheduler/sched-stats.txt static int show_schedstat(struct seq_file *seq, void *v) · line 38

Quoted 5 lines

Documentation/scheduler/sched-stats.txt +1 -1

@@ -34,7 +35,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
 		seq_printf(seq,
 		    "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_sched_switch, rq->sched_count, rq->sched_goidle,

printf can print a 0 value just fine as part of the format string ... :-)

But yeah, this logic is what i had in mind.

Thanks,

Ingo

Rakib Mullick

author

2012-01-17 15:33 UTC #6 lore raw

Quoted 25 lines · Ingo Molnar

Quoted 22 lines · Rakib Mullick
Quoted 6 lines · Ingo Molnar
Yes - but we should also change it to export a value of zero. The field is a legacy 'array expirations' field, and as such it should be zero.
Ok, thanks for suggestions. Please, look at the following patch, it's been roughly created to address your suggestion. If it looks okay, then I can send formal patch (perhaps two separate patches?). In this way, I think we can also remove rq->sched_switch field (not done in this patch)?
Documentation/scheduler/sched-stats.txt +0 -0
index 1cd5d51..cc2d107 100644
@@ -38,7 +38,8 @@ First field is a sched_yield() statistic:
1) # of times sched_yield() was called

Quoted 11 lines

Quoted 4 lines
Next three are schedule() statistics:
- 2) # of times we switched to the expired queue and reused it
+ 2) This field is a legacy array expiration field used in O(1) scheduler.
+ But still kept for ABI integrity.
I'd formulate it like this:
Quoted 3 lines
+ 2) This field is a legacy array expiration count field
+ used in the O(1) scheduler. We kept it for ABI
+ compatibility, but it is always set to zero.

Okay, I'll take the above description.

Quoted 15 lines

Documentation/scheduler/sched-stats.txt static int show_schedstat(struct seq_file *seq, void *v) · line 38
Quoted 11 lines
+ int rq_sched_switch = 0;
#ifdef CONFIG_SMP
struct sched_domain *sd;
int dcount = 0;
Documentation/scheduler/sched-stats.txt +1 -1
@@ -34,7 +35,7 @@ static int show_schedstat(struct seq_file *seq, void *v)
               seq_printf(seq,
                   "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_sched_switch, rq->sched_count, rq->sched_goidle,
printf can print a 0 value just fine as part of the format string ... :-)

Umm... actually, I thought using a variable would have been nice to indicate it's purpose rather straightly putting 0. Anyway, will change it.

Quoted 2 lines

But yeah, this logic is what i had in mind.

I was confused what you asked for cause present code returns 0 too. Now got it, will send a patch shortly.

Thanks,
Rakib

Quoted 3 lines

Thanks,
Ingo