linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Christoph Hellwig <hch@lst.de>, Tejun Heo <tj@kernel.org>,
	martin.petersen@oracle.com, axboe@kernel.dk,
	linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-block@vger.kernel.org
Subject: Re: support ranges TRIM for libata
Date: Thu, 23 Mar 2017 14:55:21 +0100	[thread overview]
Message-ID: <20170323135521.GA30361@lst.de> (raw)
In-Reply-To: <1490276861.2202.8.camel@HansenPartnership.com>

On Thu, Mar 23, 2017 at 09:47:41AM -0400, James Bottomley wrote:
> > The current implementation already has the issue of that it does
> > corrupt user data reliably if the using SG_IO for WRITE SAME
> > commands.
> 
> That does need fixing.

I don't think it's fixable as long as we translate the data payload.

> Why can't we do what the t10 sat document recommends: if the ATA device
> doesn't support the XL version (32 bit ranges) then translate unmap to
> multiple non-XL commands?

Because libata sits underneath the tag allocator we'd getting into
a giant set of problems.  Where do you expect the new commands to
magically come from?  And adhere to the queueing limits and not actually
deadlock in one way or another?

> I don't necessarily object to the vendor specific 1<->1 approach, it's
> just it won't fix the problem you cited above (SG_IO WRITE SAME), its
> just that now we error the command, which may cause some surprise.

We now error WRITE SAME for passthrough consistently.  Before we only
accepted it only with the unmap bit set, and even did so incorrectly
(not checking that the payload was all zeros).

> I
> also wonder if we couldn't simply do an ATA_16 TRIM if we're already
> going to all the trouble of recognising ATA devices in the sd discard
> path?

We probably could do that as well.  But that would drag a lot more
ATA-specific code into sd.c than just formatting the ranges.

  reply	other threads:[~2017-03-23 13:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-20 20:43 support ranges TRIM for libata Christoph Hellwig
2017-03-20 20:43 ` [PATCH 1/7] ѕd: split sd_setup_discard_cmnd Christoph Hellwig
2017-03-27 22:24   ` Bart Van Assche
2017-03-28 14:05     ` axboe
2017-03-30  8:49       ` hch
2017-03-20 20:43 ` [PATCH 2/7] sd: provide a new ata trim provisioning mode Christoph Hellwig
2017-03-27 22:40   ` Bart Van Assche
2017-03-20 20:43 ` [PATCH 3/7] libata: remove SCT WRITE SAME support Christoph Hellwig
2017-03-20 20:43 ` [PATCH 4/7] libata: simplify the trim implementation Christoph Hellwig
2017-03-20 20:43 ` [PATCH 5/7] block: add a max_discard_segment_size queue limit Christoph Hellwig
2017-03-27 23:09   ` Bart Van Assche
2017-03-20 20:43 ` [PATCH 6/7] sd: support multi-range TRIM for ATA disks Christoph Hellwig
2017-03-27 23:38   ` Bart Van Assche
2017-03-20 20:43 ` [PATCH 7/7] sd: use ZERO_PAGE for WRITE_SAME payloads Christoph Hellwig
2017-03-27 23:40   ` Bart Van Assche
2017-03-21 18:59 ` support ranges TRIM for libata Tejun Heo
2017-03-22 18:19   ` Christoph Hellwig
2017-03-23 13:47     ` James Bottomley
2017-03-23 13:55       ` Christoph Hellwig [this message]
2017-03-23 14:35         ` James Bottomley
2017-03-23 14:43           ` Christoph Hellwig
2017-03-23 15:04             ` Tejun Heo
2017-03-23 15:27               ` Christoph Hellwig
2017-03-23 15:30             ` Christoph Hellwig
2017-03-23 15:39               ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170323135521.GA30361@lst.de \
    --to=hch@lst.de \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).