Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Leon Romanovsky]

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  with 
>   radix-tree: split struct radix_tree_root to 
>   irqdomain: replace  with 
>   writeback: replace  with 
>   iocontext.h: replace  with
> 
>   fs: replace  with 
>   blkcg: replace  with 
>   fscache: include 
>   sh: intc: replace  with 
>   net/mlx4: replace  with 
>   net/mlx5: replace  with 
>   drm/i915: replace  with 
>
>  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
>


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Matthew Wilcox]

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.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Leon Romanovsky]

On Mon, Oct 09, 2017 at 02:58:58PM +0900, Masahiro Yamada wrote:
> 2017-10-09 3:52 GMT+09:00 Leon Romanovsky :
> > 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Matthew Wilcox]

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 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

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.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Masahiro Yamada]

2017-10-10 21:18 GMT+09:00 Matthew Wilcox :
> 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 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
>
> 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 ?


 includes the following three.

#include 
#include 
#include 

Your statement applies to them too.
Actually, I did not see any impact
by replacing  in my files with 



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...



-- 
Best Regards
Masahiro Yamada
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Masahiro Yamada]

2017-10-09 3:52 GMT+09:00 Leon Romanovsky :
> 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.



-- 
Best Regards
Masahiro Yamada
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 00/12] radix-tree: split out struct radix_tree_root out to

[Masahiro Yamada]

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,  needs to include ,
therefore, users of  include a lot of bloat
from .

If you see the definition of radix_tree_root,

   struct radix_tree_root {
   gfp_tgfp_mask;
   struct radix_tree_node   __rcu *rnode;
   };

it is a very simple structure.
It only depends on  for gfp_t and
 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 
with  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  with 
  radix-tree: split struct radix_tree_root to 
  irqdomain: replace  with 
  writeback: replace  with 
  iocontext.h: replace  with

  fs: replace  with 
  blkcg: replace  with 
  fscache: include 
  sh: intc: replace  with 
  net/mlx4: replace  with 
  net/mlx5: replace  with 
  drm/i915: replace  with 

 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx