scsi: sd: add new match array for cache_type
PATCH v3 1/1 scsi: sd: add new match array for cache_type
add user friendly command strings sd_wr_cache to enable/disable write&read cache. user can enable both write and read cache by one of the following commands:
echo w1r1 > cache_type
echo "write back" > cache_type
sd_wr_cache[] = {"w0r1", "w0r0", "w1r1", "w1r0"};
for sd_wr_cache 0 means disable, 1 means enable.
Encoding | WCE RCD | Write_cache Read_cache
--------------------------------------------------------------------
write through / w0r1 | 0 0 | off on none / w0r0 | 0 1 | off off write back / w1r1 | 1 0 | on on write back, no read (daft) / w1r0 | 1 1 | on off
--- drivers/scsi/sd.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
drivers/scsi/sd.c +19 -2
@@ -134,10 +134,20 @@ static DEFINE_MUTEX(sd_ref_mutex);static struct kmem_cache *sd_cdb_cache;static mempool_t *sd_cdb_pool; +/* +* Encoding | WCE RCD | Write_cache Read_cache +* -------------------------------------------------------------------- +* write through / w0r1 | 0 0 | off on +* none / w0r0 | 0 1 | off off +* write back / w1r1 | 1 0 | on on +* write back, no read (daft) / w1r0 | 1 1 | on off +*/ static const char *sd_cache_types[] = {"write through" ,"none" ,"write back" ,"write back, no read (daft)" }; +/* 0:disable, 1:enable */ +static const char *const sd_wr_cache[] = {"w0r1" ,"w0r0" ,"w1r1" ,"w1r0" };static void sd_set_flush_flag (struct scsi_disk *sdkp) {
@@ -172,6 +182,10 @@ cache_type_store(struct device *dev, struct device_attribute *attr,* it's not worth the risk */ return -EINVAL ; +/* +* for "temporary", we only change request_queue's flag, not send +* any command to disk, so actually disk'cache dosen't changed yet. +*/ if (strncmp (buf, temp,sizeof (temp) -1 ) ==0 ) { buf +=sizeof (temp) -1 ; sdkp->cache_override =1 ;
@@ -180,8 +194,11 @@ cache_type_store(struct device *dev, struct device_attribute *attr, } ct =sysfs_match_string (sd_cache_types, buf); -if (ct <0 ) -return -EINVAL ; +if (ct <0 ) { + ct =sysfs_match_string (sd_wr_cache, buf); +if (ct <0 ) +return -EINVAL ; + } rcd = ct &0x01 ?1 :0 ; wce = (ct &0x02 ) && !sdkp->write_prot ?1 :0 ;
Signature · 1 line
2.9.4
Weiping,
Quoted 3 lines
add user friendly command strings
sd_wr_cacheto enable/disable write&read cache. user can enable both write and read cache by one of the following commands:
I remain unconvinced that introducing redundant option strings is a benefit.
These shorthand forms may seem obvious to you, but not to everyone else. Adding to the murkiness is that fact that write cache must be *enabled* and read cache *disabled* in the protocol. The fact that the ??rN and RCD columns below contain inverse values just emphasizes that point that "0" and "1" are not a particularly good interface either.
It is also confusing that the existing longhand forms will be printed when the user subsequently queries the state instead of what they used to toggle it.
This, however, is pretty much what I was looking for:
Quoted 6 lines
Encoding | WCE RCD |
Write_cacheRead_cache
--------------------------------------------------------------------write through / w0r1 | 0 0 | off on none / w0r0 | 0 1 | off off write back / w1r1 | 1 0 | on on write back, no read (daft) / w1r0 | 1 1 | on off
Please drop the w?r? options and submit a patch with this table and a note about the "temporary" prefix. Put it in Documentation/scsi/sd-parameters.txt or something to that effect.
Bonus points for also documenting the remaining sd sysfs attributes and their options.
Thanks!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
2018-01-31 11:22 GMT+08:00 Martin K. Petersen <[email protected]>:
Quoted 16 lines
Weiping,
Quoted 3 lines
add user friendly command strings
sd_wr_cacheto enable/disable write&read cache. user can enable both write and read cache by one of the following commands:I remain unconvinced that introducing redundant option strings is a benefit.
These shorthand forms may seem obvious to you, but not to everyone else. Adding to the murkiness is that fact that write cache must be *enabled* and read cache *disabled* in the protocol. The fact that the ??rN and RCD columns below contain inverse values just emphasizes that point that "0" and "1" are not a particularly good interface either.
Quoted 4 lines
It is also confusing that the existing longhand forms will be printed when the user subsequently queries the state instead of what they used to toggle it.
Yes, indeed.
Quoted 10 lines
This, however, is pretty much what I was looking for:
Quoted 6 lines
Encoding | WCE RCD |
Write_cacheRead_cache
--------------------------------------------------------------------write through / w0r1 | 0 0 | off on none / w0r0 | 0 1 | off off write back / w1r1 | 1 0 | on on write back, no read (daft) / w1r0 | 1 1 | on off
Quoted 4 lines
Please drop the w?r? options and submit a patch with this table and a note about the "temporary" prefix. Put it in
Documentation/scsi/sd-parameters.txtor something to that effect.
It's ok for me.
Quoted 2 lines
Bonus points for also documenting the remaining sd sysfs attributes and their options.
I'll try.
Thanks
Quoted 5 lines
Thanks!
--
Martin K. Petersen Oracle Linux Engineering