From: Matthew Wilcox <matthew@wil.cx>
To: James Bottomley <James.Bottomley@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>,
linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org, liml@rtr.ca, jens.axboe@oracle.com,
dwmw2@infradead.org
Subject: Re: [PATCH 0/7] discard support revisited
Date: Sun, 30 Aug 2009 15:42:29 -0600 [thread overview]
Message-ID: <20090830214229.GG22870@parisc-linux.org> (raw)
In-Reply-To: <1251663439.10135.159.camel@mulgrave.site>
On Sun, Aug 30, 2009 at 03:17:19PM -0500, James Bottomley wrote:
> > > Jens had some objections to the block layer bits last time I posted
> > > these. I forget what they were now (this would have been around May
> > > 2nd, I think). What I've done instead in my current patchset (which
> > > undoubtedly has bugs because it isn't tested, because I'm not supposed
> > > to be working on the weekends) is to make sd_prep_fn() call a new method
> > > in the scsi_host_template. That should translate the discard request
> > > into a BLOCK_PC ATA_16 command, and we'll all be happy.
> > >
> > > It goes a little something like this:
> > > http://git.kernel.org/?p=linux/kernel/git/willy/ssd.git;a=shortlog;h=trim-20090829
> >
> > Queue flag and handling the discard in the prep function is much better
> > than the prepare function, yes. I don't like the prep_fn callout to the
> > host a lot.
>
> Me neither. I'm sort of OK with a transformed operation, callout for
> the ULD, but I really don't see why a disk only function should go
> through the host template.
I'm fine with putting the function pointer elsewhere ... the host template
seemed like a good place because the two things I see it being useful
for (USB and ATA) are a per-host thing rather than a per-device thing.
Would you like to see it in the scsi_disk instead? Maybe the scsi_device?
> So the last ATA data set management with TRIM proposal I saw had a set
> of discontiguous ranges, very like UNMAP. It's certainly possible to do
> the transformation, you just have to drop the sector buffer and add one
> for the ranges (then reverse it in the back translation for the
> completion) but it's not pretty.
Not pretty, and also has some practical problems, in that I cannot figure
out where Linux stores the length. Even after I put on a new page,
it would only send the first 24 bytes (length of the UNMAP payload)
rather than the 512 bytes of the TRIM payload.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
next prev parent reply other threads:[~2009-08-30 21:42 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-29 23:03 [PATCH 0/7] discard support revisited Christoph Hellwig
2009-08-29 23:03 ` [PATCH 1/7] Make DISCARD_BARRIER and DISCARD_NOBARRIER writes instead of reads Christoph Hellwig
2009-09-03 17:52 ` David Woodhouse
2009-09-03 17:56 ` Matthew Wilcox
2009-08-29 23:03 ` [PATCH 2/7] block: use blkdev_issue_discard in blk_ioctl_discard Christoph Hellwig
2009-09-01 9:06 ` Steven Whitehouse
2009-09-11 22:26 ` Christoph Hellwig
2009-09-14 9:40 ` Jens Axboe
2009-08-29 23:03 ` [PATCH 3/7] block: discard may need to allocate pages Christoph Hellwig
2009-08-29 23:03 ` [PATCH 4/7] sd: add support for WRITE SAME (16) with unmap bit Christoph Hellwig
2009-08-30 0:43 ` Douglas Gilbert
2009-08-30 1:05 ` Christoph Hellwig
2009-08-30 2:43 ` Douglas Gilbert
2009-08-30 2:48 ` Christoph Hellwig
2009-08-30 11:12 ` Sergei Shtylyov
2009-08-30 17:14 ` Christoph Hellwig
2009-08-29 23:03 ` [PATCH 5/7] libata: Add support for TRIM Christoph Hellwig
2009-08-29 23:03 ` [PATCH 6/7] block: allow large discard requests Christoph Hellwig
2009-08-30 2:49 ` Mark Lord
2009-08-30 2:50 ` Matthew Wilcox
2009-08-30 2:52 ` Mark Lord
2009-08-30 2:56 ` Christoph Hellwig
2009-08-29 23:03 ` [PATCH 7/7] xfs: add batches discard support Christoph Hellwig
2009-08-29 23:37 ` [PATCH 0/7] discard support revisited Matthew Wilcox
2009-08-30 2:15 ` Christoph Hellwig
2009-08-30 3:03 ` Matthew Wilcox
2009-08-30 20:17 ` James Bottomley
2009-08-30 21:42 ` Matthew Wilcox [this message]
2009-08-30 22:48 ` Christoph Hellwig
2009-09-02 19:46 ` Matthew Wilcox
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=20090830214229.GG22870@parisc-linux.org \
--to=matthew@wil.cx \
--cc=James.Bottomley@suse.de \
--cc=dwmw2@infradead.org \
--cc=hch@infradead.org \
--cc=jens.axboe@oracle.com \
--cc=liml@rtr.ca \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.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).