diff mbox

megaraid_sas: Re-enable WRITE SAME

Message ID 1519133994-1393-1-git-send-email-shivasharan.srikanteshwara@broadcom.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Shivasharan Srikanteshwara Feb. 20, 2018, 1:39 p.m. UTC
Below commit disable WRITE SAME behavior via host template
no_write_same settings for megaraid_sas driver.

commit 54b2b50c20a6
("[SCSI] Disable WRITE SAME for RAID and virtual host adapter drivers")

For MegaRAID controller we want to support WRITE SAME for non-raid volumes.
Because of host wide "no_write_same = 1" settings, we are not able to use
provision mode. Virtual Disk will not enable WRITE SAME feature providing
correct VPD block limit page. Virtual Disk in MegaRAID provides below
values in VPD page 0xb0 which will disable WRITE SAME commands.

  Maximum unmap LBA count: 0
  Maximum unmap block descriptor count: 0
  Optimal unmap granularity: 0
  Unmap granularity alignment valid: 0
  Unmap granularity alignment: 0
  Maximum write same length: 0x0 blocks

Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Martin K. Petersen Feb. 22, 2018, 4:22 a.m. UTC | #1
Hi Shivasharan!

> For MegaRAID controller we want to support WRITE SAME for non-raid
> volumes.  Because of host wide "no_write_same = 1" settings, we are
> not able to use provision mode.

The no_write_same flag gates whether the WRITE SAME(10/16) commands are
used to zero block ranges. It does not influence whether WRITE SAME with
the UNMAP bit set is used to deallocate block ranges on devices which
support logical block provisioning.

> Virtual Disk will not enable WRITE SAME feature providing correct VPD
> block limit page.

This wasn't always the case.

If you want to enable WRITE SAME you'll have to provide some sort of
heuristic to avoid breaking things on firmware versions and controllers
that predate correct reporting.

> Virtual Disk in MegaRAID provides below values in VPD page 0xb0 which
> will disable WRITE SAME commands.

A value of 0 in

  Maximum write same length: 0x0 blocks

means the maximum length is not reported. It does not mean that WRITE
SAME isn't supported.
Shivasharan Srikanteshwara Feb. 27, 2018, 4:58 a.m. UTC | #2
> -----Original Message-----
> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
> Sent: Thursday, February 22, 2018 9:52 AM
> To: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
> Cc: linux-scsi@vger.kernel.org; kashyap.desai@broadcom.com;
> sumit.saxena@broadcom.com
> Subject: Re: [PATCH] megaraid_sas: Re-enable WRITE SAME
>
>
> Hi Shivasharan!
>
> > For MegaRAID controller we want to support WRITE SAME for non-raid
> > volumes.  Because of host wide "no_write_same = 1" settings, we are
> > not able to use provision mode.
>
> The no_write_same flag gates whether the WRITE SAME(10/16) commands
> are
> used to zero block ranges. It does not influence whether WRITE SAME with
> the UNMAP bit set is used to deallocate block ranges on devices which
> support logical block provisioning.

Hi Martin,

Thanks for this clarification. I checked further to understand difference
between
zeroout block ranges and deallocation (unmap) of block ranges using WRITE
SAME
command and impact of setting no_write_same flag.
So my understanding is when driver sets no_write_same flag, it is only
disabling
BLKZEROOUT/write_zeroes requests from using WRITE SAME command.
Instead individual writes are sent to zero the blocks (if
BLKDEV_ZERO_NOFALLBACK flag is not set).

>
> > Virtual Disk will not enable WRITE SAME feature providing correct VPD
> > block limit page.
>
> This wasn't always the case.
>
> If you want to enable WRITE SAME you'll have to provide some sort of
> heuristic to avoid breaking things on firmware versions and controllers
> that predate correct reporting.
>
Our inhouse driver code did not set the no_write_same flag.
And so far we have not seen any issues in this area with inhouse driver.
I am running tests now to check firmware behavior you mentioned.
Will get back to you with the results.

Are you aware of any issue that led to this change?

> > Virtual Disk in MegaRAID provides below values in VPD page 0xb0 which
> > will disable WRITE SAME commands.
>
> A value of 0 in
>
>   Maximum write same length: 0x0 blocks
>
> means the maximum length is not reported. It does not mean that WRITE
> SAME isn't supported.
>

Ok. With latest firmware, for virtual disks we do not set write same/unmap
support
in the logical block provisioning VPD page.
Will check behavior with older firmware versions also and get back.

Logical block provisioning VPD page (SBC):
  Unmap command supported (LBPU): 0
 Write same (16) with unmap bit supported (LBWS): 0
  Write same (10) with unmap bit supported (LBWS10): 0
  Logical block provisioning read zeros (LBPRZ): 0
  Anchored LBAs supported (ANC_SUP): 0
  Threshold exponent: 0
  Descriptor present (DP): 0
  Minimum percentage: 0
  Provisioning type: 0
  Threshold percentage: 0

Thanks,
Shivasharan

> --
> Martin K. Petersen	Oracle Linux Engineering
Martin K. Petersen Feb. 28, 2018, 3:18 a.m. UTC | #3
Shivasharan,

> So my understanding is when driver sets no_write_same flag, it is only
> disabling BLKZEROOUT/write_zeroes requests from using WRITE SAME
> command.  Instead individual writes are sent to zero the blocks (if
> BLKDEV_ZERO_NOFALLBACK flag is not set).

Correct.

> Our inhouse driver code did not set the no_write_same flag.  And so
> far we have not seen any issues in this area with inhouse driver.  I
> am running tests now to check firmware behavior you mentioned.  Will
> get back to you with the results.

> Are you aware of any issue that led to this change?

My recollection is that there were older MR controllers that would cause
problems when receiving a WRITE SAME command. I can't remember whether
this was with VD or PD.

For logical block provisioning we rely on LBPME in READ CAPACITY(16) and
the VPD page. So if you want to enable WRITE SAME for the purposes of
ATA TRIM translation or SCSI unmapping, filling out the block
provisioning fields correctly should suffice.

If you want to enable WRITE SAME for the purpose of zeroing block ranges
and the disk has an ATA Information VPD page, you need to set
no_write_same = 0 in the host template as well as claim support for
WRITE SAME(10) or WRITE SAME(16) in REPORT SUPPORTED OPERATION
CODES. Otherwise we'll disable zeroing under the assumption that the SAT
does not handle this correctly (which used to be the case).
diff mbox

Patch

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index a71ee67df084..dbb16718ce9e 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3176,7 +3176,6 @@  static struct scsi_host_template megasas_template = {
 	.bios_param = megasas_bios_param,
 	.use_clustering = ENABLE_CLUSTERING,
 	.change_queue_depth = scsi_change_queue_depth,
-	.no_write_same = 1,
 };
 
 /**