| Message ID | 20231216032651.3553101-9-kent.overstreet@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | big header dependency cleanup targeting sched.h | expand |
On 12/15/23 22:26, Kent Overstreet wrote: > Trimming down sched.h dependencies: we don't want to include more than > the base types. > > Signed-off-by: Kent Overstreet<kent.overstreet@linux.dev> > Cc: Peter Zijlstra<peterz@infradead.org> > Cc: Ingo Molnar<mingo@redhat.com> > Cc: Will Deacon<will@kernel.org> > Cc: Waiman Long<longman@redhat.com> > Cc: Boqun Feng<boqun.feng@gmail.com> > Signed-off-by: Kent Overstreet<kent.overstreet@linux.dev> > --- > include/linux/mutex.h | 52 +-------------------------- > include/linux/mutex_types.h | 71 +++++++++++++++++++++++++++++++++++++ > include/linux/sched.h | 2 +- > 3 files changed, 73 insertions(+), 52 deletions(-) > create mode 100644 include/linux/mutex_types.h > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > index a33aa9eb9fc3..0dfba5df6524 100644 > --- a/include/linux/mutex.h > +++ b/include/linux/mutex.h > @@ -20,6 +20,7 @@ > #include <linux/osq_lock.h> > #include <linux/debug_locks.h> > #include <linux/cleanup.h> > +#include <linux/mutex_types.h> > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > # define __DEP_MAP_MUTEX_INITIALIZER(lockname) \ > @@ -33,49 +34,6 @@ > > #ifndef CONFIG_PREEMPT_RT > > -/* > - * Simple, straightforward mutexes with strict semantics: > - * > - * - only one task can hold the mutex at a time > - * - only the owner can unlock the mutex > - * - multiple unlocks are not permitted > - * - recursive locking is not permitted > - * - a mutex object must be initialized via the API > - * - a mutex object must not be initialized via memset or copying > - * - task may not exit with mutex held > - * - memory areas where held locks reside must not be freed > - * - held mutexes must not be reinitialized > - * - mutexes may not be used in hardware or software interrupt > - * contexts such as tasklets and timers > - * > - * These semantics are fully enforced when DEBUG_MUTEXES is > - * enabled. Furthermore, besides enforcing the above rules, the mutex > - * debugging code also implements a number of additional features > - * that make lock debugging easier and faster: > - * > - * - uses symbolic names of mutexes, whenever they are printed in debug output > - * - point-of-acquire tracking, symbolic lookup of function names > - * - list of all locks held in the system, printout of them > - * - owner tracking > - * - detects self-recursing locks and prints out all relevant info > - * - detects multi-task circular deadlocks and prints out all affected > - * locks and tasks (and only those tasks) > - */ > -struct mutex { > - atomic_long_t owner; > - raw_spinlock_t wait_lock; > -#ifdef CONFIG_MUTEX_SPIN_ON_OWNER > - struct optimistic_spin_queue osq; /* Spinner MCS lock */ > -#endif > - struct list_head wait_list; > -#ifdef CONFIG_DEBUG_MUTEXES > - void *magic; > -#endif > -#ifdef CONFIG_DEBUG_LOCK_ALLOC > - struct lockdep_map dep_map; > -#endif > -}; > - > #ifdef CONFIG_DEBUG_MUTEXES > > #define __DEBUG_MUTEX_INITIALIZER(lockname) \ > @@ -131,14 +89,6 @@ extern bool mutex_is_locked(struct mutex *lock); > /* > * Preempt-RT variant based on rtmutexes. > */ > -#include <linux/rtmutex.h> Including rtmutex.h here means that mutex_types.h is no longer a simple header for types only. So unless you also break out a rtmutex_types.h, it is inconsistent. Besides, the kernel/sched code does use mutex_lock/unlock calls quite frequently. With this patch, mutex.h will not be directly included. I suspect that it is indirectly included via other header files. This may be an issue with some configurations. Cheers, Longman
On Mon, Dec 18, 2023 at 11:53:08AM -0500, Waiman Long wrote: > On 12/15/23 22:26, Kent Overstreet wrote: > > -#include <linux/rtmutex.h> > > Including rtmutex.h here means that mutex_types.h is no longer a simple > header for types only. So unless you also break out a rtmutex_types.h, it is > inconsistent. good observation, I'll have to leave it for the next round of cleanups though since the merge window is approaching and I'll have to redo all the testing. > Besides, the kernel/sched code does use mutex_lock/unlock calls quite > frequently. With this patch, mutex.h will not be directly included. I > suspect that it is indirectly included via other header files. This may be > an issue with some configurations. I've now put it through randconfig testing on every arch that debian includes a compiler for (excluding sh and xtensa, which throw internal compiler errors) and that one hasn't come up yet. could still be included indirectly though - I haven't checked for that one specifically yet.
On 12/18/23 20:46, Kent Overstreet wrote: > On Mon, Dec 18, 2023 at 11:53:08AM -0500, Waiman Long wrote: >> On 12/15/23 22:26, Kent Overstreet wrote: >>> -#include <linux/rtmutex.h> >> Including rtmutex.h here means that mutex_types.h is no longer a simple >> header for types only. So unless you also break out a rtmutex_types.h, it is >> inconsistent. > good observation, I'll have to leave it for the next round of cleanups > though since the merge window is approaching and I'll have to redo all > the testing. > >> Besides, the kernel/sched code does use mutex_lock/unlock calls quite >> frequently. With this patch, mutex.h will not be directly included. I >> suspect that it is indirectly included via other header files. This may be >> an issue with some configurations. > I've now put it through randconfig testing on every arch that debian > includes a compiler for (excluding sh and xtensa, which throw internal > compiler errors) and that one hasn't come up yet. > > could still be included indirectly though - I haven't checked for that > one specifically yet. If you are replacing mutex.h in include/linux/sched.h by mutex_types.h, I would suggest you add mutex.h to kernel/sched/sched.h to ensure this header file is included by the scheduler code. Cheers, Longman
On Mon, Dec 18, 2023 at 10:04:39PM -0500, Waiman Long wrote: > > On 12/18/23 20:46, Kent Overstreet wrote: > > On Mon, Dec 18, 2023 at 11:53:08AM -0500, Waiman Long wrote: > > > On 12/15/23 22:26, Kent Overstreet wrote: > > > > -#include <linux/rtmutex.h> > > > Including rtmutex.h here means that mutex_types.h is no longer a simple > > > header for types only. So unless you also break out a rtmutex_types.h, it is > > > inconsistent. > > good observation, I'll have to leave it for the next round of cleanups > > though since the merge window is approaching and I'll have to redo all > > the testing. > > > > > Besides, the kernel/sched code does use mutex_lock/unlock calls quite > > > frequently. With this patch, mutex.h will not be directly included. I > > > suspect that it is indirectly included via other header files. This may be > > > an issue with some configurations. > > I've now put it through randconfig testing on every arch that debian > > includes a compiler for (excluding sh and xtensa, which throw internal > > compiler errors) and that one hasn't come up yet. > > > > could still be included indirectly though - I haven't checked for that > > one specifically yet. > > If you are replacing mutex.h in include/linux/sched.h by mutex_types.h, I > would suggest you add mutex.h to kernel/sched/sched.h to ensure this header > file is included by the scheduler code. It already does, via mutex_api.h...
On 12/18/23 22:37, Kent Overstreet wrote: > On Mon, Dec 18, 2023 at 10:04:39PM -0500, Waiman Long wrote: >> On 12/18/23 20:46, Kent Overstreet wrote: >>> On Mon, Dec 18, 2023 at 11:53:08AM -0500, Waiman Long wrote: >>>> On 12/15/23 22:26, Kent Overstreet wrote: >>>>> -#include <linux/rtmutex.h> >>>> Including rtmutex.h here means that mutex_types.h is no longer a simple >>>> header for types only. So unless you also break out a rtmutex_types.h, it is >>>> inconsistent. >>> good observation, I'll have to leave it for the next round of cleanups >>> though since the merge window is approaching and I'll have to redo all >>> the testing. >>> >>>> Besides, the kernel/sched code does use mutex_lock/unlock calls quite >>>> frequently. With this patch, mutex.h will not be directly included. I >>>> suspect that it is indirectly included via other header files. This may be >>>> an issue with some configurations. >>> I've now put it through randconfig testing on every arch that debian >>> includes a compiler for (excluding sh and xtensa, which throw internal >>> compiler errors) and that one hasn't come up yet. >>> >>> could still be included indirectly though - I haven't checked for that >>> one specifically yet. >> If you are replacing mutex.h in include/linux/sched.h by mutex_types.h, I >> would suggest you add mutex.h to kernel/sched/sched.h to ensure this header >> file is included by the scheduler code. > It already does, via mutex_api.h... You are right. I am not aware that there is a mutex_api.h header file which is just an alias of mutex.h. Cheers, Longman
diff --git a/include/linux/mutex.h b/include/linux/mutex.h index a33aa9eb9fc3..0dfba5df6524 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -20,6 +20,7 @@ #include <linux/osq_lock.h> #include <linux/debug_locks.h> #include <linux/cleanup.h> +#include <linux/mutex_types.h> #ifdef CONFIG_DEBUG_LOCK_ALLOC # define __DEP_MAP_MUTEX_INITIALIZER(lockname) \ @@ -33,49 +34,6 @@ #ifndef CONFIG_PREEMPT_RT -/* - * Simple, straightforward mutexes with strict semantics: - * - * - only one task can hold the mutex at a time - * - only the owner can unlock the mutex - * - multiple unlocks are not permitted - * - recursive locking is not permitted - * - a mutex object must be initialized via the API - * - a mutex object must not be initialized via memset or copying - * - task may not exit with mutex held - * - memory areas where held locks reside must not be freed - * - held mutexes must not be reinitialized - * - mutexes may not be used in hardware or software interrupt - * contexts such as tasklets and timers - * - * These semantics are fully enforced when DEBUG_MUTEXES is - * enabled. Furthermore, besides enforcing the above rules, the mutex - * debugging code also implements a number of additional features - * that make lock debugging easier and faster: - * - * - uses symbolic names of mutexes, whenever they are printed in debug output - * - point-of-acquire tracking, symbolic lookup of function names - * - list of all locks held in the system, printout of them - * - owner tracking - * - detects self-recursing locks and prints out all relevant info - * - detects multi-task circular deadlocks and prints out all affected - * locks and tasks (and only those tasks) - */ -struct mutex { - atomic_long_t owner; - raw_spinlock_t wait_lock; -#ifdef CONFIG_MUTEX_SPIN_ON_OWNER - struct optimistic_spin_queue osq; /* Spinner MCS lock */ -#endif - struct list_head wait_list; -#ifdef CONFIG_DEBUG_MUTEXES - void *magic; -#endif -#ifdef CONFIG_DEBUG_LOCK_ALLOC - struct lockdep_map dep_map; -#endif -}; - #ifdef CONFIG_DEBUG_MUTEXES #define __DEBUG_MUTEX_INITIALIZER(lockname) \ @@ -131,14 +89,6 @@ extern bool mutex_is_locked(struct mutex *lock); /* * Preempt-RT variant based on rtmutexes. */ -#include <linux/rtmutex.h> - -struct mutex { - struct rt_mutex_base rtmutex; -#ifdef CONFIG_DEBUG_LOCK_ALLOC - struct lockdep_map dep_map; -#endif -}; #define __MUTEX_INITIALIZER(mutexname) \ { \ diff --git a/include/linux/mutex_types.h b/include/linux/mutex_types.h new file mode 100644 index 000000000000..fdf7f515fde8 --- /dev/null +++ b/include/linux/mutex_types.h @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __LINUX_MUTEX_TYPES_H +#define __LINUX_MUTEX_TYPES_H + +#include <linux/atomic.h> +#include <linux/lockdep_types.h> +#include <linux/osq_lock.h> +#include <linux/spinlock_types.h> +#include <linux/types.h> + +#ifndef CONFIG_PREEMPT_RT + +/* + * Simple, straightforward mutexes with strict semantics: + * + * - only one task can hold the mutex at a time + * - only the owner can unlock the mutex + * - multiple unlocks are not permitted + * - recursive locking is not permitted + * - a mutex object must be initialized via the API + * - a mutex object must not be initialized via memset or copying + * - task may not exit with mutex held + * - memory areas where held locks reside must not be freed + * - held mutexes must not be reinitialized + * - mutexes may not be used in hardware or software interrupt + * contexts such as tasklets and timers + * + * These semantics are fully enforced when DEBUG_MUTEXES is + * enabled. Furthermore, besides enforcing the above rules, the mutex + * debugging code also implements a number of additional features + * that make lock debugging easier and faster: + * + * - uses symbolic names of mutexes, whenever they are printed in debug output + * - point-of-acquire tracking, symbolic lookup of function names + * - list of all locks held in the system, printout of them + * - owner tracking + * - detects self-recursing locks and prints out all relevant info + * - detects multi-task circular deadlocks and prints out all affected + * locks and tasks (and only those tasks) + */ +struct mutex { + atomic_long_t owner; + raw_spinlock_t wait_lock; +#ifdef CONFIG_MUTEX_SPIN_ON_OWNER + struct optimistic_spin_queue osq; /* Spinner MCS lock */ +#endif + struct list_head wait_list; +#ifdef CONFIG_DEBUG_MUTEXES + void *magic; +#endif +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct lockdep_map dep_map; +#endif +}; + +#else /* !CONFIG_PREEMPT_RT */ +/* + * Preempt-RT variant based on rtmutexes. + */ +#include <linux/rtmutex.h> + +struct mutex { + struct rt_mutex_base rtmutex; +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct lockdep_map dep_map; +#endif +}; + +#endif /* CONFIG_PREEMPT_RT */ + +#endif /* __LINUX_MUTEX_TYPES_H */ diff --git a/include/linux/sched.h b/include/linux/sched.h index 3762809652da..e8892789969b 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -15,7 +15,7 @@ #include <linux/sem.h> #include <linux/shm.h> #include <linux/kmsan_types.h> -#include <linux/mutex.h> +#include <linux/mutex_types.h> #include <linux/plist.h> #include <linux/hrtimer_types.h> #include <linux/irqflags.h>