| Message ID | 20190204201854.2328-3-nitesh@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Guest Free Page Hinting | expand |
On Mon, Feb 4, 2019 at 2:11 PM Nitesh Narayan Lal <nitesh@redhat.com> wrote: > > This patch enables the guest free page hinting support > to enable or disable based on the STATIC key which > could be set via sysctl. > > Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> > --- > include/linux/gfp.h | 2 ++ > include/linux/page_hinting.h | 5 +++++ > kernel/sysctl.c | 9 +++++++++ > virt/kvm/page_hinting.c | 23 +++++++++++++++++++++++ > 4 files changed, 39 insertions(+) > > diff --git a/include/linux/gfp.h b/include/linux/gfp.h > index e596527284ba..8389219a076a 100644 > --- a/include/linux/gfp.h > +++ b/include/linux/gfp.h > @@ -461,6 +461,8 @@ static inline struct zonelist *node_zonelist(int nid, gfp_t flags) > #define HAVE_ARCH_FREE_PAGE > static inline void arch_free_page(struct page *page, int order) > { > + if (!static_branch_unlikely(&guest_page_hinting_key)) > + return; > guest_free_page(page, order); > } > #endif > diff --git a/include/linux/page_hinting.h b/include/linux/page_hinting.h > index b54f7428f348..9bdcf63e1306 100644 > --- a/include/linux/page_hinting.h > +++ b/include/linux/page_hinting.h > @@ -14,4 +14,9 @@ struct hypervisor_pages { > unsigned int order; > }; > > +extern int guest_page_hinting_flag; > +extern struct static_key_false guest_page_hinting_key; > + > +int guest_page_hinting_sysctl(struct ctl_table *table, int write, > + void __user *buffer, size_t *lenp, loff_t *ppos); > void guest_free_page(struct page *page, int order); > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index ba4d9e85feb8..5d53629c9bfb 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1690,6 +1690,15 @@ static struct ctl_table vm_table[] = { > .extra1 = (void *)&mmap_rnd_compat_bits_min, > .extra2 = (void *)&mmap_rnd_compat_bits_max, > }, > +#endif > +#ifdef CONFIG_KVM_FREE_PAGE_HINTING > + { > + .procname = "guest-page-hinting", > + .data = &guest_page_hinting_flag, > + .maxlen = sizeof(guest_page_hinting_flag), > + .mode = 0644, > + .proc_handler = guest_page_hinting_sysctl, > + }, > #endif > { } > }; Since you are adding a new sysctl shouldn't you also be updating Documentation/sysctl/vm.txt? > diff --git a/virt/kvm/page_hinting.c b/virt/kvm/page_hinting.c > index 818bd6b84e0c..4a34ea8db0c8 100644 > --- a/virt/kvm/page_hinting.c > +++ b/virt/kvm/page_hinting.c > @@ -1,6 +1,7 @@ > #include <linux/gfp.h> > #include <linux/mm.h> > #include <linux/kernel.h> > +#include <linux/kvm_host.h> > > /* > * struct kvm_free_pages - Tracks the pages which are freed by the guest. > @@ -31,6 +32,28 @@ struct page_hinting { > > DEFINE_PER_CPU(struct page_hinting, hinting_obj); > > +struct static_key_false guest_page_hinting_key = STATIC_KEY_FALSE_INIT; > +EXPORT_SYMBOL(guest_page_hinting_key); > +static DEFINE_MUTEX(hinting_mutex); > +int guest_page_hinting_flag; > +EXPORT_SYMBOL(guest_page_hinting_flag); I'm not entirely sure this flag makes sense to me. What is to prevent someone from turning this on when there is no means of actually using the hints. I understand right now that guest_free_page doesn't actually do anything, but when it does I would assume it has to interact with a device. If that device is not present would it still make sense for us to be generating hints? > + > +int guest_page_hinting_sysctl(struct ctl_table *table, int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos) > +{ > + int ret; > + > + mutex_lock(&hinting_mutex); > + ret = proc_dointvec(table, write, buffer, lenp, ppos); > + if (guest_page_hinting_flag) > + static_key_enable(&guest_page_hinting_key.key); > + else > + static_key_disable(&guest_page_hinting_key.key); > + mutex_unlock(&hinting_mutex); > + return ret; > +} > + > void guest_free_page(struct page *page, int order) > { > } > -- > 2.17.2 >
On 2/8/19 1:07 PM, Alexander Duyck wrote: > On Mon, Feb 4, 2019 at 2:11 PM Nitesh Narayan Lal <nitesh@redhat.com> wrote: >> This patch enables the guest free page hinting support >> to enable or disable based on the STATIC key which >> could be set via sysctl. >> >> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> >> --- >> include/linux/gfp.h | 2 ++ >> include/linux/page_hinting.h | 5 +++++ >> kernel/sysctl.c | 9 +++++++++ >> virt/kvm/page_hinting.c | 23 +++++++++++++++++++++++ >> 4 files changed, 39 insertions(+) >> >> diff --git a/include/linux/gfp.h b/include/linux/gfp.h >> index e596527284ba..8389219a076a 100644 >> --- a/include/linux/gfp.h >> +++ b/include/linux/gfp.h >> @@ -461,6 +461,8 @@ static inline struct zonelist *node_zonelist(int nid, gfp_t flags) >> #define HAVE_ARCH_FREE_PAGE >> static inline void arch_free_page(struct page *page, int order) >> { >> + if (!static_branch_unlikely(&guest_page_hinting_key)) >> + return; >> guest_free_page(page, order); >> } >> #endif >> diff --git a/include/linux/page_hinting.h b/include/linux/page_hinting.h >> index b54f7428f348..9bdcf63e1306 100644 >> --- a/include/linux/page_hinting.h >> +++ b/include/linux/page_hinting.h >> @@ -14,4 +14,9 @@ struct hypervisor_pages { >> unsigned int order; >> }; >> >> +extern int guest_page_hinting_flag; >> +extern struct static_key_false guest_page_hinting_key; >> + >> +int guest_page_hinting_sysctl(struct ctl_table *table, int write, >> + void __user *buffer, size_t *lenp, loff_t *ppos); >> void guest_free_page(struct page *page, int order); >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index ba4d9e85feb8..5d53629c9bfb 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -1690,6 +1690,15 @@ static struct ctl_table vm_table[] = { >> .extra1 = (void *)&mmap_rnd_compat_bits_min, >> .extra2 = (void *)&mmap_rnd_compat_bits_max, >> }, >> +#endif >> +#ifdef CONFIG_KVM_FREE_PAGE_HINTING >> + { >> + .procname = "guest-page-hinting", >> + .data = &guest_page_hinting_flag, >> + .maxlen = sizeof(guest_page_hinting_flag), >> + .mode = 0644, >> + .proc_handler = guest_page_hinting_sysctl, >> + }, >> #endif >> { } >> }; > Since you are adding a new sysctl shouldn't you also be updating > Documentation/sysctl/vm.txt? Indeed I will be doing that. However, I would first like to close other major issues with the design. > >> diff --git a/virt/kvm/page_hinting.c b/virt/kvm/page_hinting.c >> index 818bd6b84e0c..4a34ea8db0c8 100644 >> --- a/virt/kvm/page_hinting.c >> +++ b/virt/kvm/page_hinting.c >> @@ -1,6 +1,7 @@ >> #include <linux/gfp.h> >> #include <linux/mm.h> >> #include <linux/kernel.h> >> +#include <linux/kvm_host.h> >> >> /* >> * struct kvm_free_pages - Tracks the pages which are freed by the guest. >> @@ -31,6 +32,28 @@ struct page_hinting { >> >> DEFINE_PER_CPU(struct page_hinting, hinting_obj); >> >> +struct static_key_false guest_page_hinting_key = STATIC_KEY_FALSE_INIT; >> +EXPORT_SYMBOL(guest_page_hinting_key); >> +static DEFINE_MUTEX(hinting_mutex); >> +int guest_page_hinting_flag; >> +EXPORT_SYMBOL(guest_page_hinting_flag); > I'm not entirely sure this flag makes sense to me. What is to prevent > someone from turning this on when there is no means of actually using > the hints. I understand right now that guest_free_page doesn't > actually do anything, but when it does I would assume it has to > interact with a device. If that device is not present would it still > make sense for us to be generating hints? Fair point, I will address this issue. > >> + >> +int guest_page_hinting_sysctl(struct ctl_table *table, int write, >> + void __user *buffer, size_t *lenp, >> + loff_t *ppos) >> +{ >> + int ret; >> + >> + mutex_lock(&hinting_mutex); >> + ret = proc_dointvec(table, write, buffer, lenp, ppos); >> + if (guest_page_hinting_flag) >> + static_key_enable(&guest_page_hinting_key.key); >> + else >> + static_key_disable(&guest_page_hinting_key.key); >> + mutex_unlock(&hinting_mutex); >> + return ret; >> +} >> + >> void guest_free_page(struct page *page, int order) >> { >> } >> -- >> 2.17.2 >>
diff --git a/include/linux/gfp.h b/include/linux/gfp.h index e596527284ba..8389219a076a 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -461,6 +461,8 @@ static inline struct zonelist *node_zonelist(int nid, gfp_t flags) #define HAVE_ARCH_FREE_PAGE static inline void arch_free_page(struct page *page, int order) { + if (!static_branch_unlikely(&guest_page_hinting_key)) + return; guest_free_page(page, order); } #endif diff --git a/include/linux/page_hinting.h b/include/linux/page_hinting.h index b54f7428f348..9bdcf63e1306 100644 --- a/include/linux/page_hinting.h +++ b/include/linux/page_hinting.h @@ -14,4 +14,9 @@ struct hypervisor_pages { unsigned int order; }; +extern int guest_page_hinting_flag; +extern struct static_key_false guest_page_hinting_key; + +int guest_page_hinting_sysctl(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos); void guest_free_page(struct page *page, int order); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index ba4d9e85feb8..5d53629c9bfb 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1690,6 +1690,15 @@ static struct ctl_table vm_table[] = { .extra1 = (void *)&mmap_rnd_compat_bits_min, .extra2 = (void *)&mmap_rnd_compat_bits_max, }, +#endif +#ifdef CONFIG_KVM_FREE_PAGE_HINTING + { + .procname = "guest-page-hinting", + .data = &guest_page_hinting_flag, + .maxlen = sizeof(guest_page_hinting_flag), + .mode = 0644, + .proc_handler = guest_page_hinting_sysctl, + }, #endif { } }; diff --git a/virt/kvm/page_hinting.c b/virt/kvm/page_hinting.c index 818bd6b84e0c..4a34ea8db0c8 100644 --- a/virt/kvm/page_hinting.c +++ b/virt/kvm/page_hinting.c @@ -1,6 +1,7 @@ #include <linux/gfp.h> #include <linux/mm.h> #include <linux/kernel.h> +#include <linux/kvm_host.h> /* * struct kvm_free_pages - Tracks the pages which are freed by the guest. @@ -31,6 +32,28 @@ struct page_hinting { DEFINE_PER_CPU(struct page_hinting, hinting_obj); +struct static_key_false guest_page_hinting_key = STATIC_KEY_FALSE_INIT; +EXPORT_SYMBOL(guest_page_hinting_key); +static DEFINE_MUTEX(hinting_mutex); +int guest_page_hinting_flag; +EXPORT_SYMBOL(guest_page_hinting_flag); + +int guest_page_hinting_sysctl(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, + loff_t *ppos) +{ + int ret; + + mutex_lock(&hinting_mutex); + ret = proc_dointvec(table, write, buffer, lenp, ppos); + if (guest_page_hinting_flag) + static_key_enable(&guest_page_hinting_key.key); + else + static_key_disable(&guest_page_hinting_key.key); + mutex_unlock(&hinting_mutex); + return ret; +} + void guest_free_page(struct page *page, int order) { }
This patch enables the guest free page hinting support to enable or disable based on the STATIC key which could be set via sysctl. Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> --- include/linux/gfp.h | 2 ++ include/linux/page_hinting.h | 5 +++++ kernel/sysctl.c | 9 +++++++++ virt/kvm/page_hinting.c | 23 +++++++++++++++++++++++ 4 files changed, 39 insertions(+)