Re: [PATCH] net: octeontx2: fix building without DEBUGFS

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch does not fix the CONFIG_DEBUG_FS=n build.
- [Low] The Fixes tag points to the wrong commit.
- [Low] The commit message says making the member unconditional will
  "waste the 8 bytes in the structure in this config".

--

net: octeontx2: fix building without DEBUGFS

This commit removes the CONFIG_DEBUG_FS guard around the rvu_dbg member
of struct rvu. The goal is to let the rvu->rvu_dbg.npc references in
cn20k/debugfs.c build when debugfs is disabled.

> Remove the #ifdef and waste the 8 bytes in the structure in this
> config.

[Severity: Low]
Is the 8 byte figure accurate? rvu_dbg is embedded in struct rvu by
value, not as a pointer.

struct rvu_debugfs contains 12 struct dentry pointers, 6 struct dump_ctx,
a cpt_ctx[MAX_CPT_BLKS] array (MAX_CPT_BLKS is 2) and two ints:

struct rvu_debugfs {
	struct dentry *root;
	...
	struct dump_ctx nix_tm_ctx;
	struct cpt_ctx cpt_ctx[MAX_CPT_BLKS];
	int npa_qsize_id;
	int nix_qsize_id;
};

That comes to about 208 bytes on 64-bit.

> Fixes: 45229e9a9ab5 ("octeontx2-af: Extend debugfs support for cn20k NIX")

[Severity: Low]
Is this the right commit for the Fixes tag?

At 45229e9a9ab5, rvu_debugfs.c holds the only references to rvu_dbg, and
that file is fully wrapped in #ifdef CONFIG_DEBUG_FS. At that point
cn20k/debugfs.c contained only seq_file print helpers.

The always-built cn20k/debugfs.c seems to have first referenced
rvu->rvu_dbg in 1396771b0b7741 ("octeontx2-af: npc: cn20k: Index
management"), which added:

void npc_cn20k_debugfs_deinit(struct rvu *rvu)
{
	debugfs_remove_recursive(rvu->rvu_dbg.npc);
}

The debugfs_create_file() call quoted in the error message came later,
in 1100af13fd14b5. That may also explain why the error only showed up
recently.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> index fb4870cd18e9e..3b647e7b4bc20 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> @@ -649,9 +649,7 @@ struct rvu {
>  	int			mcs_blk_cnt;
>  	int			cpt_pf_num;
>  
> -#ifdef CONFIG_DEBUG_FS
>  	struct rvu_debugfs	rvu_dbg;
> -#endif

[Severity: High]
Does this build with CONFIG_DEBUG_FS=n?

The definition of struct rvu_debugfs is still inside an
#ifdef CONFIG_DEBUG_FS block earlier in rvu.h, with no #else. So are the
struct dump_ctx and struct cpt_ctx types it embeds:

#ifdef CONFIG_DEBUG_FS

struct dump_ctx {
...
struct rvu_debugfs {
	struct dentry *root;
	...
	int nix_qsize_id;
};
#endif

With OCTEONTX2_AF enabled and DEBUG_FS disabled, struct rvu now has a
by-value member of incomplete type. The compiler would reject that with
"field 'rvu_dbg' has incomplete type".

Every rvu_af object includes rvu.h (rvu.o, rvu_nix.o, rvu_npc.o,
cn20k/*.o, ...). Wouldn't this turn the one error in cn20k/debugfs.c into
a failure in every rvu_af translation unit?

OCTEONTX2_AF does not depend on DEBUG_FS in octeontx2/Kconfig, so this
config can be selected.

Would either of these work instead?

- Move the #ifdef CONFIG_DEBUG_FS so that dump_ctx, cpt_ctx and
  rvu_debugfs are always defined.
- Build cn20k/debugfs.o only when CONFIG_DEBUG_FS is set.

The Makefile currently builds cn20k/debugfs.o unconditionally:

rvu_af-y := cgx.o rvu.o rvu_cgx.o rvu_npa.o rvu_nix.o \
	...
		  rvu_rep.o cn20k/mbox_init.o cn20k/nix.o cn20k/debugfs.o \

>  	struct rvu_devlink	*rvu_dl;
>  
>  	/* RVU switch implementation over NPC with DMAC rules */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027151737.386155-1-arnd%40kernel.org