diff mbox

[4/4] scsi: scsi_dh_alua: do not print target port group state if it remains unavailable

Message ID 1491873481-23900-5-git-send-email-mauricfo@linux.vnet.ibm.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Mauricio Faria de Oliveira April 11, 2017, 1:18 a.m. UTC
Path checkers will periodically check all paths to a target port group
in unavailable state more often (as they are 'failed'), possibly for a
long or indefinite period of time, or for a large number of paths.

That might end up flooding the kernel log with the scsi_dh_alua target
port group state messages, which are triggered by the rtpg recheck
scheduled in alua_check_sense().

So, modify alua_rtpg() not to print such message when that unavailable
state remains (i.e., the target port group state did not transition to
or from the unavailable state). All other cases continue to be printed.

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Comments

Bart Van Assche April 13, 2017, 9:40 p.m. UTC | #1
On Mon, 2017-04-10 at 22:18 -0300, Mauricio Faria de Oliveira wrote:
>  /*
> + * alua_state_remains - Whether a RTPG state remains the same across 2 values.
> + * @state: the state value to check for.
> + * @old_state: the old state value.
> + * @new_state: the new state value.
> + */
> +static bool alua_state_remains(int state, int old_state, int new_state)
> +{
> +	return ((old_state == state) && (new_state == state));
> +}

Hello Mauricio,

All parentheses in the return statement are superfluous. Please consider
removing these.

> +/*
> -	alua_rtpg_print(sdev, pg, &valid_states);
> +
> +	/* Print RTPG information (except if state remains 'unavailable'). */
> +	if (likely(!alua_state_remains(SCSI_ACCESS_STATE_UNAVAILABLE,
> +					orig_state, pg->state)))
> +		alua_rtpg_print(sdev, pg, &valid_states);

Using "likely()" may prevent the CPU branch predictor to do it's work so in
kernel code usually likely() is only used in code that is in the hot path.

Thanks,

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index c2c9173fd883..8025e024c011 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -554,6 +554,17 @@  static void alua_rtpg_print(struct scsi_device *sdev, struct alua_port_group *pg
 }
 
 /*
+ * alua_state_remains - Whether a RTPG state remains the same across 2 values.
+ * @state: the state value to check for.
+ * @old_state: the old state value.
+ * @new_state: the new state value.
+ */
+static bool alua_state_remains(int state, int old_state, int new_state)
+{
+	return ((old_state == state) && (new_state == state));
+}
+
+/*
  * alua_rtpg - Evaluate REPORT TARGET GROUP STATES
  * @sdev: the device to be evaluated.
  *
@@ -571,6 +582,7 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 	unsigned int tpg_desc_tbl_off;
 	unsigned char orig_transition_tmo;
 	unsigned long flags;
+	int orig_state;
 
 	if (!pg->expiry) {
 		unsigned long transition_tmo = ALUA_FAILOVER_TIMEOUT * HZ;
@@ -656,6 +668,8 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		goto retry;
 	}
 
+	orig_state = pg->state;
+
 	orig_transition_tmo = pg->transition_tmo;
 	if ((buff[4] & RTPG_FMT_MASK) == RTPG_FMT_EXT_HDR && buff[5] != 0)
 		pg->transition_tmo = buff[5];
@@ -689,6 +703,7 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 				    !(tmp_pg->flags & ALUA_PG_RUNNING)) {
 					struct alua_dh_data *h;
 					struct scsi_device *tmp_pg_sdev = NULL;
+					int tmp_pg_orig_state = tmp_pg->state;
 
 					tmp_pg->state = desc[0] & 0x0f;
 					tmp_pg->pref = desc[0] >> 7;
@@ -704,8 +719,11 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 						 * is not running, tmp_pg->rtpg_sdev might be NULL.
 						 * Use another/one associated scsi_device to print
 						 * its RTPG information.
+						 * Don't print it if state remains 'unavailable'.
 						 */
-						if ((tmp_pg != pg) && !tmp_pg_sdev) {
+						if ((tmp_pg != pg) && !tmp_pg_sdev &&
+						    !alua_state_remains(SCSI_ACCESS_STATE_UNAVAILABLE,
+									tmp_pg_orig_state, tmp_pg->state)) {
 							tmp_pg_sdev = h->sdev;
 							alua_rtpg_print(tmp_pg_sdev, tmp_pg, NULL);
 						}
@@ -722,7 +740,11 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 	}
 
 	spin_lock_irqsave(&pg->lock, flags);
-	alua_rtpg_print(sdev, pg, &valid_states);
+
+	/* Print RTPG information (except if state remains 'unavailable'). */
+	if (likely(!alua_state_remains(SCSI_ACCESS_STATE_UNAVAILABLE,
+					orig_state, pg->state)))
+		alua_rtpg_print(sdev, pg, &valid_states);
 
 	switch (pg->state) {
 	case SCSI_ACCESS_STATE_TRANSITIONING: