All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: James Bottomley <James.Bottomley@suse.de>
Cc: Mike Snitzer <snitzer@redhat.com>, Christoph Hellwig <hch@lst.de>,
	axboe@kernel.dk, dm-devel@redhat.com,
	linux-kernel@vger.kernel.org, martin.petersen@oracle.com,
	akpm@linux-foundation.org, linux-scsi@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: [PATCH 1/2] block: fix leaks associated with discard request payload
Date: Mon, 28 Jun 2010 13:16:42 -0400	[thread overview]
Message-ID: <yq1sk47dqmd.fsf@sermon.lab.mkp.net> (raw)
In-Reply-To: <1277652576.4366.19.camel@mulgrave.site> (James Bottomley's message of "Sun, 27 Jun 2010 10:29:36 -0500")

>>>>> "James" == James Bottomley <James.Bottomley@suse.de> writes:

James> I really hate these growing contortions for discard.  They're a
James> clear signal that we haven't implemented it right.

James> So let's first work out how it should be done.  I really like
James> Tomo's idea of doing discard through the normal REQ_TYPE_FS
James> route, which means we can control the setup in prep and the tear
James> down in done, all confined to the ULD.

Yeah, this is what I was trying to do a couple of months ago.  Trying to
make discard and write same filesystem class requests so we can split,
merge, etc. like READs and WRITEs.  I still think this is how we should
do it but it's a lot of work.

There are several challenges involved.  I was doing the "payload"
allocation at request allocation time by permitting a buffer trailing
struct request (size defined by ULD depending on req type).  However, we
have a few places in the stack where we memcpy requests and assume them
to be the same size.  That needs to be fixed.  That's also the roadblock
I ran into wrt. 32-byte CDB allocation so for that I ended up allocating
the command in sd.

Also, another major headache of mine is WRITE SAME/UNMAP to DSM TRIM
conversion.  Because of the limitations of the TRIM command format a
single WRITE SAME can turn into effectively hundreds of TRIM commands to
be issued.  I tried to limit this by using UNMAP translation instead.
But we can still get into cases where we need to either loop or allocate
a bunch of TRIMs in the translation layer.  That leaves two options:
Either pass really conservative limits up the stack and loop up there.
Or deal with the allocation/translation stuff at the bottom of the pile.
None of my attempts in these departments turned out to be very nice.
I'm still dreaming of the day where libata moves out from under SCSI so
we don't have to translate square pegs into round holes...

-- 
Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2010-06-28 17:18 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-18 14:59 [PATCH, RFC] block: don't allocate a payload for discard request Christoph Hellwig
2010-06-19  4:25 ` Mike Snitzer
2010-06-22 18:00   ` Mike Snitzer
2010-06-26 19:56     ` [PATCH 1/2] block: fix leaks associated with discard request payload Mike Snitzer
2010-06-27  8:49       ` FUJITA Tomonori
2010-06-27  9:26         ` Christoph Hellwig
2010-06-27 10:01           ` FUJITA Tomonori
2010-06-27 10:35             ` FUJITA Tomonori
2010-06-27 11:07               ` Christoph Hellwig
2010-06-27 12:32                 ` FUJITA Tomonori
2010-06-27 14:16                   ` Mike Snitzer
2010-06-27 15:35                     ` Christoph Hellwig
2010-06-27 16:23                       ` FUJITA Tomonori
2010-06-27 15:33                   ` Christoph Hellwig
2010-06-28  7:57                   ` Christoph Hellwig
2010-06-28  8:14                     ` FUJITA Tomonori
2010-06-28  8:18                       ` Jens Axboe
2010-06-28  8:45                         ` FUJITA Tomonori
2010-06-28  8:45                           ` FUJITA Tomonori
2010-06-28 15:25                       ` Christoph Hellwig
2010-06-30 11:55                         ` FUJITA Tomonori
2010-07-01  4:21                           ` FUJITA Tomonori
2010-06-27  9:38       ` Christoph Hellwig
2010-06-27 15:29       ` James Bottomley
2010-06-28 17:16         ` Martin K. Petersen [this message]
2010-06-29  8:00           ` Boaz Harrosh
2010-06-29 22:28         ` [dm-devel] " Mikulas Patocka
2010-06-29 23:03           ` James Bottomley
2010-06-29 23:51             ` Mike Snitzer
2010-06-29 23:51               ` Mike Snitzer
2010-06-30  0:11             ` [dm-devel] " Mikulas Patocka
2010-06-30 14:22               ` James Bottomley
2010-06-30 15:36                 ` Mike Snitzer
2010-06-30 16:26                   ` James Bottomley
2010-07-01 12:28                 ` [dm-devel] " Mikulas Patocka
2010-07-01 12:46                   ` Mike Snitzer
2010-07-01 14:03                     ` Mikulas Patocka
2010-07-01 14:03                       ` Mikulas Patocka
2010-07-01 12:49                   ` [dm-devel] " Alasdair G Kergon
2010-06-30  8:32         ` Boaz Harrosh
2010-06-30  8:42           ` Christoph Hellwig
2010-06-30 10:25             ` Boaz Harrosh
2010-06-30 10:41               ` Christoph Hellwig
2010-06-30 10:57                 ` Boaz Harrosh
2010-06-30 12:18                   ` Mike Snitzer
2010-06-26 19:56     ` [PATCH 2/2] block: defer the use of inline biovecs for discard requests Mike Snitzer
2010-06-27  9:39       ` Christoph Hellwig
2010-06-27 14:00         ` Mike Snitzer
2010-06-27 14:55       ` [PATCH 2/2 v2] " Mike Snitzer
2010-06-27 15:33         ` Christoph Hellwig
2010-06-28 10:33       ` [PATCH 2/2] " FUJITA Tomonori
2010-06-28 12:29         ` Mike Snitzer
2010-06-28 15:15           ` FUJITA Tomonori
2010-06-28 15:31             ` Mike Snitzer
2010-06-28 12:34       ` Jens Axboe
2010-06-28 12:37         ` Mike Snitzer
2010-06-28 12:41           ` Jens Axboe
2010-06-28 12:44             ` Christoph Hellwig
2010-06-28 12:49               ` Jens Axboe
2010-06-28 12:45             ` Mike Snitzer

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=yq1sk47dqmd.fsf@sermon.lab.mkp.net \
    --to=martin.petersen@oracle.com \
    --cc=James.Bottomley@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.