On Fri, Jun 12, 2020 at 7:39 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > As it stands if you include printk.h by itself it will fail to > compile because it requires definitions from ratelimit.h. However, > simply including ratelimit.h from printk.h does not work due to > inclusion loops involving sched.h and kernel.h. > > This patch solves this by moving bits from ratelimit.h into a new > header file which can then be included by printk.h without any > worries about header loops. > > The build bot then revealed some intriguing failures arising out > of this patch. On s390 there is an inclusion loop with asm/bug.h > and linux/kernel.h that triggers a compile failure, because kernel.h > will cause asm-generic/bug.h to be included before s390's own > asm/bug.h has finished processing. This has been fixed by not > including kernel.h in arch/s390/include/asm/bug.h. > > A related failure was seen on powerpc where asm/bug.h leads to > the inclusion of linux/kernel.h via asm-generic/bug.h which then > prematurely tries to use the very macros defined in asm/bug.h. > The particular inclusion path which led to this involves lockdep.h. > I have fixed this moving the type definitions lockdep.h into the > new lockdep_types.h. This change is a step to the right direction, thanks! FWIW, Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> It seems you are opening a can of worms :-) Have you a chance to look at [1]? I think it's a long road, but maybe you are interested to help with? [1]: https://lore.kernel.org/lkml/20200422125201.37618-1-andriy.shevchenko@xxxxxxxxxxxxxxx/ > Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > > diff --git a/include/linux/ratelimit_types.h b/include/linux/ratelimit_types.h > new file mode 100644 > index 000000000000..b676aa419eef > --- /dev/null > +++ b/include/linux/ratelimit_types.h > @@ -0,0 +1,43 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_RATELIMIT_TYPES_H > +#define _LINUX_RATELIMIT_TYPES_H > + > +#include <linux/bits.h> > +#include <linux/param.h> > +#include <linux/spinlock_types.h> > + > +#define DEFAULT_RATELIMIT_INTERVAL (5 * HZ) > +#define DEFAULT_RATELIMIT_BURST 10 > + > +/* issue num suppressed message on exit */ > +#define RATELIMIT_MSG_ON_RELEASE BIT(0) > + > +struct ratelimit_state { > + raw_spinlock_t lock; /* protect the state */ > + > + int interval; > + int burst; > + int printed; > + int missed; > + unsigned long begin; > + unsigned long flags; > +}; > + > +#define RATELIMIT_STATE_INIT(name, interval_init, burst_init) { \ > + .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \ > + .interval = interval_init, \ > + .burst = burst_init, \ > + } > + > +#define RATELIMIT_STATE_INIT_DISABLED \ > + RATELIMIT_STATE_INIT(ratelimit_state, 0, DEFAULT_RATELIMIT_BURST) > + > +#define DEFINE_RATELIMIT_STATE(name, interval_init, burst_init) \ > + \ > + struct ratelimit_state name = \ > + RATELIMIT_STATE_INIT(name, interval_init, burst_init) \ > + > +extern int ___ratelimit(struct ratelimit_state *rs, const char *func); > +#define __ratelimit(state) ___ratelimit(state, __func__) > + > +#endif /* _LINUX_RATELIMIT_TYPES_H */ > diff --git a/include/linux/printk.h b/include/linux/printk.h > index e061635e0409..1cd862cfd2f4 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -7,6 +7,7 @@ > #include <linux/kern_levels.h> > #include <linux/linkage.h> > #include <linux/cache.h> > +#include <linux/ratelimit_types.h> > > extern const char linux_banner[]; > extern const char linux_proc_banner[]; > diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h > index 8ddf79e9207a..b17e0cd0a30c 100644 > --- a/include/linux/ratelimit.h > +++ b/include/linux/ratelimit.h > @@ -2,41 +2,10 @@ > #ifndef _LINUX_RATELIMIT_H > #define _LINUX_RATELIMIT_H > > -#include <linux/param.h> > +#include <linux/ratelimit_types.h> > #include <linux/sched.h> > #include <linux/spinlock.h> > > -#define DEFAULT_RATELIMIT_INTERVAL (5 * HZ) > -#define DEFAULT_RATELIMIT_BURST 10 > - > -/* issue num suppressed message on exit */ > -#define RATELIMIT_MSG_ON_RELEASE BIT(0) > - > -struct ratelimit_state { > - raw_spinlock_t lock; /* protect the state */ > - > - int interval; > - int burst; > - int printed; > - int missed; > - unsigned long begin; > - unsigned long flags; > -}; > - > -#define RATELIMIT_STATE_INIT(name, interval_init, burst_init) { \ > - .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \ > - .interval = interval_init, \ > - .burst = burst_init, \ > - } > - > -#define RATELIMIT_STATE_INIT_DISABLED \ > - RATELIMIT_STATE_INIT(ratelimit_state, 0, DEFAULT_RATELIMIT_BURST) > - > -#define DEFINE_RATELIMIT_STATE(name, interval_init, burst_init) \ > - \ > - struct ratelimit_state name = \ > - RATELIMIT_STATE_INIT(name, interval_init, burst_init) \ > - > static inline void ratelimit_state_init(struct ratelimit_state *rs, > int interval, int burst) > { > @@ -73,9 +42,6 @@ ratelimit_set_flags(struct ratelimit_state *rs, unsigned long flags) > > extern struct ratelimit_state printk_ratelimit_state; > > -extern int ___ratelimit(struct ratelimit_state *rs, const char *func); > -#define __ratelimit(state) ___ratelimit(state, __func__) > - > #ifdef CONFIG_PRINTK > > #define WARN_ON_RATELIMIT(condition, state) ({ \ > diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h > index 7725f8006fdf..0b25f28351ed 100644 > --- a/arch/s390/include/asm/bug.h > +++ b/arch/s390/include/asm/bug.h > @@ -2,7 +2,7 @@ > #ifndef _ASM_S390_BUG_H > #define _ASM_S390_BUG_H > > -#include <linux/kernel.h> > +#include <linux/compiler.h> > > #ifdef CONFIG_BUG > > diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h > new file mode 100644 > index 000000000000..7b9350624577 > --- /dev/null > +++ b/include/linux/lockdep_types.h > @@ -0,0 +1,196 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Runtime locking correctness validator > + * > + * Copyright (C) 2006,2007 Red Hat, Inc., Ingo Molnar <mingo@xxxxxxxxxx> > + * Copyright (C) 2007 Red Hat, Inc., Peter Zijlstra > + * > + * see Documentation/locking/lockdep-design.rst for more details. > + */ > +#ifndef __LINUX_LOCKDEP_TYPES_H > +#define __LINUX_LOCKDEP_TYPES_H > + > +#include <linux/types.h> > + > +#define MAX_LOCKDEP_SUBCLASSES 8UL > + > +enum lockdep_wait_type { > + LD_WAIT_INV = 0, /* not checked, catch all */ > + > + LD_WAIT_FREE, /* wait free, rcu etc.. */ > + LD_WAIT_SPIN, /* spin loops, raw_spinlock_t etc.. */ > + > +#ifdef CONFIG_PROVE_RAW_LOCK_NESTING > + LD_WAIT_CONFIG, /* CONFIG_PREEMPT_LOCK, spinlock_t etc.. */ > +#else > + LD_WAIT_CONFIG = LD_WAIT_SPIN, > +#endif > + LD_WAIT_SLEEP, /* sleeping locks, mutex_t etc.. */ > + > + LD_WAIT_MAX, /* must be last */ > +}; > + > +#ifdef CONFIG_LOCKDEP > + > +#include <linux/list.h> > + > +/* > + * We'd rather not expose kernel/lockdep_states.h this wide, but we do need > + * the total number of states... :-( > + */ > +#define XXX_LOCK_USAGE_STATES (1+2*4) > + > +/* > + * NR_LOCKDEP_CACHING_CLASSES ... Number of classes > + * cached in the instance of lockdep_map > + * > + * Currently main class (subclass == 0) and signle depth subclass > + * are cached in lockdep_map. This optimization is mainly targeting > + * on rq->lock. double_rq_lock() acquires this highly competitive with > + * single depth. > + */ > +#define NR_LOCKDEP_CACHING_CLASSES 2 > + > +/* > + * A lockdep key is associated with each lock object. For static locks we use > + * the lock address itself as the key. Dynamically allocated lock objects can > + * have a statically or dynamically allocated key. Dynamically allocated lock > + * keys must be registered before being used and must be unregistered before > + * the key memory is freed. > + */ > +struct lockdep_subclass_key { > + char __one_byte; > +} __attribute__ ((__packed__)); > + > +/* hash_entry is used to keep track of dynamically allocated keys. */ > +struct lock_class_key { > + union { > + struct hlist_node hash_entry; > + struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES]; > + }; > +}; > + > +extern struct lock_class_key __lockdep_no_validate__; > + > +struct lock_trace; > + > +#define LOCKSTAT_POINTS 4 > + > +/* > + * The lock-class itself. The order of the structure members matters. > + * reinit_class() zeroes the key member and all subsequent members. > + */ > +struct lock_class { > + /* > + * class-hash: > + */ > + struct hlist_node hash_entry; > + > + /* > + * Entry in all_lock_classes when in use. Entry in free_lock_classes > + * when not in use. Instances that are being freed are on one of the > + * zapped_classes lists. > + */ > + struct list_head lock_entry; > + > + /* > + * These fields represent a directed graph of lock dependencies, > + * to every node we attach a list of "forward" and a list of > + * "backward" graph nodes. > + */ > + struct list_head locks_after, locks_before; > + > + const struct lockdep_subclass_key *key; > + unsigned int subclass; > + unsigned int dep_gen_id; > + > + /* > + * IRQ/softirq usage tracking bits: > + */ > + unsigned long usage_mask; > + const struct lock_trace *usage_traces[XXX_LOCK_USAGE_STATES]; > + > + /* > + * Generation counter, when doing certain classes of graph walking, > + * to ensure that we check one node only once: > + */ > + int name_version; > + const char *name; > + > + short wait_type_inner; > + short wait_type_outer; > + > +#ifdef CONFIG_LOCK_STAT > + unsigned long contention_point[LOCKSTAT_POINTS]; > + unsigned long contending_point[LOCKSTAT_POINTS]; > +#endif > +} __no_randomize_layout; > + > +#ifdef CONFIG_LOCK_STAT > +struct lock_time { > + s64 min; > + s64 max; > + s64 total; > + unsigned long nr; > +}; > + > +enum bounce_type { > + bounce_acquired_write, > + bounce_acquired_read, > + bounce_contended_write, > + bounce_contended_read, > + nr_bounce_types, > + > + bounce_acquired = bounce_acquired_write, > + bounce_contended = bounce_contended_write, > +}; > + > +struct lock_class_stats { > + unsigned long contention_point[LOCKSTAT_POINTS]; > + unsigned long contending_point[LOCKSTAT_POINTS]; > + struct lock_time read_waittime; > + struct lock_time write_waittime; > + struct lock_time read_holdtime; > + struct lock_time write_holdtime; > + unsigned long bounces[nr_bounce_types]; > +}; > + > +struct lock_class_stats lock_stats(struct lock_class *class); > +void clear_lock_stats(struct lock_class *class); > +#endif > + > +/* > + * Map the lock object (the lock instance) to the lock-class object. > + * This is embedded into specific lock instances: > + */ > +struct lockdep_map { > + struct lock_class_key *key; > + struct lock_class *class_cache[NR_LOCKDEP_CACHING_CLASSES]; > + const char *name; > + short wait_type_outer; /* can be taken in this context */ > + short wait_type_inner; /* presents this context */ > +#ifdef CONFIG_LOCK_STAT > + int cpu; > + unsigned long ip; > +#endif > +}; > + > +struct pin_cookie { unsigned int val; }; > + > +#else /* !CONFIG_LOCKDEP */ > + > +/* > + * The class key takes no space if lockdep is disabled: > + */ > +struct lock_class_key { }; > + > +/* > + * The lockdep_map takes no space if lockdep is disabled: > + */ > +struct lockdep_map { }; > + > +struct pin_cookie { }; > + > +#endif /* !LOCKDEP */ > + > +#endif /* __LINUX_LOCKDEP_TYPES_H */ > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index 206774ac6946..1655d767c2c7 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -10,181 +10,20 @@ > #ifndef __LINUX_LOCKDEP_H > #define __LINUX_LOCKDEP_H > > +#include <linux/lockdep_types.h> > + > struct task_struct; > -struct lockdep_map; > > /* for sysctl */ > extern int prove_locking; > extern int lock_stat; > > -#define MAX_LOCKDEP_SUBCLASSES 8UL > - > -#include <linux/types.h> > - > -enum lockdep_wait_type { > - LD_WAIT_INV = 0, /* not checked, catch all */ > - > - LD_WAIT_FREE, /* wait free, rcu etc.. */ > - LD_WAIT_SPIN, /* spin loops, raw_spinlock_t etc.. */ > - > -#ifdef CONFIG_PROVE_RAW_LOCK_NESTING > - LD_WAIT_CONFIG, /* CONFIG_PREEMPT_LOCK, spinlock_t etc.. */ > -#else > - LD_WAIT_CONFIG = LD_WAIT_SPIN, > -#endif > - LD_WAIT_SLEEP, /* sleeping locks, mutex_t etc.. */ > - > - LD_WAIT_MAX, /* must be last */ > -}; > - > #ifdef CONFIG_LOCKDEP > > #include <linux/linkage.h> > -#include <linux/list.h> > #include <linux/debug_locks.h> > #include <linux/stacktrace.h> > > -/* > - * We'd rather not expose kernel/lockdep_states.h this wide, but we do need > - * the total number of states... :-( > - */ > -#define XXX_LOCK_USAGE_STATES (1+2*4) > - > -/* > - * NR_LOCKDEP_CACHING_CLASSES ... Number of classes > - * cached in the instance of lockdep_map > - * > - * Currently main class (subclass == 0) and signle depth subclass > - * are cached in lockdep_map. This optimization is mainly targeting > - * on rq->lock. double_rq_lock() acquires this highly competitive with > - * single depth. > - */ > -#define NR_LOCKDEP_CACHING_CLASSES 2 > - > -/* > - * A lockdep key is associated with each lock object. For static locks we use > - * the lock address itself as the key. Dynamically allocated lock objects can > - * have a statically or dynamically allocated key. Dynamically allocated lock > - * keys must be registered before being used and must be unregistered before > - * the key memory is freed. > - */ > -struct lockdep_subclass_key { > - char __one_byte; > -} __attribute__ ((__packed__)); > - > -/* hash_entry is used to keep track of dynamically allocated keys. */ > -struct lock_class_key { > - union { > - struct hlist_node hash_entry; > - struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES]; > - }; > -}; > - > -extern struct lock_class_key __lockdep_no_validate__; > - > -struct lock_trace; > - > -#define LOCKSTAT_POINTS 4 > - > -/* > - * The lock-class itself. The order of the structure members matters. > - * reinit_class() zeroes the key member and all subsequent members. > - */ > -struct lock_class { > - /* > - * class-hash: > - */ > - struct hlist_node hash_entry; > - > - /* > - * Entry in all_lock_classes when in use. Entry in free_lock_classes > - * when not in use. Instances that are being freed are on one of the > - * zapped_classes lists. > - */ > - struct list_head lock_entry; > - > - /* > - * These fields represent a directed graph of lock dependencies, > - * to every node we attach a list of "forward" and a list of > - * "backward" graph nodes. > - */ > - struct list_head locks_after, locks_before; > - > - const struct lockdep_subclass_key *key; > - unsigned int subclass; > - unsigned int dep_gen_id; > - > - /* > - * IRQ/softirq usage tracking bits: > - */ > - unsigned long usage_mask; > - const struct lock_trace *usage_traces[XXX_LOCK_USAGE_STATES]; > - > - /* > - * Generation counter, when doing certain classes of graph walking, > - * to ensure that we check one node only once: > - */ > - int name_version; > - const char *name; > - > - short wait_type_inner; > - short wait_type_outer; > - > -#ifdef CONFIG_LOCK_STAT > - unsigned long contention_point[LOCKSTAT_POINTS]; > - unsigned long contending_point[LOCKSTAT_POINTS]; > -#endif > -} __no_randomize_layout; > - > -#ifdef CONFIG_LOCK_STAT > -struct lock_time { > - s64 min; > - s64 max; > - s64 total; > - unsigned long nr; > -}; > - > -enum bounce_type { > - bounce_acquired_write, > - bounce_acquired_read, > - bounce_contended_write, > - bounce_contended_read, > - nr_bounce_types, > - > - bounce_acquired = bounce_acquired_write, > - bounce_contended = bounce_contended_write, > -}; > - > -struct lock_class_stats { > - unsigned long contention_point[LOCKSTAT_POINTS]; > - unsigned long contending_point[LOCKSTAT_POINTS]; > - struct lock_time read_waittime; > - struct lock_time write_waittime; > - struct lock_time read_holdtime; > - struct lock_time write_holdtime; > - unsigned long bounces[nr_bounce_types]; > -}; > - > -struct lock_class_stats lock_stats(struct lock_class *class); > -void clear_lock_stats(struct lock_class *class); > -#endif > - > -/* > - * Map the lock object (the lock instance) to the lock-class object. > - * This is embedded into specific lock instances: > - */ > -struct lockdep_map { > - struct lock_class_key *key; > - struct lock_class *class_cache[NR_LOCKDEP_CACHING_CLASSES]; > - const char *name; > - short wait_type_outer; /* can be taken in this context */ > - short wait_type_inner; /* presents this context */ > -#ifdef CONFIG_LOCK_STAT > - int cpu; > - unsigned long ip; > -#endif > -}; > - > static inline void lockdep_copy_map(struct lockdep_map *to, > struct lockdep_map *from) > { > @@ -421,8 +260,6 @@ static inline void lock_set_subclass(struct lockdep_map *lock, > > extern void lock_downgrade(struct lockdep_map *lock, unsigned long ip); > > -struct pin_cookie { unsigned int val; }; > - > #define NIL_COOKIE (struct pin_cookie){ .val = 0U, } > > extern struct pin_cookie lock_pin_lock(struct lockdep_map *lock); > @@ -501,10 +338,6 @@ static inline void lockdep_set_selftest_task(struct task_struct *task) > # define lockdep_reset() do { debug_locks = 1; } while (0) > # define lockdep_free_key_range(start, size) do { } while (0) > # define lockdep_sys_exit() do { } while (0) > -/* > - * The class key takes no space if lockdep is disabled: > - */ > -struct lock_class_key { }; > > static inline void lockdep_register_key(struct lock_class_key *key) > { > @@ -514,11 +347,6 @@ static inline void lockdep_unregister_key(struct lock_class_key *key) > { > } > > -/* > - * The lockdep_map takes no space if lockdep is disabled: > - */ > -struct lockdep_map { }; > - > #define lockdep_depth(tsk) (0) > > #define lockdep_is_held_type(l, r) (1) > @@ -530,8 +358,6 @@ struct lockdep_map { }; > > #define lockdep_recursing(tsk) (0) > > -struct pin_cookie { }; > - > #define NIL_COOKIE (struct pin_cookie){ } > > #define lockdep_pin_lock(l) ({ struct pin_cookie cookie = { }; cookie; }) > -- > Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- With Best Regards, Andy Shevchenko
