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.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
