Re: [patch 22/30] net/mlx5: Replace irq_to_desc() abuse
On Thu, 2020-12-10 at 20:25 +0100, Thomas Gleixner wrote:
> No driver has any business with the internals of an interrupt
> descriptor. Storing a pointer to it just to use yet another helper at
> the
> actual usage site to retrieve the affinity mask is creative at best.
> Just
> because C does not allow encapsulation does not mean that the kernel
> has no
> limits.
>
you can't blame the developers for using stuff from include/linux/
Not all developers are the same, and sometime we don't read in between
the lines, you can't assume all driver developers to be expert on irq
APIs disciplines.
your rules must be programmatically expressed, for instance,
you can just hide struct irq_desc and irq_to_desc() in kernel/irq/ and
remove them from include/linux/ header files, if you want privacy in
your subsystem, don't put all your header files on display under
include/linux.
> Retrieve a pointer to the affinity mask itself and use that. It's
> still
> using an interface which is usually not for random drivers, but
> definitely
> less hideous than the previous hack.
>
> Signed-off-by: Thomas Gleixner
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h |2 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c |2 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |6 +-
> 3 files changed, 3 insertions(+), 7 deletions(-)
>
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -669,7 +669,7 @@ struct mlx5e_channel {
> spinlock_t async_icosq_lock;
>
> /* data path - accessed per napi poll */
> - struct irq_desc *irq_desc;
> + const struct cpumask *aff_mask;
> struct mlx5e_ch_stats *stats;
>
> /* control */
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
> c->num_tc = params->num_tc;
> c->xdp = !!params->xdp_prog;
> c->stats= >channel_stats[ix].ch;
> - c->irq_desc = irq_to_desc(irq);
> + c->aff_mask = irq_get_affinity_mask(irq);
as long as the affinity mask pointer stays the same for the lifetime of
the irq vector.
Assuming that:
Acked-by: Saeed Mahameed
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 22/30] net/mlx5: Replace irq_to_desc() abuse
On 12/10/2020 9:25 PM, Thomas Gleixner wrote: No driver has any business with the internals of an interrupt descriptor. Storing a pointer to it just to use yet another helper at the actual usage site to retrieve the affinity mask is creative at best. Just because C does not allow encapsulation does not mean that the kernel has no limits. Retrieve a pointer to the affinity mask itself and use that. It's still using an interface which is usually not for random drivers, but definitely less hideous than the previous hack. Signed-off-by: Thomas Gleixner --- drivers/net/ethernet/mellanox/mlx5/core/en.h |2 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c |2 +- drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |6 +- 3 files changed, 3 insertions(+), 7 deletions(-) Reviewed-by: Tariq Toukan Thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[patch 22/30] net/mlx5: Replace irq_to_desc() abuse
No driver has any business with the internals of an interrupt
descriptor. Storing a pointer to it just to use yet another helper at the
actual usage site to retrieve the affinity mask is creative at best. Just
because C does not allow encapsulation does not mean that the kernel has no
limits.
Retrieve a pointer to the affinity mask itself and use that. It's still
using an interface which is usually not for random drivers, but definitely
less hideous than the previous hack.
Signed-off-by: Thomas Gleixner
---
drivers/net/ethernet/mellanox/mlx5/core/en.h |2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c |2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |6 +-
3 files changed, 3 insertions(+), 7 deletions(-)
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -669,7 +669,7 @@ struct mlx5e_channel {
spinlock_t async_icosq_lock;
/* data path - accessed per napi poll */
- struct irq_desc *irq_desc;
+ const struct cpumask *aff_mask;
struct mlx5e_ch_stats *stats;
/* control */
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -1998,7 +1998,7 @@ static int mlx5e_open_channel(struct mlx
c->num_tc = params->num_tc;
c->xdp = !!params->xdp_prog;
c->stats= >channel_stats[ix].ch;
- c->irq_desc = irq_to_desc(irq);
+ c->aff_mask = irq_get_affinity_mask(irq);
c->lag_port = mlx5e_enumerate_lag_port(priv->mdev, ix);
netif_napi_add(netdev, >napi, mlx5e_napi_poll, 64);
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -40,12 +40,8 @@
static inline bool mlx5e_channel_no_affinity_change(struct mlx5e_channel *c)
{
int current_cpu = smp_processor_id();
- const struct cpumask *aff;
- struct irq_data *idata;
- idata = irq_desc_get_irq_data(c->irq_desc);
- aff = irq_data_get_affinity_mask(idata);
- return cpumask_test_cpu(current_cpu, aff);
+ return cpumask_test_cpu(current_cpu, c->aff_mask);
}
static void mlx5e_handle_tx_dim(struct mlx5e_txqsq *sq)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
