Re: [dm-devel] [PATCH 2/6] scsi_dh : increment the refcounts while calling activate

[Shyam Iyer <shyam_iyer@dell.com>]

On 07/28/2010 06:59 PM, Moger, Babu wrote:
> Hold the refcounts for device and scsi_dh_data while calling handler's activate. This will make sure that devices and scsi_dh_data are not removed while activate is still in progress. Make sure to call put_device and kref_put in the handler after activate is complete.
>
> Signed-off-by: Babu Moger<babu.moger@lsi.com>
> ---
> --- linux-2.6.35-rc5/drivers/scsi/device_handler/scsi_dh.c.orig	2010-07-23 05:40:11.000000000 -0500
> +++ linux-2.6.35-rc5/drivers/scsi/device_handler/scsi_dh.c	2010-07-23 05:48:53.000000000 -0500
> @@ -228,7 +228,8 @@ store_dh_state(struct device *dev, struc
>   			 * Activate a device handler
>   			 */
>   			if (scsi_dh->activate)
> -				err = scsi_dh->activate(sdev, NULL, NULL);
> +				err = scsi_dh_activate(sdev->request_queue,
> +							NULL, NULL);
>   			else
>   				err = 0;
>   		}
> @@ -431,6 +432,8 @@ EXPORT_SYMBOL_GPL(scsi_unregister_device
>    *         do not hold the lock in the caller which may be needed in fn.
>    * @data - data passed to the function fn upon completion.
>    *
> + * NOTE  - Remember to call put_device and kref_put in the handler after
> + *         calling the callback function. Otherwise things could become messy.
>    */
>   int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
>   {
> @@ -450,9 +453,12 @@ int scsi_dh_activate(struct request_queu
>   	if (err)
>   		return err;
>
> -	if (scsi_dh->activate)
> +	if (scsi_dh->activate) {
> +		kref_get(&sdev->scsi_dh_data->kref);
>   		err = scsi_dh->activate(sdev, fn, data);
>    
Why not kref_put here instead of in the device handler.. It is easier to 
associate the ref counts..

Also, you are removing the put_device here and adding them to the device 
handler which can be avoided ..
> -	put_device(&sdev->sdev_gendev);
> +	} else
> +		put_device(&sdev->sdev_gendev);
> +
>   	return err;
>   }
>   EXPORT_SYMBOL_GPL(scsi_dh_activate);
>
>
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>    

-Shyam Iyer