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

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

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.