[2/4] scsi: scsi_dh_alua: create alua_rtpg_print() for alua_rtpg() sdev_printk

Commit Message

Mauricio Faria de Oliveira April 11, 2017, 1:17 a.m. UTC

Factor out the sdev_printk() statement with the RTPG information
in alua_rtpg() into a new function, alua_rtpg_print(), that will
also be used in the following patch.

The only functional difference is that the 'valid_states' value
is now referenced via a pointer, and can be NULL (optional), in
which case the 'valid_states' information is not printed.  That
is for the following patch too.

Signed-off-by: Mauricio Faria de Oliveira <[e-mail]>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 43 ++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 11 deletions(-)

Comments

Bart Van Assche April 13, 2017, 9:18 p.m. UTC | #1

On Mon, 2017-04-10 at 22:17 -0300, Mauricio Faria de Oliveira wrote:
>  /*
> + * alua_rtpg_print - Print REPORT TARGET GROUP STATES information
> + * @sdev: the device evaluated (or associated with this port group).
> + * @pg: the port group the information is associated with.
> + * @valid_states: pointer to valid_states value.
> + *                (optional; e.g., obtained via another port group)
> + *
> + * Must be called with pg->lock held.
> + */

Please use lockdep_assert_held() instead of documenting locking conventions as
comments. lockdep_assert_held() is verified at runtime with CONFIG_PROVE_LOCKING=y
but comments not.

> +static void alua_rtpg_print(struct scsi_device *sdev, struct alua_port_group *pg,
> +			    int *valid_states)
> +{
> +	if (valid_states)
> +		sdev_printk(KERN_INFO, sdev,
> +		    "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n",
> +		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
> +		    pg->pref ? "preferred" : "non-preferred",
> +		    (*valid_states) & TPGS_SUPPORT_TRANSITION ? 'T' : 't',
> +		    (*valid_states) & TPGS_SUPPORT_OFFLINE ? 'O' : 'o',
> +		    (*valid_states) & TPGS_SUPPORT_LBA_DEPENDENT ? 'L' : 'l',
> +		    (*valid_states) & TPGS_SUPPORT_UNAVAILABLE ? 'U' : 'u',
> +		    (*valid_states) & TPGS_SUPPORT_STANDBY ? 'S' : 's',
> +		    (*valid_states) & TPGS_SUPPORT_NONOPTIMIZED ? 'N' : 'n',
> +		    (*valid_states) & TPGS_SUPPORT_OPTIMIZED  ?  'A' : 'a');
> +	else
> +		sdev_printk(KERN_INFO, sdev,
> +		    "%s: port group %02x state %c %s\n",
> +		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
> +		    pg->pref ? "preferred" : "non-preferred");
> +}

Please define two functions - one for valid_states != NULL and one for valid_states
== NULL such that valid_states can be passed by value instead of by reference.

Thanks,

Bart.

Patch

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 5e5a33cac951..0d3508f2e285 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -523,6 +523,37 @@  static int alua_tur(struct scsi_device *sdev)
 }
 
 /*
+ * alua_rtpg_print - Print REPORT TARGET GROUP STATES information
+ * @sdev: the device evaluated (or associated with this port group).
+ * @pg: the port group the information is associated with.
+ * @valid_states: pointer to valid_states value.
+ *                (optional; e.g., obtained via another port group)
+ *
+ * Must be called with pg->lock held.
+ */
+static void alua_rtpg_print(struct scsi_device *sdev, struct alua_port_group *pg,
+			    int *valid_states)
+{
+	if (valid_states)
+		sdev_printk(KERN_INFO, sdev,
+		    "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n",
+		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
+		    pg->pref ? "preferred" : "non-preferred",
+		    (*valid_states) & TPGS_SUPPORT_TRANSITION ? 'T' : 't',
+		    (*valid_states) & TPGS_SUPPORT_OFFLINE ? 'O' : 'o',
+		    (*valid_states) & TPGS_SUPPORT_LBA_DEPENDENT ? 'L' : 'l',
+		    (*valid_states) & TPGS_SUPPORT_UNAVAILABLE ? 'U' : 'u',
+		    (*valid_states) & TPGS_SUPPORT_STANDBY ? 'S' : 's',
+		    (*valid_states) & TPGS_SUPPORT_NONOPTIMIZED ? 'N' : 'n',
+		    (*valid_states) & TPGS_SUPPORT_OPTIMIZED  ?  'A' : 'a');
+	else
+		sdev_printk(KERN_INFO, sdev,
+		    "%s: port group %02x state %c %s\n",
+		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
+		    pg->pref ? "preferred" : "non-preferred");
+}
+
+/*
  * alua_rtpg - Evaluate REPORT TARGET GROUP STATES
  * @sdev: the device to be evaluated.
  *
@@ -679,17 +710,7 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 	}
 
 	spin_lock_irqsave(&pg->lock, flags);
-	sdev_printk(KERN_INFO, sdev,
-		    "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n",
-		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
-		    pg->pref ? "preferred" : "non-preferred",
-		    valid_states&TPGS_SUPPORT_TRANSITION?'T':'t',
-		    valid_states&TPGS_SUPPORT_OFFLINE?'O':'o',
-		    valid_states&TPGS_SUPPORT_LBA_DEPENDENT?'L':'l',
-		    valid_states&TPGS_SUPPORT_UNAVAILABLE?'U':'u',
-		    valid_states&TPGS_SUPPORT_STANDBY?'S':'s',
-		    valid_states&TPGS_SUPPORT_NONOPTIMIZED?'N':'n',
-		    valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a');
+	alua_rtpg_print(sdev, pg, &valid_states);
 
 	switch (pg->state) {
 	case SCSI_ACCESS_STATE_TRANSITIONING: