Re: Re: [PATCH] target: For iblock at default writecache enable.

[majianpeng <majianpeng@gmail.com>]

>On Tue, 2013-01-29 at 15:04 -0800, Andy Grover wrote:
>> On 01/29/2013 11:03 AM, Nicholas A. Bellinger wrote:
>> 
>> > 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.
>> 
>
>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.
>
>> 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:
>	/*
>	 * 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:
>/*
>	 * 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!