| Message ID | 20250328131022.452068-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RDMA/mlx5: hide unused code | expand |
On Fri, Mar 28, 2025 at 02:10:17PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > After a recent rework, a few 'static const' objects have become unused: > > In file included from drivers/infiniband/hw/mlx5/fs.c:27: > drivers/infiniband/hw/mlx5/fs.c:26:28: error: 'mlx5_ib_object_MLX5_IB_OBJECT_STEERING_ANCHOR' defined but not used [-Werror=unused-const-variable=] > include/rdma/uverbs_named_ioctl.h:52:47: note: in expansion of macro 'UVERBS_OBJECT' > 52 | static const struct uverbs_object_def UVERBS_OBJECT(_object_id) = { \ > | ^~~~~~~~~~~~~ > drivers/infiniband/hw/mlx5/fs.c:3457:1: note: in expansion of macro 'DECLARE_UVERBS_NAMED_OBJECT' > 3457 | DECLARE_UVERBS_NAMED_OBJECT( > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/infiniband/hw/mlx5/fs.c:26:28: error: 'mlx5_ib_object_MLX5_IB_OBJECT_FLOW_MATCHER' defined but not used [-Werror=unused-const-variable=] > include/rdma/uverbs_named_ioctl.h:52:47: note: in expansion of macro 'UVERBS_OBJECT' > 52 | static const struct uverbs_object_def UVERBS_OBJECT(_object_id) = { \ > | ^~~~~~~~~~~~~ > drivers/infiniband/hw/mlx5/fs.c:3429:1: note: in expansion of macro 'DECLARE_UVERBS_NAMED_OBJECT' > 3429 | DECLARE_UVERBS_NAMED_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > > These come from a complex set of macros, and it would be possible to > shut up the warnings here by adding __maybe_unused annotations inside > of the macros, it seems cleaner in this case to have a large #ifdef block > around all the unused parts of the file, in order to still be able to > catch unused ones elsewhere. IDK, I'm tempted to revert 36e0d433672f ("RDMA/mlx5: Compile fs.c regardless of INFINIBAND_USER_ACCESS config") I don't think that was so well thought out. The entire file was designed to be USER_ACCESS only because it uses all this mechanism. #ifdefing away half the file seems ugly. Jason
On Fri, Mar 28, 2025, at 14:15, Jason Gunthorpe wrote: > On Fri, Mar 28, 2025 at 02:10:17PM +0100, Arnd Bergmann wrote: > > IDK, I'm tempted to revert 36e0d433672f ("RDMA/mlx5: Compile fs.c > regardless of INFINIBAND_USER_ACCESS config") > > I don't think that was so well thought out. The entire file was > designed to be USER_ACCESS only because it uses all this mechanism. > > #ifdefing away half the file seems ugly. That sounds better to me, yes. I assumed that a later patch required that one, but I don't see such a dependency. Arnd
On 28/03/2025 16:15, Jason Gunthorpe wrote: > On Fri, Mar 28, 2025 at 02:10:17PM +0100, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> After a recent rework, a few 'static const' objects have become unused: >> >> In file included from drivers/infiniband/hw/mlx5/fs.c:27: >> drivers/infiniband/hw/mlx5/fs.c:26:28: error: 'mlx5_ib_object_MLX5_IB_OBJECT_STEERING_ANCHOR' defined but not used [-Werror=unused-const-variable=] >> include/rdma/uverbs_named_ioctl.h:52:47: note: in expansion of macro 'UVERBS_OBJECT' >> 52 | static const struct uverbs_object_def UVERBS_OBJECT(_object_id) = { \ >> | ^~~~~~~~~~~~~ >> drivers/infiniband/hw/mlx5/fs.c:3457:1: note: in expansion of macro 'DECLARE_UVERBS_NAMED_OBJECT' >> 3457 | DECLARE_UVERBS_NAMED_OBJECT( >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> drivers/infiniband/hw/mlx5/fs.c:26:28: error: 'mlx5_ib_object_MLX5_IB_OBJECT_FLOW_MATCHER' defined but not used [-Werror=unused-const-variable=] >> include/rdma/uverbs_named_ioctl.h:52:47: note: in expansion of macro 'UVERBS_OBJECT' >> 52 | static const struct uverbs_object_def UVERBS_OBJECT(_object_id) = { \ >> | ^~~~~~~~~~~~~ >> drivers/infiniband/hw/mlx5/fs.c:3429:1: note: in expansion of macro 'DECLARE_UVERBS_NAMED_OBJECT' >> 3429 | DECLARE_UVERBS_NAMED_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER, >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> These come from a complex set of macros, and it would be possible to >> shut up the warnings here by adding __maybe_unused annotations inside >> of the macros, it seems cleaner in this case to have a large #ifdef block >> around all the unused parts of the file, in order to still be able to >> catch unused ones elsewhere. > > IDK, I'm tempted to revert 36e0d433672f ("RDMA/mlx5: Compile fs.c > regardless of INFINIBAND_USER_ACCESS config") I believe the issue arises because uverbs_destroy_def_handler() is declared in ib_verbs.h, but if uverbs isn't built, there is no corresponding implementation of this function. One possible solution is to provide an empty implementation when USER_ACCESS is not set, similar to how rdma_user_mmap_disassociate() is handled. Alternatively, since uverbs_destroy_def_handler() currently does nothing (always returning 0), we could simply define it as a static inline function inside ib_verbs.h and resolve the issue that way. An example of the first approach would be: diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c index 251246c73b33..0ff9f18a71e8 100644 --- a/drivers/infiniband/hw/mlx5/fs.c +++ b/drivers/infiniband/hw/mlx5/fs.c @@ -3461,7 +3461,6 @@ DECLARE_UVERBS_NAMED_OBJECT( &UVERBS_METHOD(MLX5_IB_METHOD_STEERING_ANCHOR_DESTROY)); const struct uapi_definition mlx5_ib_flow_defs[] = { -#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) UAPI_DEF_CHAIN_OBJ_TREE_NAMED( MLX5_IB_OBJECT_FLOW_MATCHER), UAPI_DEF_CHAIN_OBJ_TREE( @@ -3472,7 +3471,6 @@ const struct uapi_definition mlx5_ib_flow_defs[] = { UAPI_DEF_CHAIN_OBJ_TREE_NAMED( MLX5_IB_OBJECT_STEERING_ANCHOR, UAPI_DEF_IS_OBJ_SUPPORTED(mlx5_ib_shared_ft_allowed)), -#endif {}, }; diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index d42eae69d9a8..901353796fbb 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -4790,7 +4790,14 @@ void roce_del_all_netdev_gids(struct ib_device *ib_dev, struct ib_ucontext *ib_uverbs_get_ucontext_file(struct ib_uverbs_file *ufile); +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) int uverbs_destroy_def_handler(struct uverbs_attr_bundle *attrs); +#else +static inline int uverbs_destroy_def_handler(struct uverbs_attr_bundle *attrs) +{ + return 0; +} +#endif struct net_device *rdma_alloc_netdev(struct ib_device *device, u32 port_num, enum rdma_netdev_t type, const char *name, > > I don't think that was so well thought out. The entire file was > designed to be USER_ACCESS only because it uses all this mechanism. > > #ifdefing away half the file seems ugly. > > Jason
diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c index 251246c73b33..f089abbed6af 100644 --- a/drivers/infiniband/hw/mlx5/fs.c +++ b/drivers/infiniband/hw/mlx5/fs.c @@ -684,12 +684,14 @@ enum flow_table_type { #define MLX5_FS_MAX_TYPES 6 #define MLX5_FS_MAX_ENTRIES BIT(16) -static bool __maybe_unused mlx5_ib_shared_ft_allowed(struct ib_device *device) +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) +static bool mlx5_ib_shared_ft_allowed(struct ib_device *device) { struct mlx5_ib_dev *dev = to_mdev(device); return MLX5_CAP_GEN(dev->mdev, shared_object_to_user_object_allowed); } +#endif static struct mlx5_ib_flow_prio *_get_prio(struct mlx5_ib_dev *dev, struct mlx5_flow_namespace *ns, @@ -1888,6 +1890,7 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp, return ERR_PTR(err); } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int mlx5_ib_fill_transport_ns_info(struct mlx5_ib_dev *dev, enum mlx5_flow_namespace_type type, u32 *flags, u16 *vport_idx, @@ -2227,6 +2230,7 @@ static struct mlx5_ib_flow_handler *raw_fs_rule_add( return ERR_PTR(err); } +#endif static void destroy_flow_action_raw(struct mlx5_ib_flow_action *maction) { @@ -2263,6 +2267,7 @@ static int mlx5_ib_destroy_flow_action(struct ib_flow_action *action) return 0; } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int mlx5_ib_ft_type_to_namespace(enum mlx5_ib_uapi_flow_table_type table_type, enum mlx5_flow_namespace_type *namespace) @@ -2618,6 +2623,7 @@ static int steering_anchor_create_ft(struct mlx5_ib_dev *dev, return 0; } +#endif static void steering_anchor_destroy_ft(struct mlx5_ib_flow_prio *ft_prio) { @@ -2627,6 +2633,7 @@ static void steering_anchor_destroy_ft(struct mlx5_ib_flow_prio *ft_prio) } } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int steering_anchor_create_fg_drop(struct mlx5_ib_flow_prio *ft_prio) { @@ -2658,6 +2665,7 @@ steering_anchor_create_fg_drop(struct mlx5_ib_flow_prio *ft_prio) return err; } +#endif static void steering_anchor_destroy_fg_drop(struct mlx5_ib_flow_prio *ft_prio) @@ -2668,6 +2676,7 @@ steering_anchor_destroy_fg_drop(struct mlx5_ib_flow_prio *ft_prio) } } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int steering_anchor_create_fg_goto_table(struct mlx5_ib_flow_prio *ft_prio) { @@ -2695,6 +2704,7 @@ steering_anchor_create_fg_goto_table(struct mlx5_ib_flow_prio *ft_prio) return err; } +#endif static void steering_anchor_destroy_fg_goto_table(struct mlx5_ib_flow_prio *ft_prio) @@ -2705,6 +2715,7 @@ steering_anchor_destroy_fg_goto_table(struct mlx5_ib_flow_prio *ft_prio) } } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int steering_anchor_create_rule_drop(struct mlx5_ib_flow_prio *ft_prio) { @@ -2726,6 +2737,7 @@ steering_anchor_create_rule_drop(struct mlx5_ib_flow_prio *ft_prio) return 0; } +#endif static void steering_anchor_destroy_rule_drop(struct mlx5_ib_flow_prio *ft_prio) { @@ -2735,6 +2747,7 @@ static void steering_anchor_destroy_rule_drop(struct mlx5_ib_flow_prio *ft_prio) } } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int steering_anchor_create_rule_goto_table(struct mlx5_ib_flow_prio *ft_prio) { @@ -2761,6 +2774,7 @@ steering_anchor_create_rule_goto_table(struct mlx5_ib_flow_prio *ft_prio) return 0; } +#endif static void steering_anchor_destroy_rule_goto_table(struct mlx5_ib_flow_prio *ft_prio) @@ -2771,6 +2785,7 @@ steering_anchor_destroy_rule_goto_table(struct mlx5_ib_flow_prio *ft_prio) } } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int steering_anchor_create_res(struct mlx5_ib_dev *dev, struct mlx5_ib_flow_prio *ft_prio, enum mlx5_flow_namespace_type ns_type) @@ -2810,6 +2825,7 @@ static int steering_anchor_create_res(struct mlx5_ib_dev *dev, return err; } +#endif static void mlx5_steering_anchor_destroy_res(struct mlx5_ib_flow_prio *ft_prio) { @@ -2820,6 +2836,7 @@ static void mlx5_steering_anchor_destroy_res(struct mlx5_ib_flow_prio *ft_prio) steering_anchor_destroy_ft(ft_prio); } +#ifdef CONFIG_INFINIBAND_USER_ACCESS static int steering_anchor_cleanup(struct ib_uobject *uobject, enum rdma_remove_reason why, struct uverbs_attr_bundle *attrs) @@ -2839,6 +2856,7 @@ static int steering_anchor_cleanup(struct ib_uobject *uobject, kfree(obj); return 0; } +#endif static void fs_cleanup_anchor(struct mlx5_ib_flow_prio *prio, int count) @@ -2858,6 +2876,7 @@ void mlx5_ib_fs_cleanup_anchor(struct mlx5_ib_dev *dev) fs_cleanup_anchor(dev->flow_db->rdma_tx, MLX5_IB_NUM_FLOW_FT); } +#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS) static int mlx5_ib_matcher_ns(struct uverbs_attr_bundle *attrs, struct mlx5_ib_flow_matcher *obj) { @@ -3459,6 +3478,7 @@ DECLARE_UVERBS_NAMED_OBJECT( UVERBS_TYPE_ALLOC_IDR(steering_anchor_cleanup), &UVERBS_METHOD(MLX5_IB_METHOD_STEERING_ANCHOR_CREATE), &UVERBS_METHOD(MLX5_IB_METHOD_STEERING_ANCHOR_DESTROY)); +#endif const struct uapi_definition mlx5_ib_flow_defs[] = { #if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS)