Re: [RFC 2/3] headers: introduce linux/struct_types.h

From: Matthew Wilcox
Date: Tue Dec 07 2021 - 10:32:37 EST


On Tue, Dec 07, 2021 at 04:09:26PM +0100, Arnd Bergmann wrote:
> Working towards a cleaner header structure, start by moving the most
> commonly embedded structures into a single header file that itself
> has only a minimum set of indirect includes. At this moment, this
> include structures for
>
> - locking
> - timers
> - work queues
> - waitqueues
> - rcu
> - xarray
> - kobject
> - bio_vec

I generally support all of this. I did look at adding struct xarray to
types.h when I first added it, but was stymied by the need to embed the
spinlock. I looked at adding a linux/adt.h or a linux/struct.h but
it was just too much work. So thank you for taking this on.

> diff --git a/include/linux/struct_types.h b/include/linux/struct_types.h
> new file mode 100644
> index 000000000000..5a06849fd347
> --- /dev/null
> +++ b/include/linux/struct_types.h
> @@ -0,0 +1,483 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_STRUCT_TYPES_H
> +#define __LINUX_STRUCT_TYPES_H
> +/*
> + * This header includes data structures that build on top of
> + * the plain types from linux/types.h and that are commonly
> + * embedded within other structures in the kernel.
> + *
> + * By keeping these in one place that has a minimum set of
> + * indirect includes, we can avoid deeply nested include
> + * hierarchies that slow down the build and cause frequent
> + * recompiles after header changes.
> + *
> + * Be careful about including further headers here.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/bits.h>
> +#include <linux/threads.h>
> +#include <linux/lockdep_types.h>
> +#include <linux/rbtree_types.h>
> +
> +#if defined(CONFIG_SMP)
> +# include <asm/spinlock_types.h>
> +#else
> +# include <linux/spinlock_types_up.h>
> +#endif
> +
> +/**
> + * typedef refcount_t - variant of atomic_t specialized for reference counts
> + * @refs: atomic_t counter field
> + *
> + * The counter saturates at REFCOUNT_SATURATED and will not move once
> + * there. This avoids wrapping the counter and causing 'spurious'
> + * use-after-free bugs.
> + */

There's no corresponding patch to Documentation, so this kernel-doc is
orphaned.

> diff --git a/include/linux/swait.h b/include/linux/swait.h
> index 6a8c22b8c2a5..d7798752922d 100644
> --- a/include/linux/swait.h
> +++ b/include/linux/swait.h
> @@ -38,18 +38,6 @@
> * wait queues in most cases.
> */
>
> -struct task_struct;
> -
> -struct swait_queue_head {
> - raw_spinlock_t lock;
> - struct list_head task_list;
> -};
> -
> -struct swait_queue {
> - struct task_struct *task;
> - struct list_head task_list;
> -};
> -
> #define __SWAITQUEUE_INITIALIZER(name) { \
> .task = current, \
> .task_list = LIST_HEAD_INIT((name).task_list), \

swait.h doesn't need to include <linux/struct_types.h> ?

> diff --git a/include/linux/xarray.h b/include/linux/xarray.h
> index a91e3d90df8a..4f1e55074ef0 100644
> --- a/include/linux/xarray.h
> +++ b/include/linux/xarray.h
> @@ -275,29 +275,6 @@ enum xa_lock_type {
> #define XA_FLAGS_ALLOC (XA_FLAGS_TRACK_FREE | XA_FLAGS_MARK(XA_FREE_MARK))
> #define XA_FLAGS_ALLOC1 (XA_FLAGS_TRACK_FREE | XA_FLAGS_ZERO_BUSY)
>
> -/**
> - * struct xarray - The anchor of the XArray.
> - * @xa_lock: Lock that protects the contents of the XArray.
> - *
> - * To use the xarray, define it statically or embed it in your data structure.
> - * It is a very small data structure, so it does not usually make sense to
> - * allocate it separately and keep a pointer to it in your data structure.
> - *
> - * You may use the xa_lock to protect your own data structures as well.
> - */
> -/*
> - * If all of the entries in the array are NULL, @xa_head is a NULL pointer.
> - * If the only non-NULL entry in the array is at index 0, @xa_head is that
> - * entry. If any other entry in the array is non-NULL, @xa_head points
> - * to an @xa_node.
> - */
> -struct xarray {
> - spinlock_t xa_lock;
> -/* private: The rest of the data structure is not to be used directly. */
> - gfp_t xa_flags;
> - void __rcu * xa_head;
> -};
> -
> #define XARRAY_INIT(name, flags) { \
> .xa_lock = __SPIN_LOCK_UNLOCKED(name.xa_lock), \
> .xa_flags = flags, \

I think this is going to break:

(cd tools/testing/radix-tree; make)