target: For iblock at default writecache enable.
Subsystems (2)
PATCH 1/1 target: For iblock at default writecache enable.
Like the harddisk, at default write cache is enable.
--- drivers/target/target_core_iblock.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/target/target_core_iblock.c +3 -0
@@ -125,6 +125,9 @@ static int iblock_configure_device(struct se_device *dev) dev->dev_attrib.hw_max_sectors =UINT_MAX ; dev->dev_attrib.hw_queue_depth = q->nr_requests; +/*Default enable write cache*/ + dev->dev_attrib.emulate_write_cache =1 ; +/* * Check if the underlying struct block_device request_queue supports * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM
Signature · 1 line
1.7.9.5
Hi majianpeng,
Quoted 12 lines · majianpeng
Like the harddisk, at default write cache is enable.
Signed-off-by Jianpeng Ma <[email protected]>--- drivers/target/target_core_iblock.c | 3 +++ 1 file changed, 3 insertions(+)drivers/target/target_core_iblock.c +0
-0
drivers/target/target_core_iblock.c static int iblock_configure_device(struct se_device *dev) · line 130
Quoted 6 lines
drivers/target/target_core_iblock.c +3
-0@@ -125,6 +125,9 @@ static int iblock_configure_device(struct se_device *dev) dev->dev_attrib.hw_max_sectors =UINT_MAX ; dev->dev_attrib.hw_queue_depth = q->nr_requests; +/*Default enable write cache*/ + dev->dev_attrib.emulate_write_cache =1 ; +
So enabling emulate_write_cache=1 in the case when the underlying device has not enabled it is incorrect.
I'd like to enable this bit when we know the underlying device has WCE=1 set, but currently there is not a way to determine this (generically) from struct block_device.
So NACK for applying this until there is a method to determine what the hardware below is doing.
--nab
Quoted 5 lines
drivers/target/target_core_iblock.c +0
-0/* * Check if the underlying struct block_device request_queue supports * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM Signature · 1 line
1.7.9.5
Quoted 9 lines · Nicholas A. Bellinger
So enabling emulate_write_cache=1 in the case when the underlying device has not enabled it is incorrect.
I'd like to enable this bit when we know the underlying device has WCE=1 set, but currently there is not a way to determine this (generically) from struct
block_device.So NACK for applying this until there is a method to determine what the hardware below is doing.
This should be possible from userspace though. I'm planning on looking up underlying scsi device(s?) using libblkid, and then query the sense data using libsgutils when adding a block backstore in targetcli.
It's kind of a hassle, but isn't it a huge performance win if we can enable this?
Regards -- Andy
Quoted 9 lines · Andy Grover
Quoted 7 lines · Nicholas A. Bellinger
So enabling emulate_write_cache=1 in the case when the underlying device has not enabled it is incorrect.
I'd like to enable this bit when we know the underlying device has WCE=1 set, but currently there is not a way to determine this (generically) from struct
block_device.
Quoted 7 lines
Quoted 2 lines
So NACK for applying this until there is a method to determine what the hardware below is doing.
This should be possible from userspace though. I'm planning on looking up underlying scsi device(s?) using libblkid, and then query the sense data using libsgutils when adding a block backstore in targetcli.
Querying the mode pages from userspace would work for the SCSI backstore case, but certainly not for raw block devices.
I'd still like to see this exposed to the block layer somehow, so that the setting can be automatically determined by TCM once it's known if the underlying HW has enabled write caching.
Quoted 3 lines
It's kind of a hassle, but isn't it a huge performance win if we can enable this?
Most certainly, but the danger is reporting WCE=1 (by default in all cases) from TCM to the initiator when the underlying drives do not have caching enabled. Note that every spinning media device that I've ever seen disables WCE by default from the factory.
--nab
Quoted 25 lines · Andy Grover
Quoted 9 lines · Nicholas A. Bellinger
Quoted 8 lines
So enabling emulate_write_cache=1 in the case when the underlying device has not enabled it is incorrect.
I'd like to enable this bit when we know the underlying device has WCE=1 set, but currently there is not a way to determine this (generically) from struct
block_device.Quoted 7 lines
Quoted 2 lines
So NACK for applying this until there is a method to determine what the hardware below is doing.
This should be possible from userspace though. I'm planning on looking up underlying scsi device(s?) using libblkid, and then query the sense data using libsgutils when adding a block backstore in targetcli.
Querying the mode pages from userspace would work for the SCSI backstore case, but certainly not for raw block devices.
I'd still like to see this exposed to the block layer somehow, so that the setting can be automatically determined by TCM once it's known if the underlying HW has enabled write caching.
Quoted 8 lines
Quoted 3 lines
It's kind of a hassle, but isn't it a huge performance win if we can enable this?
Most certainly, but the danger is reporting WCE=1 (by default in all cases) from TCM to the initiator when the underlying drives do not have caching enabled. Note that every spinning media device that I've ever seen disables WCE by default from the factory.
Sorry, what's the danger?Can you explain the details?
And for most sata hdd the WCE is enable by default.
IMHO, for hard-raid the WCE will be disable, it used the cache of hardraid-card.
In func sd_revalidate_disk:
Quoted 11 lines
/*
* We now have all cache related info, determine how we deal
* with flush requests.
*/
if (sdkp->WCE) {
flush |= REQ_FLUSH;
if (sdkp->DPOFUA)
flush |= REQ_FUA;
}
blk_queue_flush(sdkp->disk->queue, flush);We can use queue->flush_flags.
But in func generic_make_request_checks:
Quoted 7 lines
/*
* Filter flush bio's early so that make_request based * drivers without flush support don't have to worry * about them. */ if ((bio->bi_rw & (REQ_FLUSH | REQ_FUA)) && !q->flush_flags) { bio->bi_rw &= ~(REQ_FLUSH | REQ_FUA);
So if uderlying device don't support WCE, it can remove REQ_FUA|REQ_FLUSH. I think enable writecache by default is ok.
Jianpeng
Thanks!
Hi majianpeng,
Quoted 29 lines · majianpeng
Quoted 29 lines · Andy Grover
Quoted 9 lines · Nicholas A. Bellinger
Quoted 8 lines
So enabling emulate_write_cache=1 in the case when the underlying device has not enabled it is incorrect.
I'd like to enable this bit when we know the underlying device has WCE=1 set, but currently there is not a way to determine this (generically) from struct
block_device.Quoted 7 lines
Quoted 2 lines
So NACK for applying this until there is a method to determine what the hardware below is doing.
This should be possible from userspace though. I'm planning on looking up underlying scsi device(s?) using libblkid, and then query the sense data using libsgutils when adding a block backstore in targetcli.
Querying the mode pages from userspace would work for the SCSI backstore case, but certainly not for raw block devices.
I'd still like to see this exposed to the block layer somehow, so that the setting can be automatically determined by TCM once it's known if the underlying HW has enabled write caching.
Quoted 3 lines
It's kind of a hassle, but isn't it a huge performance win if we can enable this?
Quoted 6 lines
Quoted 4 lines
Most certainly, but the danger is reporting WCE=1 (by default in all cases) from TCM to the initiator when the underlying drives do not have caching enabled. Note that every spinning media device that I've ever seen disables WCE by default from the factory.
Sorry, what's the danger?Can you explain the details?
Current IBLOCK code will always use WRITE_FUA with emulate_write_cache=0 default setting. It attempts to be safe and invoke write through schematics with WRITE_FUA when emulate_write_cache=0 is present.
Quoted 1 line
And for most sata hdd the WCE is enable by default.
Spinning media SAS drives from the factory still this disable per default to avoid silent data loss in the event of a hard power failure.
Quoted 13 lines
IMHO, for hard-raid the WCE will be disable, it used the cache of hardraid-card.
In func
sd_revalidate_disk:Quoted 9 lines
/* * We now have all cache related info, determine how we deal * with flush requests. */ if (sdkp->WCE) { flush |= REQ_FLUSH; if (sdkp->DPOFUA) flush |= REQ_FUA; }
Quoted 2 lines
Quoted 1 line
blk_queue_flush(sdkp->disk->queue, flush);We can use
queue->flush_flags.
Good point.
These are request queue hints that IBLOCK could be taking advantage of here.
Quoted 11 lines
But in func
generic_make_request_checks:Quoted 7 lines
/*
* Filter flush bio's early so that make_request based * drivers without flush support don't have to worry * about them. */ if ((bio->bi_rw & (REQ_FLUSH | REQ_FUA)) && !q->flush_flags) { bio->bi_rw &= ~(REQ_FLUSH | REQ_FUA);So if uderlying device don't support WCE, it can remove REQ_FUA|REQ_FLUSH. I think enable writecache by default is ok.
Blindly enabling WriteCacheEnabled=1 in all cases for IBLOCK backends to the SCSI initiator port, and ignoring what the underlying hardware is doing is not correct either.
So, I've sent out a patch proposing a method for IBLOCK to take advantage of REQ_FLUSH for WCE=1. I'm not crazy about the extra pointer dereferences in the iblock_execute_rw() data I/O path, but it's likely the most sane way to access to request_queue->flush_flags minus adding a target callback to update emulate_write_cache in the event of a blk_queue_flush() call, considering this value may change from below target/iblock.
(hch, axboe + jejb CC'ed)
--nab