| Message ID | 20241009121235.4967-1-a@unstable.cc (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tools: ynl-gen: include auto-generated uAPI header only once | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Wed, Oct 09, 2024 at 02:12:35PM CEST, a@unstable.cc wrote: >The auto-generated uAPI file is currently included in both the >.h and .c netlink stub files. >However, the .c file already includes its .h counterpart, thus >leading to a double inclusion of the uAPI header. > >Prevent the double inclusion by including the uAPI header in the >.h stub file only. > >Signed-off-by: Antonio Quartulli <a@unstable.cc> >--- > drivers/dpll/dpll_nl.c | 2 -- > drivers/net/team/team_nl.c | 2 -- > fs/nfsd/netlink.c | 2 -- > net/core/netdev-genl-gen.c | 1 - > net/devlink/netlink_gen.c | 2 -- > net/handshake/genl.c | 2 -- > net/ipv4/fou_nl.c | 2 -- > net/mptcp/mptcp_pm_gen.c | 2 -- > tools/net/ynl/ynl-gen-c.py | 4 +++- > 9 files changed, 3 insertions(+), 16 deletions(-) > >diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c >index fe9b6893d261..9a739d9dcfbd 100644 >--- a/drivers/dpll/dpll_nl.c >+++ b/drivers/dpll/dpll_nl.c >@@ -8,8 +8,6 @@ > > #include "dpll_nl.h" > >-#include <uapi/linux/dpll.h> What seems to be the problem? The uapi headers are protected for double inclusion, no? #ifndef _UAPI_LINUX_DPLL_H #define _UAPI_LINUX_DPLL_H
On 09/10/2024 14:50, Jiri Pirko wrote: > Wed, Oct 09, 2024 at 02:12:35PM CEST, a@unstable.cc wrote: >> The auto-generated uAPI file is currently included in both the >> .h and .c netlink stub files. >> However, the .c file already includes its .h counterpart, thus >> leading to a double inclusion of the uAPI header. >> >> Prevent the double inclusion by including the uAPI header in the >> .h stub file only. >> >> Signed-off-by: Antonio Quartulli <a@unstable.cc> >> --- >> drivers/dpll/dpll_nl.c | 2 -- >> drivers/net/team/team_nl.c | 2 -- >> fs/nfsd/netlink.c | 2 -- >> net/core/netdev-genl-gen.c | 1 - >> net/devlink/netlink_gen.c | 2 -- >> net/handshake/genl.c | 2 -- >> net/ipv4/fou_nl.c | 2 -- >> net/mptcp/mptcp_pm_gen.c | 2 -- >> tools/net/ynl/ynl-gen-c.py | 4 +++- >> 9 files changed, 3 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c >> index fe9b6893d261..9a739d9dcfbd 100644 >> --- a/drivers/dpll/dpll_nl.c >> +++ b/drivers/dpll/dpll_nl.c >> @@ -8,8 +8,6 @@ >> >> #include "dpll_nl.h" >> >> -#include <uapi/linux/dpll.h> > > What seems to be the problem? The uapi headers are protected for double > inclusion, no? > #ifndef _UAPI_LINUX_DPLL_H > #define _UAPI_LINUX_DPLL_H There is no problem to fix, this is just a compile-time micro-optimization by reducing the number of includes to follow. I was recently told there is ongoing effort to reduce the amount of useless includes in order to speed up the compilation process. I thought this was an easy fix in this direction :) Regards,
Wed, Oct 09, 2024 at 03:00:42PM CEST, a@unstable.cc wrote: >On 09/10/2024 14:50, Jiri Pirko wrote: >> Wed, Oct 09, 2024 at 02:12:35PM CEST, a@unstable.cc wrote: >> > The auto-generated uAPI file is currently included in both the >> > .h and .c netlink stub files. >> > However, the .c file already includes its .h counterpart, thus >> > leading to a double inclusion of the uAPI header. >> > >> > Prevent the double inclusion by including the uAPI header in the >> > .h stub file only. >> > >> > Signed-off-by: Antonio Quartulli <a@unstable.cc> >> > --- >> > drivers/dpll/dpll_nl.c | 2 -- >> > drivers/net/team/team_nl.c | 2 -- >> > fs/nfsd/netlink.c | 2 -- >> > net/core/netdev-genl-gen.c | 1 - >> > net/devlink/netlink_gen.c | 2 -- >> > net/handshake/genl.c | 2 -- >> > net/ipv4/fou_nl.c | 2 -- >> > net/mptcp/mptcp_pm_gen.c | 2 -- >> > tools/net/ynl/ynl-gen-c.py | 4 +++- >> > 9 files changed, 3 insertions(+), 16 deletions(-) >> > >> > diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c >> > index fe9b6893d261..9a739d9dcfbd 100644 >> > --- a/drivers/dpll/dpll_nl.c >> > +++ b/drivers/dpll/dpll_nl.c >> > @@ -8,8 +8,6 @@ >> > >> > #include "dpll_nl.h" >> > >> > -#include <uapi/linux/dpll.h> >> >> What seems to be the problem? The uapi headers are protected for double >> inclusion, no? >> #ifndef _UAPI_LINUX_DPLL_H >> #define _UAPI_LINUX_DPLL_H > >There is no problem to fix, this is just a compile-time micro-optimization by >reducing the number of includes to follow. > >I was recently told there is ongoing effort to reduce the amount of useless >includes in order to speed up the compilation process. Do you have some numbers? So far I had impression that the common practise is to include header directly when needed and not to depend on indirect include. > >I thought this was an easy fix in this direction :) > >Regards, > >-- >Antonio Quartulli
On 09/10/2024 15:37, Jiri Pirko wrote: > Wed, Oct 09, 2024 at 03:00:42PM CEST, a@unstable.cc wrote: >> On 09/10/2024 14:50, Jiri Pirko wrote: >>> Wed, Oct 09, 2024 at 02:12:35PM CEST, a@unstable.cc wrote: >>>> The auto-generated uAPI file is currently included in both the >>>> .h and .c netlink stub files. >>>> However, the .c file already includes its .h counterpart, thus >>>> leading to a double inclusion of the uAPI header. >>>> >>>> Prevent the double inclusion by including the uAPI header in the >>>> .h stub file only. >>>> >>>> Signed-off-by: Antonio Quartulli <a@unstable.cc> >>>> --- >>>> drivers/dpll/dpll_nl.c | 2 -- >>>> drivers/net/team/team_nl.c | 2 -- >>>> fs/nfsd/netlink.c | 2 -- >>>> net/core/netdev-genl-gen.c | 1 - >>>> net/devlink/netlink_gen.c | 2 -- >>>> net/handshake/genl.c | 2 -- >>>> net/ipv4/fou_nl.c | 2 -- >>>> net/mptcp/mptcp_pm_gen.c | 2 -- >>>> tools/net/ynl/ynl-gen-c.py | 4 +++- >>>> 9 files changed, 3 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c >>>> index fe9b6893d261..9a739d9dcfbd 100644 >>>> --- a/drivers/dpll/dpll_nl.c >>>> +++ b/drivers/dpll/dpll_nl.c >>>> @@ -8,8 +8,6 @@ >>>> >>>> #include "dpll_nl.h" >>>> >>>> -#include <uapi/linux/dpll.h> >>> >>> What seems to be the problem? The uapi headers are protected for double >>> inclusion, no? >>> #ifndef _UAPI_LINUX_DPLL_H >>> #define _UAPI_LINUX_DPLL_H >> >> There is no problem to fix, this is just a compile-time micro-optimization by >> reducing the number of includes to follow. >> >> I was recently told there is ongoing effort to reduce the amount of useless >> includes in order to speed up the compilation process. > > Do you have some numbers? > > So far I had impression that the common practise is to include header > directly when needed and not to depend on indirect include. That's the same rule I had been following in the past, but Andrew advised to do differently here: <07050ffc-aa8e-417a-b35b-0cf627fc226f@lunn.ch> He also says: "There is a massive patch series crossing the entire kernel which significantly reduces the kernel build time by optimising includes." I trusted his word and I just assumed this was the wanted direction nowadays. @Andrew: maybe you have a pointer to the series? Thanks. Regards,
On Wed, 9 Oct 2024 14:12:35 +0200 Antonio Quartulli wrote: > The auto-generated uAPI file is currently included in both the > .h and .c netlink stub files. > However, the .c file already includes its .h counterpart, thus > leading to a double inclusion of the uAPI header. > > Prevent the double inclusion by including the uAPI header in the > .h stub file only. FWIW I don't have a strong opinion either way. Since both files are auto-generated there is no risk of getting it wrong. Then again the win is fairly minor if any. But if we want to do this why only remove the uapi header? Netlink headers are also included twice.
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c index fe9b6893d261..9a739d9dcfbd 100644 --- a/drivers/dpll/dpll_nl.c +++ b/drivers/dpll/dpll_nl.c @@ -8,8 +8,6 @@ #include "dpll_nl.h" -#include <uapi/linux/dpll.h> - /* Common nested types */ const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_PHASE_OFFSET + 1] = { [DPLL_A_PIN_PARENT_ID] = { .type = NLA_U32, }, diff --git a/drivers/net/team/team_nl.c b/drivers/net/team/team_nl.c index 208424ab78f5..b00fec07a8ca 100644 --- a/drivers/net/team/team_nl.c +++ b/drivers/net/team/team_nl.c @@ -8,8 +8,6 @@ #include "team_nl.h" -#include <uapi/linux/if_team.h> - /* Common nested types */ const struct nla_policy team_attr_option_nl_policy[TEAM_ATTR_OPTION_ARRAY_INDEX + 1] = { [TEAM_ATTR_OPTION_NAME] = { .type = NLA_STRING, .len = TEAM_STRING_MAX_LEN, }, diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c index ca54aa583530..c59264f71ef6 100644 --- a/fs/nfsd/netlink.c +++ b/fs/nfsd/netlink.c @@ -8,8 +8,6 @@ #include "netlink.h" -#include <uapi/linux/nfsd_netlink.h> - /* Common nested types */ const struct nla_policy nfsd_sock_nl_policy[NFSD_A_SOCK_TRANSPORT_NAME + 1] = { [NFSD_A_SOCK_ADDR] = { .type = NLA_BINARY, }, diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c index b28424ae06d5..a5c6ca79c7a4 100644 --- a/net/core/netdev-genl-gen.c +++ b/net/core/netdev-genl-gen.c @@ -8,7 +8,6 @@ #include "netdev-genl-gen.h" -#include <uapi/linux/netdev.h> #include <linux/list.h> /* Integer value ranges */ diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c index f9786d51f68f..c65a82593e76 100644 --- a/net/devlink/netlink_gen.c +++ b/net/devlink/netlink_gen.c @@ -8,8 +8,6 @@ #include "netlink_gen.h" -#include <uapi/linux/devlink.h> - /* Common nested types */ const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1] = { [DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, }, diff --git a/net/handshake/genl.c b/net/handshake/genl.c index f55d14d7b726..8d18c0a0ca04 100644 --- a/net/handshake/genl.c +++ b/net/handshake/genl.c @@ -8,8 +8,6 @@ #include "genl.h" -#include <uapi/linux/handshake.h> - /* HANDSHAKE_CMD_ACCEPT - do */ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HANDLER_CLASS + 1] = { [HANDSHAKE_A_ACCEPT_HANDLER_CLASS] = NLA_POLICY_MAX(NLA_U32, 2), diff --git a/net/ipv4/fou_nl.c b/net/ipv4/fou_nl.c index 98b90107b5ab..7d3e107eacda 100644 --- a/net/ipv4/fou_nl.c +++ b/net/ipv4/fou_nl.c @@ -8,8 +8,6 @@ #include "fou_nl.h" -#include <uapi/linux/fou.h> - /* Global operation policy for fou */ const struct nla_policy fou_nl_policy[FOU_ATTR_IFINDEX + 1] = { [FOU_ATTR_PORT] = { .type = NLA_U16, }, diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c index c30a2a90a192..d85c4540d6e8 100644 --- a/net/mptcp/mptcp_pm_gen.c +++ b/net/mptcp/mptcp_pm_gen.c @@ -8,8 +8,6 @@ #include "mptcp_pm_gen.h" -#include <uapi/linux/mptcp_pm.h> - /* Common nested types */ const struct nla_policy mptcp_pm_address_nl_policy[MPTCP_PM_ADDR_ATTR_IF_IDX + 1] = { [MPTCP_PM_ADDR_ATTR_FAMILY] = { .type = NLA_U16, }, diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 9e8254aad578..341be0a4bcb7 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -2683,7 +2683,9 @@ def main(): if args.out_file: cw.p(f'#include "{hdr_file}"') cw.nl() - headers = ['uapi/' + parsed.uapi_header] + headers = [] + else: + headers = ['uapi/' + parsed.uapi_header] headers += parsed.kernel_family.get('headers', []) else: cw.p('#include <stdlib.h>')
The auto-generated uAPI file is currently included in both the .h and .c netlink stub files. However, the .c file already includes its .h counterpart, thus leading to a double inclusion of the uAPI header. Prevent the double inclusion by including the uAPI header in the .h stub file only. Signed-off-by: Antonio Quartulli <a@unstable.cc> --- drivers/dpll/dpll_nl.c | 2 -- drivers/net/team/team_nl.c | 2 -- fs/nfsd/netlink.c | 2 -- net/core/netdev-genl-gen.c | 1 - net/devlink/netlink_gen.c | 2 -- net/handshake/genl.c | 2 -- net/ipv4/fou_nl.c | 2 -- net/mptcp/mptcp_pm_gen.c | 2 -- tools/net/ynl/ynl-gen-c.py | 4 +++- 9 files changed, 3 insertions(+), 16 deletions(-)