On Wed, Mar 21, 2012 at 10:53:02PM -0400, Martin K. Petersen wrote:
> Nope, it's broken. But if you check my write same patches you'll see
> that I actually return the correct completion size there.
>
> There are several additional commands in the pipeline where the 1:1
> mapping between DMA size and block range is invalid. I want to get rid
> of the 1:1 assumption in general so we can handle any command without
> these evil workarounds.
What's the progress on getting these issues sorted out?
>>>>> "Christoph" == Christoph Hellwig <[email protected]> writes:
>> There are several additional commands in the pipeline where the 1:1
>> mapping between DMA size and block range is invalid. I want to get
>> rid of the 1:1 assumption in general so we can handle any command
>> without these evil workarounds.
Christoph> What's the progress on getting these issues sorted out?
This has bitrotted for a while. I'll put it on my list. I should finally
have some bandwidth again next week...
--
Martin K. Petersen Oracle Linux Engineering
2012/6/22 Martin K. Petersen <[email protected]>:
>>>>>> "Christoph" == Christoph Hellwig <[email protected]> writes:
>
>>> There are several additional commands in the pipeline where the 1:1
>>> mapping between DMA size and block range is invalid. I want to get
>>> rid of the 1:1 assumption in general so we can handle any command
>>> without these evil workarounds.
>
> Christoph> What's the progress on getting these issues sorted out?
>
> This has bitrotted for a while. I'll put it on my list. I should finally
> have some bandwidth again next week...
Any update on this? If this will not happen soon, should we just disable
discard request merge now?
Thanks,
Shaohua
On Thu, Jun 21, 2012 at 11:46 PM, Martin K. Petersen
<[email protected]> wrote:
>>>>>> "Christoph" == Christoph Hellwig <[email protected]> writes:
>
>>> There are several additional commands in the pipeline where the 1:1
>>> mapping between DMA size and block range is invalid. I want to get
>>> rid of the 1:1 assumption in general so we can handle any command
>>> without these evil workarounds.
>
> Christoph> What's the progress on getting these issues sorted out?
>
> This has bitrotted for a while. I'll put it on my list. I should finally
> have some bandwidth again next week...
Hey Martin,
I rebased (and fixed/tested) your writesame patches on v3.6-rc2 +
jens' for-linus branch, the git tree is available here:
https://github.com/snitm/linux/tree/writesame
I've also made the updated patchset available here:
http://people.redhat.com/msnitzer/patches/upstream/writesame/series.html
Should the writesame patches come before any discard merge or 1:1 DMA
and block range assumption fixes?
NOTE (for others besides martin):
http://people.redhat.com/msnitzer/patches/upstream/writesame/0001-block-Clean-up-merge-logic.patch
removes all the discard merge hacks; I think it provides a clean
baseline to then layer discard merge support back in -- but maybe
that's a flawed strategy?
Could be I've wasted a few hours by rebasing these patches...
regardless, it would be great if you could share what your plans are.
Thanks!
>>>>> "Mike" == Mike Snitzer <[email protected]> writes:
Mike> Could be I've wasted a few hours by rebasing these patches...
Mike> regardless, it would be great if you could share what your plans
Mike> are.
Heh, I worked on syncing my patch queue up to Jens' and James' trees
this afternoon. But I didn't quite finish the block stuff, mainly due to
some conflicts with a few topology changes I also have pending.
I'll take a look at your series. Maybe I'll swap things around and put
the topology changes on top instead of below. Leverage some of the work
you did...
--
Martin K. Petersen Oracle Linux Engineering
On Fri, Aug 17 2012 at 11:47pm -0400,
Martin K. Petersen <[email protected]> wrote:
> >>>>> "Mike" == Mike Snitzer <[email protected]> writes:
>
> Mike> Could be I've wasted a few hours by rebasing these patches...
> Mike> regardless, it would be great if you could share what your plans
> Mike> are.
>
> Heh, I worked on syncing my patch queue up to Jens' and James' trees
> this afternoon. But I didn't quite finish the block stuff, mainly due to
> some conflicts with a few topology changes I also have pending.
>
> I'll take a look at your series. Maybe I'll swap things around and put
> the topology changes on top instead of below. Leverage some of the work
> you did...
OK, just FYI, I had to change bio_has_data() to test bio->bi_vcnt
(rather than bio->bi_io_vec != NULL) because a discard bio has a
non-NULL bio->bi_io_vec (likely points to the bio->bi_inline_vecs but I
didn't check yet).
But I haven't put my finger on _why_ a discard bio has bio->bi_io_vec
(but given my use of DM, bio comes from bio_alloc_bioset, and DM passes
original bio->bi_max_vecs for nr_iovecs).
Anyway, this bio_has_data() change seemed reasonable considering
bio_data() checks bio->bi_vcnt.
On Mon, Aug 20, 2012 at 09:57:39AM -0400, Mike Snitzer wrote:
> But I haven't put my finger on _why_ a discard bio has bio->bi_io_vec
> (but given my use of DM, bio comes from bio_alloc_bioset, and DM passes
> original bio->bi_max_vecs for nr_iovecs).
TRIM has a payload and we cheay by preallocation a data page for it.
On Mon, Aug 20 2012 at 9:58am -0400,
Christoph Hellwig <[email protected]> wrote:
> On Mon, Aug 20, 2012 at 09:57:39AM -0400, Mike Snitzer wrote:
> > But I haven't put my finger on _why_ a discard bio has bio->bi_io_vec
> > (but given my use of DM, bio comes from bio_alloc_bioset, and DM passes
> > original bio->bi_max_vecs for nr_iovecs).
>
> TRIM has a payload and we cheay by preallocation a data page for it.
Thought we pushed that down? Hence sd_setup_discard_cmnd's
alloc_page + blk_add_request_payload hack.
On Mon, Aug 20, 2012 at 10:12:29AM -0400, Mike Snitzer wrote:
> Thought we pushed that down? Hence sd_setup_discard_cmnd's
> alloc_page + blk_add_request_payload hack.
Yeah, but we still need the bio_vec from early on, as it's allocated as
part of the bio.