Re: [PATCH 10/12] net/mlx4: replace <linux/radix-tree.h> with <linux/radix-tree-root.h>

[Leon Romanovsky]

On Mon, Oct 09, 2017 at 02:29:15AM +0900, Masahiro Yamada wrote:
> 2017-10-09 2:00 GMT+09:00 David Miller <davem@xxxxxxxxxxxxx>:
> > From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > Date: Mon,  9 Oct 2017 01:10:11 +0900
> >
> >> The headers
> >>  - include/linux/mlx4/device.h
> >>  - drivers/net/ethernet/mellanox/mlx4/mlx4.h
> >> require the definition of struct radix_tree_root, but do 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 header dependency.
> >>
> >> 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@xxxxxxxxxxxxx>
> >
> > Honestly this makes things more complicated.
>
>
> The idea is simple; include necessary headers explicitly.
>
> Putting everything into one common header
> means most of C files are forced to parse unnecessary headers.

It is neglected, only first caller will actually parse that header file,
other callers will check the #ifndef pragma without need to reparse the
whole file.

>
>
>
> > The driver was trying to consolidate all of the header needs
> > by including them all in one place, the main driver header.
> >
> > Now you're including headers in several different files.
> >
> > I really don't like the results of this change and would
> > ask you to reconsider.
> >
> > Just add both radix-tree-root.h _and_ radix-tree.h to mlx4.h
> > and leave the rest of the driver alone.
>
>
> If you do not like this, you can just throw it away.
>
> <linux/radix-tree.h> includes <linux/radix-tree-root.h>.
> You do not need to include both.
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachment: signature.asc
Description: PGP signature