* [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters
@ 2022-03-01 17:55 James Smart
2022-03-01 19:26 ` Himanshu Madhani
2022-03-09 4:14 ` Martin K. Petersen
0 siblings, 2 replies; 3+ messages in thread
From: James Smart @ 2022-03-01 17:55 UTC (permalink / raw)
To: linux-scsi; +Cc: James Smart, stable, Shyam Sundar, Nilesh Javali
In the original FPIN commit, stats were incremented by the event_count.
Event_count is the minimum # of events that must occur before an FPIN is
sent. Thus, its not the actual number of events, and could be
significantly off (too low) as it doesn't reflect anything not reported.
Rather than attempt to count events, have the statistic count how many
FPINS cross the threshold and were reported.
Fixes: 3dcfe0de5a97 ("scsi: fc: Parse FPIN packets and update statistics")
Cc: <stable@vger.kernel.org> # v5.11+
Signed-off-by: James Smart <jsmart2021@gmail.com>
Cc: Shyam Sundar <ssundar@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
---
This issue was originally reported in this thread, with no comments.
https://lore.kernel.org/linux-scsi/b472606d-e67c-66f1-06d1-ecc5fbb2071a@broadcom.com/
---
drivers/scsi/scsi_transport_fc.c | 39 +++++++++++++-------------------
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 60e406bcf42a..a2524106206d 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -34,7 +34,7 @@ static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
static void fc_bsg_remove(struct request_queue *);
static void fc_bsg_goose_queue(struct fc_rport *);
-static void fc_li_stats_update(struct fc_fn_li_desc *li_desc,
+static void fc_li_stats_update(u16 event_type,
struct fc_fpin_stats *stats);
static void fc_delivery_stats_update(u32 reason_code,
struct fc_fpin_stats *stats);
@@ -670,42 +670,34 @@ fc_find_rport_by_wwpn(struct Scsi_Host *shost, u64 wwpn)
EXPORT_SYMBOL(fc_find_rport_by_wwpn);
static void
-fc_li_stats_update(struct fc_fn_li_desc *li_desc,
+fc_li_stats_update(u16 event_type,
struct fc_fpin_stats *stats)
{
- stats->li += be32_to_cpu(li_desc->event_count);
- switch (be16_to_cpu(li_desc->event_type)) {
+ stats->li++;
+ switch (event_type) {
case FPIN_LI_UNKNOWN:
- stats->li_failure_unknown +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_failure_unknown++;
break;
case FPIN_LI_LINK_FAILURE:
- stats->li_link_failure_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_link_failure_count++;
break;
case FPIN_LI_LOSS_OF_SYNC:
- stats->li_loss_of_sync_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_loss_of_sync_count++;
break;
case FPIN_LI_LOSS_OF_SIG:
- stats->li_loss_of_signals_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_loss_of_signals_count++;
break;
case FPIN_LI_PRIM_SEQ_ERR:
- stats->li_prim_seq_err_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_prim_seq_err_count++;
break;
case FPIN_LI_INVALID_TX_WD:
- stats->li_invalid_tx_word_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_invalid_tx_word_count++;
break;
case FPIN_LI_INVALID_CRC:
- stats->li_invalid_crc_count +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_invalid_crc_count++;
break;
case FPIN_LI_DEVICE_SPEC:
- stats->li_device_specific +=
- be32_to_cpu(li_desc->event_count);
+ stats->li_device_specific++;
break;
}
}
@@ -767,6 +759,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
+ u16 event_type = be16_to_cpu(li_desc->event_type);
u64 wwpn;
rport = fc_find_rport_by_wwpn(shost,
@@ -775,7 +768,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
(rport->roles & FC_PORT_ROLE_FCP_TARGET ||
rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
attach_rport = rport;
- fc_li_stats_update(li_desc, &attach_rport->fpin_stats);
+ fc_li_stats_update(event_type, &attach_rport->fpin_stats);
}
if (be32_to_cpu(li_desc->pname_count) > 0) {
@@ -789,14 +782,14 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
if (rport == attach_rport)
continue;
- fc_li_stats_update(li_desc,
+ fc_li_stats_update(event_type,
&rport->fpin_stats);
}
}
}
if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
- fc_li_stats_update(li_desc, &fc_host->fpin_stats);
+ fc_li_stats_update(event_type, &fc_host->fpin_stats);
}
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters
2022-03-01 17:55 [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters James Smart
@ 2022-03-01 19:26 ` Himanshu Madhani
2022-03-09 4:14 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Himanshu Madhani @ 2022-03-01 19:26 UTC (permalink / raw)
To: James Smart; +Cc: linux-scsi, stable, Shyam Sundar, Nilesh Javali
> On Mar 1, 2022, at 9:55 AM, James Smart <jsmart2021@gmail.com> wrote:
>
> In the original FPIN commit, stats were incremented by the event_count.
> Event_count is the minimum # of events that must occur before an FPIN is
> sent. Thus, its not the actual number of events, and could be
> significantly off (too low) as it doesn't reflect anything not reported.
> Rather than attempt to count events, have the statistic count how many
> FPINS cross the threshold and were reported.
>
> Fixes: 3dcfe0de5a97 ("scsi: fc: Parse FPIN packets and update statistics")
> Cc: <stable@vger.kernel.org> # v5.11+
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> Cc: Shyam Sundar <ssundar@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
>
> ---
> This issue was originally reported in this thread, with no comments.
> https://lore.kernel.org/linux-scsi/b472606d-e67c-66f1-06d1-ecc5fbb2071a@broadcom.com/
> ---
> drivers/scsi/scsi_transport_fc.c | 39 +++++++++++++-------------------
> 1 file changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 60e406bcf42a..a2524106206d 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -34,7 +34,7 @@ static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
> static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
> static void fc_bsg_remove(struct request_queue *);
> static void fc_bsg_goose_queue(struct fc_rport *);
> -static void fc_li_stats_update(struct fc_fn_li_desc *li_desc,
> +static void fc_li_stats_update(u16 event_type,
> struct fc_fpin_stats *stats);
> static void fc_delivery_stats_update(u32 reason_code,
> struct fc_fpin_stats *stats);
> @@ -670,42 +670,34 @@ fc_find_rport_by_wwpn(struct Scsi_Host *shost, u64 wwpn)
> EXPORT_SYMBOL(fc_find_rport_by_wwpn);
>
> static void
> -fc_li_stats_update(struct fc_fn_li_desc *li_desc,
> +fc_li_stats_update(u16 event_type,
> struct fc_fpin_stats *stats)
> {
> - stats->li += be32_to_cpu(li_desc->event_count);
> - switch (be16_to_cpu(li_desc->event_type)) {
> + stats->li++;
> + switch (event_type) {
> case FPIN_LI_UNKNOWN:
> - stats->li_failure_unknown +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_failure_unknown++;
> break;
> case FPIN_LI_LINK_FAILURE:
> - stats->li_link_failure_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_link_failure_count++;
> break;
> case FPIN_LI_LOSS_OF_SYNC:
> - stats->li_loss_of_sync_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_loss_of_sync_count++;
> break;
> case FPIN_LI_LOSS_OF_SIG:
> - stats->li_loss_of_signals_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_loss_of_signals_count++;
> break;
> case FPIN_LI_PRIM_SEQ_ERR:
> - stats->li_prim_seq_err_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_prim_seq_err_count++;
> break;
> case FPIN_LI_INVALID_TX_WD:
> - stats->li_invalid_tx_word_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_invalid_tx_word_count++;
> break;
> case FPIN_LI_INVALID_CRC:
> - stats->li_invalid_crc_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_invalid_crc_count++;
> break;
> case FPIN_LI_DEVICE_SPEC:
> - stats->li_device_specific +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_device_specific++;
> break;
> }
> }
> @@ -767,6 +759,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> struct fc_rport *attach_rport = NULL;
> struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
> struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
> + u16 event_type = be16_to_cpu(li_desc->event_type);
> u64 wwpn;
>
> rport = fc_find_rport_by_wwpn(shost,
> @@ -775,7 +768,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> (rport->roles & FC_PORT_ROLE_FCP_TARGET ||
> rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
> attach_rport = rport;
> - fc_li_stats_update(li_desc, &attach_rport->fpin_stats);
> + fc_li_stats_update(event_type, &attach_rport->fpin_stats);
> }
>
> if (be32_to_cpu(li_desc->pname_count) > 0) {
> @@ -789,14 +782,14 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
> if (rport == attach_rport)
> continue;
> - fc_li_stats_update(li_desc,
> + fc_li_stats_update(event_type,
> &rport->fpin_stats);
> }
> }
> }
>
> if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
> - fc_li_stats_update(li_desc, &fc_host->fpin_stats);
> + fc_li_stats_update(event_type, &fc_host->fpin_stats);
> }
>
> /*
> --
> 2.26.2
>
Looks Good.
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
--
Himanshu Madhani Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters
2022-03-01 17:55 [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters James Smart
2022-03-01 19:26 ` Himanshu Madhani
@ 2022-03-09 4:14 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2022-03-09 4:14 UTC (permalink / raw)
To: James Smart, linux-scsi
Cc: Martin K . Petersen, stable, Shyam Sundar, Nilesh Javali
On Tue, 1 Mar 2022 09:55:36 -0800, James Smart wrote:
> In the original FPIN commit, stats were incremented by the event_count.
> Event_count is the minimum # of events that must occur before an FPIN is
> sent. Thus, its not the actual number of events, and could be
> significantly off (too low) as it doesn't reflect anything not reported.
> Rather than attempt to count events, have the statistic count how many
> FPINS cross the threshold and were reported.
>
> [...]
Applied to 5.18/scsi-queue, thanks!
[1/1] scsi_transport_fc: Fix FPIN Link Integrity statistics counters
https://git.kernel.org/mkp/scsi/c/07e0984b96ec
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-09 4:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 17:55 [PATCH][REPOST] scsi_transport_fc: Fix FPIN Link Integrity statistics counters James Smart
2022-03-01 19:26 ` Himanshu Madhani
2022-03-09 4:14 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).