Clocksource: Move the Hyper-V clocksource driver out of staging
Subsystems (3)
PATCH 1/1 Clocksource: Move the Hyper-V clocksource driver out of staging
Move the Hyper-V clocksource driver out of the staging area. Given the size of this driver, this driver is being built as part of the kernel.
--- arch/x86/Kconfig | 3 + drivers/clocksource/Makefile | 1 + drivers/clocksource/hv_timesource.c | 65 ++++++++++++++++++++++ drivers/staging/hv/Makefile | 2 +- drivers/staging/hv/hv_timesource.c | 102 ----------------------------------- 5 files changed, 70 insertions(+), 103 deletions(-) create mode 100644 drivers/clocksource/hv_timesource.c delete mode 100644 drivers/staging/hv/hv_timesource.c
arch/x86/Kconfig +3 -0
@@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER def_bool y depends on X86_32_NON_STANDARD +config HYPERV_CLKSRC + def_bool y + source "arch/x86/Kconfig.cpu" config HPET_TIMER
drivers/clocksource/Makefile +1 -0
@@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC) += cs5535-clockevt.o obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o +obj-$(CONFIG_HYPERV_CLKSRC) += hv_timesource.o
drivers/clocksource/hv_timesource.c new file +65 -0
65 lines not on this page · raw message
drivers/staging/hv/Makefile +1 -1
@@ -1,4 +1,4 @@ -obj-$(CONFIG_HYPERV) += hv_vmbus.o hv_timesource.o +obj-$(CONFIG_HYPERV) += hv_vmbus.o obj-$(CONFIG_HYPERV_STORAGE) += hv_storvsc.o obj-$(CONFIG_HYPERV_BLOCK) += hv_blkvsc.o obj-$(CONFIG_HYPERV_NET) += hv_netvsc.o
drivers/staging/hv/hv_timesource.c deleted +0 -102
102 lines not on this page · raw message
Signature · 1 line
1.7.4.1
arch/x86/Kconfig config X86_CYCLONE_TIMER · line 599
Quoted 8 lines · K. Y. Srinivasan
arch/x86/Kconfig +2
-0@@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER def_bool y depends on X86_32_NON_STANDARD +config HYPERV_CLKSRC + def_bool y
Errm, why do we need another random config switch for every other feature of hyperv?
arch/x86/Kconfig config X86_CYCLONE_TIMER · line 601
Quoted 2 lines
arch/x86/Kconfig +2
-0+#include <linux/clocksource.h> +#include <linux/time.h>
Why do you need time.h?
arch/x86/Kconfig config X86_CYCLONE_TIMER
Quoted 3 lines
arch/x86/Kconfig +3
-0+#include <asm/hyperv.h> +#include <asm/mshyperv.h> +#include <asm/hypervisor.h>
Can we please have one sensible asm/hyperwtf.h include for all of this ?
arch/x86/Kconfig
Quoted 9 lines
arch/x86/Kconfig +9
-0+ +static cycle_t read_hv_clock(struct clocksource *arg) +{ + cycle_t current_tick; + /* + * Read the partition counter to get the current tick count. This count + * is set to 0 when the partition is created and is incremented in + * 100 nanosecond units. + */
Please move that comment above the function. That's really irritating to read.
arch/x86/Kconfig
Quoted 7 lines
arch/x86/Kconfig +7
-0+ rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); + return current_tick; +} + +static struct clocksource hyperv_cs = { + .name = "hyperv_clocksource", + .rating = 400, /* use this when running on Hyperv*/
That tail comment is pretty useless.
arch/x86/Kconfig
Quoted 7 lines
arch/x86/Kconfig +7
-0+ .read = read_hv_clock, + .mask = CLOCKSOURCE_MASK(64), +}; + +static int __init init_hv_clocksource(void) +{ + unsigned long hv_period = 100; /* 100 ns granularity clocksource */
unsigned long period_ns = 100;
would make the horrible tail comment go away and make it obvious to the reader what the variable is for. Also please stop adding extra useless noise to local variables by adding hv_ or whatever the heck to them.
arch/x86/Kconfig
Quoted 1 line
arch/x86/Kconfig +1
-0+ u32 hv_freq;
Newline between declarations and code please.
arch/x86/Kconfig
Quoted 6 lines
arch/x86/Kconfig +6
-0+ if ((x86_hyper != &x86_hyper_ms_hyperv) || + !(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)) + return -ENODEV; + + hv_freq = NSEC_PER_SEC; + do_div(hv_freq, hv_period);
ROTFL. Do you really need runtime evaluation of 10^9/10^2 ?
#define HV_CLK_FREQ (NSEC_PER_SEC/100)
would solve that problem with 5 lines less source code and a even larger reduction of binary size.
arch/x86/Kconfig
Quoted 2 lines
arch/x86/Kconfig +2
-0+ + printk(KERN_INFO "Registering Hyper-V clock source\n");
How is that interesting ?
arch/x86/Kconfig
Quoted 2 lines
arch/x86/Kconfig +2
-0+ return clocksource_register_hz(&hyperv_cs, hv_freq); +}
And if you remove that useless stuff then the ten remaining lines should go to arch/x86/ into a file which will contain more than those ten lines. It's pretty unlikely that anything else than MSHV will reuse that code.
Thanks,
tglx
Quoted 25 lines
-----Original Message-----
From: Thomas Gleixner [mailto:[email protected]]
Sent: Tuesday, May 24, 2011 6:43 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Hank Janssen; Haiyang Zhang
Subject: Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of stagingarch/x86/Kconfig config X86_CYCLONE_TIMER · line 599
Quoted 8 lines · K. Y. Srinivasan
arch/x86/Kconfig +2
-0@@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER def_bool y depends on X86_32_NON_STANDARD +config HYPERV_CLKSRC + def_bool yErrm, why do we need another random config switch for every other feature of hyperv?
Quoted 4 lines
arch/x86/Kconfig config X86_CYCLONE_TIMER · line 601
Quoted 2 lines
arch/x86/Kconfig +2
-0+#include <linux/clocksource.h> +#include <linux/time.h>Why do you need
time.h?
For the definition of NSEC_PER_SEC
Quoted 6 lines
arch/x86/Kconfig config X86_CYCLONE_TIMER
Quoted 4 lines
arch/x86/Kconfig +3
-0+#include <asm/hyperv.h> +#include <asm/mshyperv.h> +#include <asm/hypervisor.h>Can we please have one sensible
asm/hyperwtf.hinclude for all of this ?
Well, hyperv.h has all the Hyper-V specific defines. When mshyperv.h was introduced, I tried very hard to merge it with hyperv.h and I was voted down. With your help, maybe we can merge mshyperv.h and hyperv.h. Greg may remember the arguments that kept these two related files separate.
Quoted 63 lines
arch/x86/Kconfig
Quoted 10 lines
arch/x86/Kconfig +9
-0+ +static cycle_t read_hv_clock(struct clocksource *arg) +{ + cycle_t current_tick; + /* + * Read the partition counter to get the current tick count. This count + * is set to 0 when the partition is created and is incremented in + * 100 nanosecond units. + */Please move that comment above the function. That's really irritating to read.
arch/x86/Kconfig
Quoted 7 lines
arch/x86/Kconfig +7
-0+ rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); + return current_tick; +} + +static struct clocksource hyperv_cs = { + .name = "hyperv_clocksource", + .rating = 400, /* use this when running on Hyperv*/That tail comment is pretty useless.
arch/x86/Kconfig
Quoted 7 lines
arch/x86/Kconfig +7
-0+ .read = read_hv_clock, + .mask = CLOCKSOURCE_MASK(64), +}; + +static int __init init_hv_clocksource(void) +{ + unsigned long hv_period = 100; /* 100 ns granularity clocksource */unsigned long period_ns = 100;would make the horrible tail comment go away and make it obvious to the reader what the variable is for. Also please stop adding extra useless noise to local variables by adding
hv_or whatever the heck to them.arch/x86/Kconfig
Quoted 1 line
arch/x86/Kconfig +1
-0+ u32 hv_freq;Newline between declarations and code please.
arch/x86/Kconfig
Quoted 2 lines
arch/x86/Kconfig +2
-0+ if ((x86_hyper != &x86_hyper_ms_hyperv) || + !(ms_hyperv.features &
HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))arch/x86/Kconfig
Quoted 4 lines
arch/x86/Kconfig +4
-0+ return -ENODEV; + + hv_freq = NSEC_PER_SEC; + do_div(hv_freq, hv_period);ROTFL. Do you really need runtime evaluation of 10^9/10^2 ?
#define
HV_CLK_FREQ(NSEC_PER_SEC/100)would solve that problem with 5 lines less source code and a even larger reduction of binary size.
arch/x86/Kconfig
Quoted 2 lines
arch/x86/Kconfig +2
-0+ + printk(KERN_INFO "Registering Hyper-V clock source\n");How is that interesting ?
Quoted 7 lines
arch/x86/Kconfig
Quoted 2 lines
arch/x86/Kconfig +2
-0+ return clocksource_register_hz(&hyperv_cs, hv_freq); +}And if you remove that useless stuff then the ten remaining lines should go to
arch/x86/into a file which will contain more than those ten lines. It's pretty unlikely that anything else than MSHV will reuse that code.
I like the idea of merging this code with some other file under arch/x86/. I could merge this code into mshyperv.c file that already has hyperv specific code. Who would take this patch if I were to merge this cleaned up Hyper-V clocksource code into mshyperv.c file under arch/X86/kernel/cpu.
Thomas, thank you for your patience here.
Regards,
K. Y
Quoted 5 lines · KY Srinivasan
I like the idea of merging this code with some other file under
arch/x86/. I could merge this code intomshyperv.cfile that already has hyperv specific code. Who would take this patch if I were to merge this cleaned up Hyper-V clocksource code intomshyperv.cfile underarch/X86/kernel/cpu.
x86 maintainers perhaps :)