* [PATCH] use atomic_t type for system wide credit scheduler statistics
@ 2009-03-09 7:46 Yang, Xiaowei
2009-03-09 8:09 ` Keir Fraser
0 siblings, 1 reply; 5+ messages in thread
From: Yang, Xiaowei @ 2009-03-09 7:46 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 74 bytes --]
Current code is not SMP safe. Use atomic_t type instead.
Thanks,
xiaowei
[-- Attachment #2: stats.patch --]
[-- Type: text/x-patch, Size: 2248 bytes --]
diff -r 5b9f282679ae -r 7b843d53b3ea xen/common/sched_credit.c
--- a/xen/common/sched_credit.c Mon Mar 09 00:09:52 2009 +0800
+++ b/xen/common/sched_credit.c Mon Mar 09 01:26:09 2009 +0800
@@ -79,12 +79,17 @@
*/
#ifdef CSCHED_STATS
-#define CSCHED_STAT(_X) (csched_priv.stats._X)
-#define CSCHED_STAT_DEFINE(_X) uint32_t _X;
+#define CSCHED_STAT(_X) (atomic_read(&csched_priv.stats._X))
+#define CSCHED_STAT_DEFINE(_X) atomic_t _X;
#define CSCHED_STAT_PRINTK(_X) \
do \
{ \
- printk("\t%-30s = %u\n", #_X, CSCHED_STAT(_X)); \
+ printk("\t%-30s = %i\n", #_X, CSCHED_STAT(_X)); \
+ } while ( 0 );
+#define CSCHED_STAT_RESET(_X) \
+ do \
+ { \
+ atomic_set(&csched_priv.stats._X, 0); \
} while ( 0 );
/*
@@ -137,12 +142,6 @@
CSCHED_STATS_EXPAND_CHECKS(_MACRO) \
CSCHED_STATS_EXPAND_SCHED(_MACRO)
-#define CSCHED_STATS_RESET() \
- do \
- { \
- memset(&csched_priv.stats, 0, sizeof(csched_priv.stats)); \
- } while ( 0 )
-
#define CSCHED_STATS_DEFINE() \
struct \
{ \
@@ -156,7 +155,13 @@
CSCHED_STATS_EXPAND(CSCHED_STAT_PRINTK) \
} while ( 0 )
-#define CSCHED_STAT_CRANK(_X) (CSCHED_STAT(_X)++)
+#define CSCHED_STATS_RESET() \
+ do \
+ { \
+ CSCHED_STATS_EXPAND(CSCHED_STAT_RESET) \
+ } while ( 0 )
+
+#define CSCHED_STAT_CRANK(_X) (atomic_inc(&csched_priv.stats._X))
#define CSCHED_VCPU_STATS_RESET(_V) \
do \
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] use atomic_t type for system wide credit scheduler statistics
2009-03-09 7:46 [PATCH] use atomic_t type for system wide credit scheduler statistics Yang, Xiaowei
@ 2009-03-09 8:09 ` Keir Fraser
2009-03-09 8:30 ` Yang, Xiaowei
0 siblings, 1 reply; 5+ messages in thread
From: Keir Fraser @ 2009-03-09 8:09 UTC (permalink / raw)
To: Yang, Xiaowei, xen-devel@lists.xensource.com
Not while csched stats are enabled by default. They get cranked all over the
place. How much does this race matter? We probably just lose a few
increments?
-- Keir
On 09/03/2009 07:46, "Yang, Xiaowei" <xiaowei.yang@intel.com> wrote:
> Current code is not SMP safe. Use atomic_t type instead.
>
> Thanks,
> xiaowei
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] use atomic_t type for system wide credit scheduler statistics
2009-03-09 8:09 ` Keir Fraser
@ 2009-03-09 8:30 ` Yang, Xiaowei
2009-03-09 8:36 ` Keir Fraser
0 siblings, 1 reply; 5+ messages in thread
From: Yang, Xiaowei @ 2009-03-09 8:30 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel@lists.xensource.com
Keir Fraser wrote:
> Not while csched stats are enabled by default. They get cranked all over the
> place. How much does this race matter? We probably just lose a few
> increments?
Yes. The higher frequency the count is updated, the more increments lost
can we see - comparing to atomic version, counts like "vcpu_check" and
"scheduele" lose increments very obviously due to race.
Thanks,
xiaowei
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] use atomic_t type for system wide credit scheduler statistics
2009-03-09 8:30 ` Yang, Xiaowei
@ 2009-03-09 8:36 ` Keir Fraser
2009-03-09 10:04 ` Yang, Xiaowei
0 siblings, 1 reply; 5+ messages in thread
From: Keir Fraser @ 2009-03-09 8:36 UTC (permalink / raw)
To: Yang, Xiaowei; +Cc: xen-devel@lists.xensource.com
On 09/03/2009 08:30, "Yang, Xiaowei" <xiaowei.yang@intel.com> wrote:
> Keir Fraser wrote:
>> Not while csched stats are enabled by default. They get cranked all over the
>> place. How much does this race matter? We probably just lose a few
>> increments?
>
> Yes. The higher frequency the count is updated, the more increments lost
> can we see - comparing to atomic version, counts like "vcpu_check" and
> "scheduele" lose increments very obviously due to race.
The best thing to do would be to move the stats into perfc_defn.h, don't you
think? The CSCHED_* macro wrappers could be kept but wrap the existing
common mechanism for perf counters.
-- Keir
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] use atomic_t type for system wide credit scheduler statistics
2009-03-09 8:36 ` Keir Fraser
@ 2009-03-09 10:04 ` Yang, Xiaowei
0 siblings, 0 replies; 5+ messages in thread
From: Yang, Xiaowei @ 2009-03-09 10:04 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 775 bytes --]
Keir Fraser wrote:
> On 09/03/2009 08:30, "Yang, Xiaowei" <xiaowei.yang@intel.com> wrote:
>
>> Keir Fraser wrote:
>>> Not while csched stats are enabled by default. They get cranked all over the
>>> place. How much does this race matter? We probably just lose a few
>>> increments?
>> Yes. The higher frequency the count is updated, the more increments lost
>> can we see - comparing to atomic version, counts like "vcpu_check" and
>> "scheduele" lose increments very obviously due to race.
>
> The best thing to do would be to move the stats into perfc_defn.h, don't you
> think? The CSCHED_* macro wrappers could be kept but wrap the existing
> common mechanism for perf counters.
>
Agree. updated version attached.
Signed-off-by: Xiaowei Yang <xiaowei.yang@intel.com>
[-- Attachment #2: stats2.patch --]
[-- Type: text/x-patch, Size: 7539 bytes --]
diff -r 227228b7dce4 xen/common/sched_credit.c
--- a/xen/common/sched_credit.c Mon Mar 09 03:20:57 2009 +0800
+++ b/xen/common/sched_credit.c Mon Mar 09 04:08:51 2009 +0800
@@ -25,7 +25,7 @@
/*
* CSCHED_STATS
*
- * Manage very basic counters and stats.
+ * Manage very basic per-vCPU counters and stats.
*
* Useful for debugging live systems. The stats are displayed
* with runq dumps ('r' on the Xen console).
@@ -77,85 +77,9 @@
/*
* Stats
*/
+#define CSCHED_STAT_CRANK(_X) (perfc_incr(_X))
+
#ifdef CSCHED_STATS
-
-#define CSCHED_STAT(_X) (csched_priv.stats._X)
-#define CSCHED_STAT_DEFINE(_X) uint32_t _X;
-#define CSCHED_STAT_PRINTK(_X) \
- do \
- { \
- printk("\t%-30s = %u\n", #_X, CSCHED_STAT(_X)); \
- } while ( 0 );
-
-/*
- * Try and keep often cranked stats on top so they'll fit on one
- * cache line.
- */
-#define CSCHED_STATS_EXPAND_SCHED(_MACRO) \
- _MACRO(schedule) \
- _MACRO(acct_run) \
- _MACRO(acct_no_work) \
- _MACRO(acct_balance) \
- _MACRO(acct_reorder) \
- _MACRO(acct_min_credit) \
- _MACRO(acct_vcpu_active) \
- _MACRO(acct_vcpu_idle) \
- _MACRO(vcpu_sleep) \
- _MACRO(vcpu_wake_running) \
- _MACRO(vcpu_wake_onrunq) \
- _MACRO(vcpu_wake_runnable) \
- _MACRO(vcpu_wake_not_runnable) \
- _MACRO(vcpu_park) \
- _MACRO(vcpu_unpark) \
- _MACRO(tickle_local_idler) \
- _MACRO(tickle_local_over) \
- _MACRO(tickle_local_under) \
- _MACRO(tickle_local_other) \
- _MACRO(tickle_idlers_none) \
- _MACRO(tickle_idlers_some) \
- _MACRO(load_balance_idle) \
- _MACRO(load_balance_over) \
- _MACRO(load_balance_other) \
- _MACRO(steal_trylock_failed) \
- _MACRO(steal_peer_idle) \
- _MACRO(migrate_queued) \
- _MACRO(migrate_running) \
- _MACRO(dom_init) \
- _MACRO(dom_destroy) \
- _MACRO(vcpu_init) \
- _MACRO(vcpu_destroy)
-
-#ifndef NDEBUG
-#define CSCHED_STATS_EXPAND_CHECKS(_MACRO) \
- _MACRO(vcpu_check)
-#else
-#define CSCHED_STATS_EXPAND_CHECKS(_MACRO)
-#endif
-
-#define CSCHED_STATS_EXPAND(_MACRO) \
- CSCHED_STATS_EXPAND_CHECKS(_MACRO) \
- CSCHED_STATS_EXPAND_SCHED(_MACRO)
-
-#define CSCHED_STATS_RESET() \
- do \
- { \
- memset(&csched_priv.stats, 0, sizeof(csched_priv.stats)); \
- } while ( 0 )
-
-#define CSCHED_STATS_DEFINE() \
- struct \
- { \
- CSCHED_STATS_EXPAND(CSCHED_STAT_DEFINE) \
- } stats;
-
-#define CSCHED_STATS_PRINTK() \
- do \
- { \
- printk("stats:\n"); \
- CSCHED_STATS_EXPAND(CSCHED_STAT_PRINTK) \
- } while ( 0 )
-
-#define CSCHED_STAT_CRANK(_X) (CSCHED_STAT(_X)++)
#define CSCHED_VCPU_STATS_RESET(_V) \
do \
@@ -170,10 +93,6 @@
#else /* CSCHED_STATS */
-#define CSCHED_STATS_RESET() do {} while ( 0 )
-#define CSCHED_STATS_DEFINE()
-#define CSCHED_STATS_PRINTK() do {} while ( 0 )
-#define CSCHED_STAT_CRANK(_X) do {} while ( 0 )
#define CSCHED_VCPU_STATS_RESET(_V) do {} while ( 0 )
#define CSCHED_VCPU_STAT_CRANK(_V, _X) do {} while ( 0 )
#define CSCHED_VCPU_STAT_SET(_V, _X, _Y) do {} while ( 0 )
@@ -239,7 +158,6 @@ struct csched_private {
uint32_t credit;
int credit_balance;
uint32_t runq_sort;
- CSCHED_STATS_DEFINE()
};
@@ -1331,8 +1249,6 @@ csched_dump(void)
cpumask_scnprintf(idlers_buf, sizeof(idlers_buf), csched_priv.idlers);
printk("idlers: %s\n", idlers_buf);
- CSCHED_STATS_PRINTK();
-
printk("active vcpus:\n");
loop = 0;
list_for_each( iter_sdom, &csched_priv.active_sdom )
@@ -1363,7 +1279,6 @@ csched_init(void)
csched_priv.credit = 0U;
csched_priv.credit_balance = 0;
csched_priv.runq_sort = 0U;
- CSCHED_STATS_RESET();
}
/* Tickers cannot be kicked until SMP subsystem is alive. */
diff -r 227228b7dce4 xen/include/xen/perfc_defn.h
--- a/xen/include/xen/perfc_defn.h Mon Mar 09 03:20:57 2009 +0800
+++ b/xen/include/xen/perfc_defn.h Mon Mar 09 04:04:01 2009 +0800
@@ -16,6 +16,40 @@ PERFCOUNTER(sched_run, "sch
PERFCOUNTER(sched_run, "sched: runs through scheduler")
PERFCOUNTER(sched_ctx, "sched: context switches")
+PERFCOUNTER(vcpu_check, "csched: vcpu_check")
+PERFCOUNTER(schedule, "csched: schedule")
+PERFCOUNTER(acct_run, "csched: acct_run")
+PERFCOUNTER(acct_no_work, "csched: acct_no_work")
+PERFCOUNTER(acct_balance, "csched: acct_balance")
+PERFCOUNTER(acct_reorder, "csched: acct_reorder")
+PERFCOUNTER(acct_min_credit, "csched: acct_min_credit")
+PERFCOUNTER(acct_vcpu_active, "csched: acct_vcpu_active")
+PERFCOUNTER(acct_vcpu_idle, "csched: acct_vcpu_idle")
+PERFCOUNTER(vcpu_sleep, "csched: vcpu_sleep")
+PERFCOUNTER(vcpu_wake_running, "csched: vcpu_wake_running")
+PERFCOUNTER(vcpu_wake_onrunq, "csched: vcpu_wake_onrunq")
+PERFCOUNTER(vcpu_wake_runnable, "csched: vcpu_wake_runnable")
+PERFCOUNTER(vcpu_wake_not_runnable, "csched: vcpu_wake_not_runnable")
+PERFCOUNTER(vcpu_park, "csched: vcpu_park")
+PERFCOUNTER(vcpu_unpark, "csched: vcpu_unpark")
+PERFCOUNTER(tickle_local_idler, "csched: tickle_local_idler")
+PERFCOUNTER(tickle_local_over, "csched: tickle_local_over")
+PERFCOUNTER(tickle_local_under, "csched: tickle_local_under")
+PERFCOUNTER(tickle_local_other, "csched: tickle_local_other")
+PERFCOUNTER(tickle_idlers_none, "csched: tickle_idlers_none")
+PERFCOUNTER(tickle_idlers_some, "csched: tickle_idlers_some")
+PERFCOUNTER(load_balance_idle, "csched: load_balance_idle")
+PERFCOUNTER(load_balance_over, "csched: load_balance_over")
+PERFCOUNTER(load_balance_other, "csched: load_balance_other")
+PERFCOUNTER(steal_trylock_failed, "csched: steal_trylock_failed")
+PERFCOUNTER(steal_peer_idle, "csched: steal_peer_idle")
+PERFCOUNTER(migrate_queued, "csched: migrate_queued")
+PERFCOUNTER(migrate_running, "csched: migrate_running")
+PERFCOUNTER(dom_init, "csched: dom_init")
+PERFCOUNTER(dom_destroy, "csched: dom_destroy")
+PERFCOUNTER(vcpu_init, "csched: vcpu_init")
+PERFCOUNTER(vcpu_destroy, "csched: vcpu_destroy")
+
PERFCOUNTER(need_flush_tlb_flush, "PG_need_flush tlb flushes")
/*#endif*/ /* __XEN_PERFC_DEFN_H__ */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-03-09 10:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-09 7:46 [PATCH] use atomic_t type for system wide credit scheduler statistics Yang, Xiaowei
2009-03-09 8:09 ` Keir Fraser
2009-03-09 8:30 ` Yang, Xiaowei
2009-03-09 8:36 ` Keir Fraser
2009-03-09 10:04 ` Yang, Xiaowei
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.