[PATCH 00/12] radix-tree: split out struct radix_tree_root out to <linux/radix-tree-root.h>
The motivation of this series is to cut down unnecessary header dependency in terms of radix tree.
Sub-systems or drivers that use radix-tree for data management typically embed struct radix_tree_root in their data structures, like this:
struct foo { ...
struct radix_tree_root foo_tree; ... };
So, <linux/foo.h> needs to include <linux/radix-tree.h>, therefore, users of <linux/foo.h> include a lot of bloat from <linux/radix-tree.h>.
If you see the definition of radix_tree_root,
struct radix_tree_root { gfp_t gfp_mask; struct radix_tree_node __rcu *rnode; };
it is a very simple structure. It only depends on <linux/types.h> for gfp_t and <linux/compiler.h> for __rcu.
By splitting out the radix_tree_root definition, we can reduce the header file dependency.
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
The patch 1 is a trivial clean-up; it is just here to avoid conflict.
The patch 2 is the main part of this series; split out struct radix_tree_root.
The rest of the series replace <linux/radix-tree.h> with <linux/radix-tree-root.h> where appropriate.
Please review if the idea is OK.
If it is OK, I'd like to know how to apply the series.
Perhaps, the first two for v4.15. Then, rest of series will be sent per-subsystem for v4.16?
Or, can somebody take care of the whole series?
I checked allmodconfig for x86 and arm64. I am expecting 0 day testing will check it too.
Masahiro Yamada (12): radix-tree: replace <linux/spinlock.h> with <linux/spinlock_types.h> radix-tree: split struct radix_tree_root to <linux/radix-tree-root.h> irqdomain: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> writeback: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> iocontext.h: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> fs: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> blkcg: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> fscache: include <linux-radix-tree.h> sh: intc: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> net/mlx4: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> net/mlx5: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> drm/i915: replace <linux/radix-tree.h> with <linux/radix-tree-root.h>
drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 1 + drivers/gpu/drm/i915/i915_gem_context.h | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 + drivers/gpu/drm/i915/i915_gem_object.h | 1 + drivers/net/ethernet/mellanox/mlx4/cq.c | 1 + drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +- drivers/net/ethernet/mellanox/mlx4/qp.c | 1 + drivers/net/ethernet/mellanox/mlx4/srq.c | 1 + drivers/sh/intc/internals.h | 2 +- include/linux/backing-dev-defs.h | 2 +- include/linux/blk-cgroup.h | 2 +- include/linux/fs.h | 2 +- include/linux/fscache.h | 1 + include/linux/iocontext.h | 2 +- include/linux/irqdomain.h | 2 +- include/linux/mlx4/device.h | 2 +- include/linux/mlx4/qp.h | 1 + include/linux/mlx5/driver.h | 2 +- include/linux/mlx5/qp.h | 1 + include/linux/radix-tree-root.h | 24 ++++++++++++++++++++++++ include/linux/radix-tree.h | 8 ++------ 22 files changed, 46 insertions(+), 16 deletions(-) create mode 100644 include/linux/radix-tree-root.h
The header drivers/gpu/drm/i915/i915_gem_context.h requires the definition of struct radix_tree_root, but does not need to know anything about other radix tree stuff.
Include <linux/radix-tree-root.h> instead of <linux/radix-tree.h> to reduce the number of included header files.
While we are here, let's add missing <linux/radix-tree.h> where radix tree accessors are used.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 1 + drivers/gpu/drm/i915/i915_gem_context.h | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 + drivers/gpu/drm/i915/i915_gem_object.h | 1 + 5 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 19404c9..d2356eb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -37,6 +37,7 @@ #include "intel_mocs.h" #include <linux/dma-fence-array.h> #include <linux/kthread.h> +#include <linux/radix-tree.h> #include <linux/reservation.h> #include <linux/shmem_fs.h> #include <linux/slab.h> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 58a2a44..34b2195 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -86,6 +86,7 @@ */
#include <linux/log2.h> +#include <linux/radix-tree.h> #include <drm/drmP.h> #include <drm/i915_drm.h> #include "i915_drv.h" diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 44688e2..0ebe11f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -27,7 +27,7 @@
#include <linux/bitops.h> #include <linux/list.h> -#include <linux/radix-tree.h> +#include <linux/radix-tree-root.h>
struct pid;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 92437f4..af9ee58 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -27,6 +27,7 @@ */
#include <linux/dma_remapping.h> +#include <linux/radix-tree.h> #include <linux/reservation.h> #include <linux/sync_file.h> #include <linux/uaccess.h> diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h index c30d8f8..a5a5506 100644 --- a/drivers/gpu/drm/i915/i915_gem_object.h +++ b/drivers/gpu/drm/i915/i915_gem_object.h @@ -25,6 +25,7 @@ #ifndef __I915_GEM_OBJECT_H__ #define __I915_GEM_OBJECT_H__
+#include <linux/radix-tree-root.h> #include <linux/reservation.h>
#include <drm/drm_vma_manager.h>
On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
<...>
By splitting out the radix_tree_root definition, we can reduce the header file dependency.
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
If we judge by the diffstat of this series, there won't be any visible change in anything mentioned above.
<...>
Masahiro Yamada (12): radix-tree: replace <linux/spinlock.h> with <linux/spinlock_types.h> radix-tree: split struct radix_tree_root to <linux/radix-tree-root.h> irqdomain: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> writeback: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> iocontext.h: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> fs: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> blkcg: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> fscache: include <linux-radix-tree.h> sh: intc: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> net/mlx4: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> net/mlx5: replace <linux/radix-tree.h> with <linux/radix-tree-root.h> drm/i915: replace <linux/radix-tree.h> with <linux/radix-tree-root.h>
drivers/gpu/drm/i915/i915_gem.c | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 1 + drivers/gpu/drm/i915/i915_gem_context.h | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 + drivers/gpu/drm/i915/i915_gem_object.h | 1 + drivers/net/ethernet/mellanox/mlx4/cq.c | 1 + drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +- drivers/net/ethernet/mellanox/mlx4/qp.c | 1 + drivers/net/ethernet/mellanox/mlx4/srq.c | 1 + drivers/sh/intc/internals.h | 2 +- include/linux/backing-dev-defs.h | 2 +- include/linux/blk-cgroup.h | 2 +- include/linux/fs.h | 2 +- include/linux/fscache.h | 1 + include/linux/iocontext.h | 2 +- include/linux/irqdomain.h | 2 +- include/linux/mlx4/device.h | 2 +- include/linux/mlx4/qp.h | 1 + include/linux/mlx5/driver.h | 2 +- include/linux/mlx5/qp.h | 1 + include/linux/radix-tree-root.h | 24 ++++++++++++++++++++++++ include/linux/radix-tree.h | 8 ++------ 22 files changed, 46 insertions(+), 16 deletions(-) create mode 100644 include/linux/radix-tree-root.h
-- 2.7.4
2017-10-09 3:52 GMT+09:00 Leon Romanovsky leonro@mellanox.com:
On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
<...>
By splitting out the radix_tree_root definition, we can reduce the header file dependency.
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
If we judge by the diffstat of this series, there won't be any visible change in anything mentioned above.
Of course, judging by the diffstat is wrong.
On Mon, Oct 09, 2017 at 02:58:58PM +0900, Masahiro Yamada wrote:
2017-10-09 3:52 GMT+09:00 Leon Romanovsky leonro@mellanox.com:
On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
<...>
By splitting out the radix_tree_root definition, we can reduce the header file dependency.
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
If we judge by the diffstat of this series, there won't be any visible change in anything mentioned above.
Of course, judging by the diffstat is wrong.
I'm more than happy to be wrong and you for sure can help me. Can you provide any quantitative support of your claims?
Thanks
-- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
Well, does it? You could provide measurements showing before/after time to compile, or time to recompile after touching a header file that is included by radix-tree.h and not by radix-tree-root.h.
Look at the files included (never mind the transitively included files):
#include <linux/bitops.h> #include <linux/bug.h> #include <linux/kernel.h> #include <linux/list.h> #include <linux/preempt.h> #include <linux/rcupdate.h> #include <linux/spinlock.h> #include <linux/types.h>
These are not exactly rare files to be included. My guess is that most of the files in the kernel end up depending on these files *anyway*, either directly or through some path that isn't the radix tree.
2017-10-10 21:18 GMT+09:00 Matthew Wilcox willy@infradead.org:
On Mon, Oct 09, 2017 at 01:10:01AM +0900, Masahiro Yamada wrote:
Reducing the header dependency will help for speeding the kernel build, suppressing unnecessary recompile of objects during git-bisect'ing, etc.
Well, does it? You could provide measurements showing before/after time to compile, or time to recompile after touching a header file that is included by radix-tree.h and not by radix-tree-root.h.
Look at the files included (never mind the transitively included files):
#include <linux/bitops.h> #include <linux/bug.h> #include <linux/kernel.h> #include <linux/list.h> #include <linux/preempt.h> #include <linux/rcupdate.h> #include <linux/spinlock.h> #include <linux/types.h>
These are not exactly rare files to be included. My guess is that most of the files in the kernel end up depending on these files *anyway*, either directly or through some path that isn't the radix tree.
Good question.
I tested this series, and I confirmed the total number of included headers decreased, but did not decrease as much as I had expected.
The statement "most of the files in the kernel end up depending on these files" is true.
But, with that excuse, I do not want to conclude this kind of refactoring is pointless.
For example, how can we explain commit bc6245e5efd70c41eaf9334b1b5e646745cb0fb3 ?
<linux/bug.h> includes the following three.
#include <asm/bug.h> #include <linux/compiler.h> #include <linux/build_bug.h>
Your statement applies to them too. Actually, I did not see any impact by replacing <linux/bug.h> in my files with <linux/build_bug.h>
Generally, people do not pay much attention in decreasing header dependency.
One refactoring alone does not produce much benefits, but making continuous efforts will disentangle the knotted threads. Of course, this might be a pipe dream...
On Tue, Oct 10, 2017 at 09:56:22PM +0900, Masahiro Yamada wrote:
One refactoring alone does not produce much benefits, but making continuous efforts will disentangle the knotted threads. Of course, this might be a pipe dream...
A lot of people have had that dream, and some of those refactoring efforts have proven worthwhile. But it's not a dream without costs; your refactoring will conflict with other changes. I don't think the benefit here is high enough to pursue this edition of the dream.
dri-devel@lists.freedesktop.org
-
Leon Romanovsky -
Masahiro Yamada -
Matthew Wilcox