From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759065Ab3FCP5Y (ORCPT ); Mon, 3 Jun 2013 11:57:24 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:39070 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756753Ab3FCP5W (ORCPT ); Mon, 3 Jun 2013 11:57:22 -0400 Date: Mon, 3 Jun 2013 11:56:12 -0400 From: Konrad Rzeszutek Wilk To: Raghavendra K T Cc: gleb@redhat.com, mingo@redhat.com, jeremy@goop.org, x86@kernel.org, hpa@zytor.com, pbonzini@redhat.com, linux-doc@vger.kernel.org, habanero@linux.vnet.ibm.com, xen-devel@lists.xensource.com, peterz@infradead.org, mtosatti@redhat.com, stefano.stabellini@eu.citrix.com, andi@firstfloor.org, attilio.rao@citrix.com, ouyang@cs.pitt.edu, gregkh@suse.de, agraf@suse.de, chegu_vinod@hp.com, torvalds@linux-foundation.org, avi.kivity@gmail.com, tglx@linutronix.de, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, stephan.diestelhorst@amd.com, riel@redhat.com, drjones@redhat.com, virtualization@lists.linux-foundation.org, srivatsa.vaddagiri@gmail.com Subject: Re: [PATCH RFC V9 9/19] Split out rate limiting from jump_label.h Message-ID: <20130603155612.GD4224@phenom.dumpdata.com> References: <20130601192125.5966.35563.sendpatchset@codeblue> <20130601192422.5966.44354.sendpatchset@codeblue> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130601192422.5966.44354.sendpatchset@codeblue> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: ucsinet21.oracle.com [156.151.31.93] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jun 02, 2013 at 12:54:22AM +0530, Raghavendra K T wrote: > Split jumplabel ratelimit I would change the title a bit, perhaps prefix it with: "jump_label: " > > From: Andrew Jones > > Commit b202952075f62603bea9bfb6ebc6b0420db11949 introduced rate limiting Also please add right after the git id this: ("perf, core: Rate limit perf_sched_events jump_label patching") > for jump label disabling. The changes were made in the jump label code > in order to be more widely available and to keep things tidier. This is > all fine, except now jump_label.h includes linux/workqueue.h, which > makes it impossible to include jump_label.h from anything that > workqueue.h needs. For example, it's now impossible to include > jump_label.h from asm/spinlock.h, which is done in proposed > pv-ticketlock patches. This patch splits out the rate limiting related > changes from jump_label.h into a new file, jump_label_ratelimit.h, to > resolve the issue. > > Signed-off-by: Andrew Jones > Signed-off-by: Raghavendra K T Otherwise looks fine to me: Reviewed-by: Konrad Rzeszutek Wilk > --- > include/linux/jump_label.h | 26 +------------------------- > include/linux/jump_label_ratelimit.h | 34 ++++++++++++++++++++++++++++++++++ > include/linux/perf_event.h | 1 + > kernel/jump_label.c | 1 + > 4 files changed, 37 insertions(+), 25 deletions(-) > create mode 100644 include/linux/jump_label_ratelimit.h > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 0976fc4..53cdf89 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -48,7 +48,6 @@ > > #include > #include > -#include > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > @@ -61,12 +60,6 @@ struct static_key { > #endif > }; > > -struct static_key_deferred { > - struct static_key key; > - unsigned long timeout; > - struct delayed_work work; > -}; > - > # include > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > @@ -119,10 +112,7 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry, > extern int jump_label_text_reserved(void *start, void *end); > extern void static_key_slow_inc(struct static_key *key); > extern void static_key_slow_dec(struct static_key *key); > -extern void static_key_slow_dec_deferred(struct static_key_deferred *key); > extern void jump_label_apply_nops(struct module *mod); > -extern void > -jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > @@ -141,10 +131,6 @@ static __always_inline void jump_label_init(void) > { > } > > -struct static_key_deferred { > - struct static_key key; > -}; > - > static __always_inline bool static_key_false(struct static_key *key) > { > if (unlikely(atomic_read(&key->enabled)) > 0) > @@ -169,11 +155,6 @@ static inline void static_key_slow_dec(struct static_key *key) > atomic_dec(&key->enabled); > } > > -static inline void static_key_slow_dec_deferred(struct static_key_deferred *key) > -{ > - static_key_slow_dec(&key->key); > -} > - > static inline int jump_label_text_reserved(void *start, void *end) > { > return 0; > @@ -187,12 +168,6 @@ static inline int jump_label_apply_nops(struct module *mod) > return 0; > } > > -static inline void > -jump_label_rate_limit(struct static_key_deferred *key, > - unsigned long rl) > -{ > -} > - > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1) }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > @@ -203,6 +178,7 @@ jump_label_rate_limit(struct static_key_deferred *key, > #define STATIC_KEY_INIT STATIC_KEY_INIT_FALSE > #define jump_label_enabled static_key_enabled > > +static inline int atomic_read(const atomic_t *v); > static inline bool static_key_enabled(struct static_key *key) > { > return (atomic_read(&key->enabled) > 0); > diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h > new file mode 100644 > index 0000000..1137883 > --- /dev/null > +++ b/include/linux/jump_label_ratelimit.h > @@ -0,0 +1,34 @@ > +#ifndef _LINUX_JUMP_LABEL_RATELIMIT_H > +#define _LINUX_JUMP_LABEL_RATELIMIT_H > + > +#include > +#include > + > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > +struct static_key_deferred { > + struct static_key key; > + unsigned long timeout; > + struct delayed_work work; > +}; > +#endif > + > +#ifdef HAVE_JUMP_LABEL > +extern void static_key_slow_dec_deferred(struct static_key_deferred *key); > +extern void > +jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl); > + > +#else /* !HAVE_JUMP_LABEL */ > +struct static_key_deferred { > + struct static_key key; > +}; > +static inline void static_key_slow_dec_deferred(struct static_key_deferred *key) > +{ > + static_key_slow_dec(&key->key); > +} > +static inline void > +jump_label_rate_limit(struct static_key_deferred *key, > + unsigned long rl) > +{ > +} > +#endif /* HAVE_JUMP_LABEL */ > +#endif /* _LINUX_JUMP_LABEL_RATELIMIT_H */ > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index f463a46..a8eac60 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -48,6 +48,7 @@ struct perf_guest_info_callbacks { > #include > #include > #include > +#include > #include > #include > #include > diff --git a/kernel/jump_label.c b/kernel/jump_label.c > index 60f48fa..297a924 100644 > --- a/kernel/jump_label.c > +++ b/kernel/jump_label.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > #ifdef HAVE_JUMP_LABEL > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH RFC V9 9/19] Split out rate limiting from jump_label.h Date: Mon, 3 Jun 2013 11:56:12 -0400 Message-ID: <20130603155612.GD4224@phenom.dumpdata.com> References: <20130601192125.5966.35563.sendpatchset@codeblue> <20130601192422.5966.44354.sendpatchset@codeblue> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: jeremy@goop.org, gregkh@suse.de, kvm@vger.kernel.org, linux-doc@vger.kernel.org, peterz@infradead.org, drjones@redhat.com, virtualization@lists.linux-foundation.org, andi@firstfloor.org, hpa@zytor.com, xen-devel@lists.xensource.com, x86@kernel.org, mingo@redhat.com, habanero@linux.vnet.ibm.com, riel@redhat.com, stefano.stabellini@eu.citrix.com, ouyang@cs.pitt.edu, avi.kivity@gmail.com, tglx@linutronix.de, chegu_vinod@hp.com, linux-kernel@vger.kernel.org, srivatsa.vaddagiri@gmail.com, attilio.rao@citrix.com, pbonzini@redhat.com, torvalds@linux-foundation.org, stephan.diestelhorst@amd.com To: Raghavendra K T Return-path: Content-Disposition: inline In-Reply-To: <20130601192422.5966.44354.sendpatchset@codeblue> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: kvm.vger.kernel.org On Sun, Jun 02, 2013 at 12:54:22AM +0530, Raghavendra K T wrote: > Split jumplabel ratelimit I would change the title a bit, perhaps prefix it with: "jump_label: " > > From: Andrew Jones > > Commit b202952075f62603bea9bfb6ebc6b0420db11949 introduced rate limiting Also please add right after the git id this: ("perf, core: Rate limit perf_sched_events jump_label patching") > for jump label disabling. The changes were made in the jump label code > in order to be more widely available and to keep things tidier. This is > all fine, except now jump_label.h includes linux/workqueue.h, which > makes it impossible to include jump_label.h from anything that > workqueue.h needs. For example, it's now impossible to include > jump_label.h from asm/spinlock.h, which is done in proposed > pv-ticketlock patches. This patch splits out the rate limiting related > changes from jump_label.h into a new file, jump_label_ratelimit.h, to > resolve the issue. > > Signed-off-by: Andrew Jones > Signed-off-by: Raghavendra K T Otherwise looks fine to me: Reviewed-by: Konrad Rzeszutek Wilk > --- > include/linux/jump_label.h | 26 +------------------------- > include/linux/jump_label_ratelimit.h | 34 ++++++++++++++++++++++++++++++++++ > include/linux/perf_event.h | 1 + > kernel/jump_label.c | 1 + > 4 files changed, 37 insertions(+), 25 deletions(-) > create mode 100644 include/linux/jump_label_ratelimit.h > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 0976fc4..53cdf89 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -48,7 +48,6 @@ > > #include > #include > -#include > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > @@ -61,12 +60,6 @@ struct static_key { > #endif > }; > > -struct static_key_deferred { > - struct static_key key; > - unsigned long timeout; > - struct delayed_work work; > -}; > - > # include > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > @@ -119,10 +112,7 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry, > extern int jump_label_text_reserved(void *start, void *end); > extern void static_key_slow_inc(struct static_key *key); > extern void static_key_slow_dec(struct static_key *key); > -extern void static_key_slow_dec_deferred(struct static_key_deferred *key); > extern void jump_label_apply_nops(struct module *mod); > -extern void > -jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > @@ -141,10 +131,6 @@ static __always_inline void jump_label_init(void) > { > } > > -struct static_key_deferred { > - struct static_key key; > -}; > - > static __always_inline bool static_key_false(struct static_key *key) > { > if (unlikely(atomic_read(&key->enabled)) > 0) > @@ -169,11 +155,6 @@ static inline void static_key_slow_dec(struct static_key *key) > atomic_dec(&key->enabled); > } > > -static inline void static_key_slow_dec_deferred(struct static_key_deferred *key) > -{ > - static_key_slow_dec(&key->key); > -} > - > static inline int jump_label_text_reserved(void *start, void *end) > { > return 0; > @@ -187,12 +168,6 @@ static inline int jump_label_apply_nops(struct module *mod) > return 0; > } > > -static inline void > -jump_label_rate_limit(struct static_key_deferred *key, > - unsigned long rl) > -{ > -} > - > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1) }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > @@ -203,6 +178,7 @@ jump_label_rate_limit(struct static_key_deferred *key, > #define STATIC_KEY_INIT STATIC_KEY_INIT_FALSE > #define jump_label_enabled static_key_enabled > > +static inline int atomic_read(const atomic_t *v); > static inline bool static_key_enabled(struct static_key *key) > { > return (atomic_read(&key->enabled) > 0); > diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h > new file mode 100644 > index 0000000..1137883 > --- /dev/null > +++ b/include/linux/jump_label_ratelimit.h > @@ -0,0 +1,34 @@ > +#ifndef _LINUX_JUMP_LABEL_RATELIMIT_H > +#define _LINUX_JUMP_LABEL_RATELIMIT_H > + > +#include > +#include > + > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > +struct static_key_deferred { > + struct static_key key; > + unsigned long timeout; > + struct delayed_work work; > +}; > +#endif > + > +#ifdef HAVE_JUMP_LABEL > +extern void static_key_slow_dec_deferred(struct static_key_deferred *key); > +extern void > +jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl); > + > +#else /* !HAVE_JUMP_LABEL */ > +struct static_key_deferred { > + struct static_key key; > +}; > +static inline void static_key_slow_dec_deferred(struct static_key_deferred *key) > +{ > + static_key_slow_dec(&key->key); > +} > +static inline void > +jump_label_rate_limit(struct static_key_deferred *key, > + unsigned long rl) > +{ > +} > +#endif /* HAVE_JUMP_LABEL */ > +#endif /* _LINUX_JUMP_LABEL_RATELIMIT_H */ > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index f463a46..a8eac60 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -48,6 +48,7 @@ struct perf_guest_info_callbacks { > #include > #include > #include > +#include > #include > #include > #include > diff --git a/kernel/jump_label.c b/kernel/jump_label.c > index 60f48fa..297a924 100644 > --- a/kernel/jump_label.c > +++ b/kernel/jump_label.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > #ifdef HAVE_JUMP_LABEL > >