Re: [PATCH 1/5] scsi: rescan VPD attributes

[Christoph Hellwig <hch@lst.de>]

On Wed, Jul 08, 2015 at 01:09:44PM +0200, Hannes Reinecke wrote:
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -264,8 +264,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>  	int device_id_size, device_id_type = 0;
>  	struct alua_port_group *tmp_pg, *pg = NULL;
>  
> -	if (!sdev->vpd_pg83)
> +	rcu_read_lock();
> +	if (!rcu_dereference(sdev->vpd_pg83)) {
> +		rcu_read_unlock();
>  		return SCSI_DH_DEV_UNSUPP;
> +	}
>  
>  	/*
>  	 * Look for the correct descriptor.
> @@ -281,8 +284,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>  	 */
>  	memset(device_id_str, 0, 256);
>  	device_id_size = 0;
> -	d = sdev->vpd_pg83 + 4;
> -	while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
> +	d = rcu_dereference(sdev->vpd_pg83) + 4;
> +	while (d < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {

Seem like this code would benefit from a local variable in favor of the
repeated rcu_dereference() calls.

> @@ -803,7 +803,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)

I think this function could use a new name, e.g. scsi_read_vpd_pages?

> @@ -563,8 +563,9 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
>  	if (!sdev->vpd_pg83_len)
>  		return;
>  
> -	desc = sdev->vpd_pg83 + 4;
> -	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
> +	rcu_read_lock();
> +	desc = rcu_dereference(sdev->vpd_pg83) + 4;
> +	while (desc < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {

A local variable or two would help here as well.