diff mbox

[RFC,09/10] scsi: sd: Avoid sending medium write commands if device is write protected

Message ID 1407760838-6406-10-git-send-email-draviv@codeaurora.org (mailing list archive)
State Deferred
Headers show

Commit Message

Dolev Raviv Aug. 11, 2014, 12:40 p.m. UTC
From: Sujit Reddy Thumma <sthumma@codeaurora.org>

The SYNCHRONIZE_CACHE command is a medium write command and hence can
fail when the device is write protected. Avoid sending such commands by
making sure that write-cache-enable is disabled even though the device
claim to support it.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Dolev Raviv <draviv@codeaurora.org>

Comments

Christoph Hellwig Aug. 19, 2014, 5:30 p.m. UTC | #1
This looks reasonable to me.  Martin, and objections?

On Mon, Aug 11, 2014 at 03:40:37PM +0300, Dolev Raviv wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 
> The SYNCHRONIZE_CACHE command is a medium write command and hence can
> fail when the device is write protected. Avoid sending such commands by
> making sure that write-cache-enable is disabled even though the device
> claim to support it.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 3663e38..67282bf 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -185,7 +185,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr,
>  	if (ct < 0)
>  		return -EINVAL;
>  	rcd = ct & 0x01 ? 1 : 0;
> -	wce = ct & 0x02 ? 1 : 0;
> +	wce = (ct & 0x02) && !sdkp->write_prot ? 1 : 0;
>  
>  	if (sdkp->cache_override) {
>  		sdkp->WCE = wce;
> @@ -2493,6 +2493,10 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
>  			sdkp->DPOFUA = 0;
>  		}
>  
> +		/* No cache flush allowed for write protected devices */
> +		if (sdkp->WCE && sdkp->write_prot)
> +			sdkp->WCE = 0;
> +
>  		if (sdkp->first_scan || old_wce != sdkp->WCE ||
>  		    old_rcd != sdkp->RCD || old_dpofua != sdkp->DPOFUA)
>  			sd_printk(KERN_NOTICE, sdkp,
> -- 
> 1.8.5.2
> -- 
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> --
> 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
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Venkatesh Srinivas Aug. 19, 2014, 5:55 p.m. UTC | #2
On Mon, Aug 11, 2014 at 5:40 AM, Dolev Raviv <draviv@codeaurora.org> wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
>
> The SYNCHRONIZE_CACHE command is a medium write command and hence can
> fail when the device is write protected. Avoid sending such commands by
> making sure that write-cache-enable is disabled even though the device
> claim to support it.
>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>

Looks good. Setting SWP is defined to flush caches; there is no other
reason I could imagine WP media would need a SYNCHRONIZE CACHE.

Reviewed-by: Venkatesh Srinivas <venkateshs@google.com>

-- vs;
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin K. Petersen Aug. 20, 2014, 1:24 p.m. UTC | #3
>>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes:

Christoph> This looks reasonable to me.

I agree.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Christoph Hellwig Aug. 24, 2014, 4:28 p.m. UTC | #4
Thanks,

applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3663e38..67282bf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -185,7 +185,7 @@  cache_type_store(struct device *dev, struct device_attribute *attr,
 	if (ct < 0)
 		return -EINVAL;
 	rcd = ct & 0x01 ? 1 : 0;
-	wce = ct & 0x02 ? 1 : 0;
+	wce = (ct & 0x02) && !sdkp->write_prot ? 1 : 0;
 
 	if (sdkp->cache_override) {
 		sdkp->WCE = wce;
@@ -2493,6 +2493,10 @@  sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
 			sdkp->DPOFUA = 0;
 		}
 
+		/* No cache flush allowed for write protected devices */
+		if (sdkp->WCE && sdkp->write_prot)
+			sdkp->WCE = 0;
+
 		if (sdkp->first_scan || old_wce != sdkp->WCE ||
 		    old_rcd != sdkp->RCD || old_dpofua != sdkp->DPOFUA)
 			sd_printk(KERN_NOTICE, sdkp,