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

[Hannes Reinecke <hare@suse.de>]

On 07/24/2015 04:38 PM, Christoph Hellwig wrote:
> 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.
> 
Yeah, possibly. After all, the variable isn't expected to change
under rcu_read_lock().

>> @@ -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.
> 
Okay.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html