Re: [PATCH] packet: Don't include <linux/rculist.h>

[Christophe JAILLET]

Le 05/12/2022 à 06:24, Eric Dumazet a écrit :
> On Sat, Dec 3, 2022 at 5:49 PM Christophe JAILLET
> <christophe.jaillet@xxxxxxxxxx> wrote:
> > There is no need to include <linux/rculist.h> here.
> >
> > Prefer the less invasive <linux/types.h> which is needed for 'hlist_head'.
> >
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> > ---
> > Let see if build-bots agree with me!
>
> net/packet/af_packet.c does not explicitly include linux/rculist.h
>
> It might be provided by include/linux/netdevice.h, but I wonder if
> this is best practice.

At least, it is not what I expect.

My goal is to avoid some unneeded includes AND the related indirect 
needed includes that are buried somewhere in the dependency hell.

I missed the one in af_packet.c

I'll repost a v2 with the fix for af_packet.c (and double-check if some 
other are also needed)

> > Just declaring 'struct mutex' and 'struct hlist_head' would also be an
> > option.
>
> I do not get it, see [1]

Just forget about it.

Requirement for:
   struct my_struct {
          struct another_struct            x;

and
   struct my_struct {
          struct another_struct            *x;
                                          ~~~
are not the same, even if 'my_struct' is not used at all...

(*ashamed *)

CJ

> > It would remove the need of any include, but is more likely to break
> > something.
>
> I do not see why you are even trying this ?
>
> > ---
> >   include/net/netns/packet.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/net/netns/packet.h b/include/net/netns/packet.h
> > index aae69bb43cde..74750865df36 100644
> > --- a/include/net/netns/packet.h
> > +++ b/include/net/netns/packet.h
> > @@ -5,8 +5,8 @@
> >   #ifndef __NETNS_PACKET_H__
> >   #define __NETNS_PACKET_H__
> >
> > -#include <linux/rculist.h>
> >   #include <linux/mutex.h>
> > +#include <linux/types.h>
> >
> >   struct netns_packet {
> >          struct mutex            sklist_lock;
>
> [1] Definition of 'struct mutex' is definitely needed here.
>
> > --
> > 2.34.1