Hi,
This small series from Shahar adds the sensors names to the temperature
event messages, in addition to the existing bitmap indicators.
This improves human readability.
Series starts with simple refactoring and modifications. The top patch
adds the sensors names.
Regards,
Tariq
Shahar Shitrit (4):
net/mlx5: Apply rate-limiting to high temperature warning
net/mlx5: Prefix temperature event bitmap with '0x' for clarity
net/mlx5: Modify LSB bitmask in temperature event to include only the
first bit
net/mlx5: Add sensor name to temperature event message
.../net/ethernet/mellanox/mlx5/core/events.c | 36 +++++++++++++++++--
.../net/ethernet/mellanox/mlx5/core/hwmon.c | 5 +++
.../net/ethernet/mellanox/mlx5/core/hwmon.h | 1 +
3 files changed, 39 insertions(+), 3 deletions(-)
base-commit: 8dbf0c7556454b52af91bae305ca71500c31495c
--
2.45.0
From: Shahar Shitrit <redacted>
Wrap the high temperature warning in a temperature event with
a call to net_ratelimit() to prevent flooding the kernel log
with repeated warning messages when temperature exceeds the
threshold multiple times within a short duration.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
@@ -165,9 +165,10 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)value_lsb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_lsb);value_msb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_msb);-mlx5_core_warn(events->dev,-"High temperature on sensors with bit set %llx %llx",-value_msb,value_lsb);+if(net_ratelimit())+mlx5_core_warn(events->dev,+"High temperature on sensors with bit set %llx %llx",+value_msb,value_lsb);returnNOTIFY_OK;}
From: Shahar Shitrit <redacted>
Prepend '0x' to the sensor bitmap in the warning message to clearly
indicate that the bitmap is in hexadecimal format.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -167,7 +167,7 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)if(net_ratelimit())mlx5_core_warn(events->dev,-"High temperature on sensors with bit set %llx %llx",+"High temperature on sensors with bit set %#llx %#llx",value_msb,value_lsb);returnNOTIFY_OK;
From: Shahar Shitrit <redacted>
In the sensor_count field of the MTEWE register, bits 1-62 are
supported only for unmanaged switches, not for NICs, and bit 63
is reserved for internal use.
To prevent confusing output that may include set bits that are
not relevant to NIC sensors, we update the bitmask to retain only
the first bit, which corresponds to the sensor ASIC.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -163,6 +163,10 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)u64value_msb;value_lsb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_lsb);+/* bit 1-63 are not supported for NICs,+*hencereadonlybit0(asic)fromlsb.+*/+value_lsb&=0x1;value_msb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_msb);if(net_ratelimit())
From: Shahar Shitrit <redacted>
Previously, a temperature event message included a bitmap indicating
which sensors detect high temperatures.
To enhance clarity, we modify the message format to explicitly list
the names of the overheating sensors, alongside the sensors bitmap.
If HWMON is not configured, the event message remains unchanged.
Signed-off-by: Shahar Shitrit <redacted>
Reviewed-by: Carolina Jubran <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/events.c | 31 +++++++++++++++++--
.../net/ethernet/mellanox/mlx5/core/hwmon.c | 5 +++
.../net/ethernet/mellanox/mlx5/core/hwmon.h | 1 +
3 files changed, 34 insertions(+), 3 deletions(-)
@@ -153,11 +154,28 @@ static int any_notifier(struct notifier_block *nb,returnNOTIFY_OK;}+#if IS_ENABLED(CONFIG_HWMON)+staticvoidprint_sensor_names_in_bit_set(structmlx5_core_dev*dev,structmlx5_hwmon*hwmon,+u64bit_set,intbit_set_offset)+{+unsignedlong*bit_set_ptr=(unsignedlong*)&bit_set;+intnum_bits=sizeof(bit_set)*BITS_PER_BYTE;+inti;++for_each_set_bit(i,bit_set_ptr,num_bits){+constchar*sensor_name=hwmon_get_sensor_name(hwmon,i+bit_set_offset);++mlx5_core_warn(dev,"Sensor name[%d]: %s\n",i+bit_set_offset,sensor_name);+}+}+#endif /* CONFIG_HWMON */+/* type == MLX5_EVENT_TYPE_TEMP_WARN_EVENT */staticinttemp_warn(structnotifier_block*nb,unsignedlongtype,void*data){structmlx5_event_nb*event_nb=mlx5_nb_cof(nb,structmlx5_event_nb,nb);structmlx5_events*events=event_nb->ctx;+structmlx5_core_dev*dev=events->dev;structmlx5_eqe*eqe=data;u64value_lsb;u64value_msb;
@@ -169,10 +187,17 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)value_lsb&=0x1;value_msb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_msb);-if(net_ratelimit())-mlx5_core_warn(events->dev,-"High temperature on sensors with bit set %#llx %#llx",+if(net_ratelimit()){+mlx5_core_warn(dev,"High temperature on sensors with bit set %#llx %#llx.\n",value_msb,value_lsb);+#if IS_ENABLED(CONFIG_HWMON)+if(dev->hwmon){+print_sensor_names_in_bit_set(dev,dev->hwmon,value_lsb,0);+print_sensor_names_in_bit_set(dev,dev->hwmon,value_msb,+sizeof(value_lsb)*BITS_PER_BYTE);+}+#endif+}returnNOTIFY_OK;}
From: Shahar Shitrit <redacted>
Wrap the high temperature warning in a temperature event with
a call to net_ratelimit() to prevent flooding the kernel log
with repeated warning messages when temperature exceeds the
threshold multiple times within a short duration.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
@@ -165,9 +165,10 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)value_lsb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_lsb);value_msb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_msb);-mlx5_core_warn(events->dev,-"High temperature on sensors with bit set %llx %llx",-value_msb,value_lsb);+if(net_ratelimit())+mlx5_core_warn(events->dev,+"High temperature on sensors with bit set %llx %llx",+value_msb,value_lsb);returnNOTIFY_OK;}
From: Shahar Shitrit <redacted>
Prepend '0x' to the sensor bitmap in the warning message to clearly
indicate that the bitmap is in hexadecimal format.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -167,7 +167,7 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)if(net_ratelimit())mlx5_core_warn(events->dev,-"High temperature on sensors with bit set %llx %llx",+"High temperature on sensors with bit set %#llx %#llx",value_msb,value_lsb);returnNOTIFY_OK;
From: Shahar Shitrit <redacted>
In the sensor_count field of the MTEWE register, bits 1-62 are
supported only for unmanaged switches, not for NICs, and bit 63
is reserved for internal use.
To prevent confusing output that may include set bits that are
not relevant to NIC sensors, we update the bitmask to retain only
the first bit, which corresponds to the sensor ASIC.
Signed-off-by: Shahar Shitrit <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/events.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -163,6 +163,10 @@ static int temp_warn(struct notifier_block *nb, unsigned long type, void *data)u64value_msb;value_lsb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_lsb);+/* bit 1-63 are not supported for NICs,+*hencereadonlybit0(asic)fromlsb.+*/+value_lsb&=0x1;value_msb=be64_to_cpu(eqe->data.temp_warning.sensor_warning_msb);if(net_ratelimit())
From: Simon Horman <horms@kernel.org> Date: 2025-02-15 19:29:40
On Thu, Feb 13, 2025 at 11:46:41AM +0200, Tariq Toukan wrote:
From: Shahar Shitrit <redacted>
Previously, a temperature event message included a bitmap indicating
which sensors detect high temperatures.
To enhance clarity, we modify the message format to explicitly list
the names of the overheating sensors, alongside the sensors bitmap.
If HWMON is not configured, the event message remains unchanged.
Signed-off-by: Shahar Shitrit <redacted>
Reviewed-by: Carolina Jubran <redacted>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
...
+#if IS_ENABLED(CONFIG_HWMON)
+static void print_sensor_names_in_bit_set(struct mlx5_core_dev *dev, struct mlx5_hwmon *hwmon,
+ u64 bit_set, int bit_set_offset)
+{
+ unsigned long *bit_set_ptr = (unsigned long *)&bit_set;
+ int num_bits = sizeof(bit_set) * BITS_PER_BYTE;
+ int i;
+
+ for_each_set_bit(i, bit_set_ptr, num_bits) {
+ const char *sensor_name = hwmon_get_sensor_name(hwmon, i + bit_set_offset);
+
+ mlx5_core_warn(dev, "Sensor name[%d]: %s\n", i + bit_set_offset, sensor_name);
+ }
+}
nit:
If you have to respin for some other reason, please consider limiting lines
to 80 columns wide or less here and elsewhere in this patch where it
doesn't reduce readability (subjective I know).
e.g.:
static void print_sensor_names_in_bit_set(struct mlx5_core_dev *dev,
struct mlx5_hwmon *hwmon,
u64 bit_set, int bit_set_offset)
{
unsigned long *bit_set_ptr = (unsigned long *)&bit_set;
int num_bits = sizeof(bit_set) * BITS_PER_BYTE;
int i;
for_each_set_bit(i, bit_set_ptr, num_bits) {
const char *sensor_name;
sensor_name = hwmon_get_sensor_name(hwmon, i + bit_set_offset);
mlx5_core_warn(dev, "Sensor name[%d]: %s\n",
i + bit_set_offset, sensor_name);
}
}
...
nit:
If you have to respin for some other reason, please consider limiting lines
to 80 columns wide or less here and elsewhere in this patch where it
doesn't reduce readability (subjective I know).
+1, please try to catch such situations going forward
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski [off-list ref]:
On Thu, 13 Feb 2025 11:46:37 +0200 you wrote:
Hi,
This small series from Shahar adds the sensors names to the temperature
event messages, in addition to the existing bitmap indicators.
This improves human readability.
Series starts with simple refactoring and modifications. The top patch
adds the sensors names.
[...]
nit:
If you have to respin for some other reason, please consider limiting lines
to 80 columns wide or less here and elsewhere in this patch where it
doesn't reduce readability (subjective I know).
+1, please try to catch such situations going forward
Hi Jakub,
This was not missed.
This is not a new thing...
We've been enforcing a max line length of 100 chars in mlx5 driver for
the past few years.
I don't have the full image now, but I'm convinced that this dates back
to an agreement between the mlx5 and netdev maintainers at that time.
80 chars could be too restrictive, especially with today's large
monitors, while 100-chars is still highly readable.
This is subjective of course...
If you don't have a strong preference, we'll keep the current 100 chars
limit. Otherwise, just let me know and we'll start enforcing the
80-chars limit for future patches.
Regards,
Tariq
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-02-19 15:28:30
On Wed, 19 Feb 2025 15:00:57 +0200 Tariq Toukan wrote:
quoted
quoted
If you have to respin for some other reason, please consider limiting lines
to 80 columns wide or less here and elsewhere in this patch where it
doesn't reduce readability (subjective I know).
+1, please try to catch such situations going forward
This was not missed.
This is not a new thing...
We've been enforcing a max line length of 100 chars in mlx5 driver for
the past few years.
I don't have the full image now, but I'm convinced that this dates back
to an agreement between the mlx5 and netdev maintainers at that time.
80 chars could be too restrictive, especially with today's large
monitors, while 100-chars is still highly readable.
This is subjective of course...
If you don't have a strong preference, we'll keep the current 100 chars
limit. Otherwise, just let me know and we'll start enforcing the
80-chars limit for future patches.
Right, I think mlx5 is the only exception to the 80 column guidance.
I don't think it's resulting in more readable code, so yes, my
preference is to end this experiment.
On Wed, 19 Feb 2025 15:00:57 +0200 Tariq Toukan wrote:
quoted
quoted
quoted
If you have to respin for some other reason, please consider limiting lines
to 80 columns wide or less here and elsewhere in this patch where it
doesn't reduce readability (subjective I know).
+1, please try to catch such situations going forward
This was not missed.
This is not a new thing...
We've been enforcing a max line length of 100 chars in mlx5 driver for
the past few years.
I don't have the full image now, but I'm convinced that this dates back
to an agreement between the mlx5 and netdev maintainers at that time.
80 chars could be too restrictive, especially with today's large
monitors, while 100-chars is still highly readable.
This is subjective of course...
If you don't have a strong preference, we'll keep the current 100 chars
limit. Otherwise, just let me know and we'll start enforcing the
80-chars limit for future patches.
Right, I think mlx5 is the only exception to the 80 column guidance.
I don't think it's resulting in more readable code, so yes, my
preference is to end this experiment.
The reason in mlx5 was that we wanted to preserve the official HW spec
auto-generated fields names and they are really long.
100 chars worked very well with us for example the following sequence of
code setting up a FW command buffer would have to be broken in every line
if we were to restrict 80 chars per line.
MLX5_SET(modify_vhca_state_in, in, opcode, MLX5_CMD_OP_MODIFY_VHCA_STATE);
MLX5_SET(modify_vhca_state_in, in, vhca_state_field_select.sw_function_id, 1);
MLX5_SET(modify_vhca_state_in, in, vhca_state_context.sw_function_id, sw_fn_id);
MLX5_SET(modify_vhca_state_in, in, vhca_state_context.arm_change_event, 1);
MLX5_SET(modify_vhca_state_in, in, vhca_state_field_select.arm_change_event, 1);
But I believe the driver grow larger than caring about those lines too
much, I just did a quick check and it seems less than 2% of the lines are
actually > 80, not sure this is due to being more strict in the past few
years or that we don't really need more than 80 lines.
I also check the interesting cases with macros such
MLX5_SET/MLX5_GET/MLX5_CAP and also the percentile of long lines was very
minor just about 5% in all cases..
So I kinda agree mlx5 doesn't need be so special anymore.
Tariq up to you, you are the main reviewer now.
Thanks
Saeed.