Re: [PATCH v9 net-next 04/14] pds_core: add devlink health facilities

[Leon Romanovsky <leon@kernel.org>]

On Thu, Apr 06, 2023 at 04:41:33PM -0700, Shannon Nelson wrote:
> Add devlink health reporting on top of our fw watchdog.
> 
> Example:
>   # devlink health show pci/0000:2b:00.0 reporter fw
>   pci/0000:2b:00.0:
>     reporter fw
>       state healthy error 0 recover 0
>   # devlink health diagnose pci/0000:2b:00.0 reporter fw
>    Status: healthy State: 1 Generation: 0 Recoveries: 0
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
>  .../device_drivers/ethernet/amd/pds_core.rst  | 12 ++++++
>  drivers/net/ethernet/amd/pds_core/Makefile    |  1 +
>  drivers/net/ethernet/amd/pds_core/core.c      |  6 +++
>  drivers/net/ethernet/amd/pds_core/core.h      |  6 +++
>  drivers/net/ethernet/amd/pds_core/devlink.c   | 37 +++++++++++++++++++
>  drivers/net/ethernet/amd/pds_core/main.c      | 22 +++++++++++
>  6 files changed, 84 insertions(+)
>  create mode 100644 drivers/net/ethernet/amd/pds_core/devlink.c

<...>

> +int pdsc_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
> +			      struct devlink_fmsg *fmsg,
> +			      struct netlink_ext_ack *extack)
> +{
> +	struct pdsc *pdsc = devlink_health_reporter_priv(reporter);
> +	int err = 0;
> +
> +	if (test_bit(PDSC_S_FW_DEAD, &pdsc->state))

How is this check protected from race with your health workqueue added
in previous patch? 

> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "dead");
> +	else if (!pdsc_is_fw_good(pdsc))

Same question.

> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "unhealthy");
> +	else
> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");
> +	if (err)
> +		return err;

Thanks