FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 08-27-2010, 02:13 PM
Mike Snitzer
 
Default dm: support REQ_FLUSH directly

On Fri, Aug 27 2010 at 1:52am -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:

> Hi Mike,
>
> (08/27/10 13:08), Mike Snitzer wrote:
> > But do you agree that the request-based barrier code (added in commit
> > d0bcb8786) could be reverted given the new FLUSH work?
>
> No, it's a separate thing.
> If we don't need to care about the case where multiple clones
> of flush request are necessary, the special casing of flush
> request can be removed regardless of the new FLUSH work.

Ah, yes thanks for clarifying. But we've never cared about multiple
clone of a flush so it's odd that such elaborate infrastructure was
introduced without a need.

> > We no longer need waiting now that ordering isn't a concern. Especially
>
> The waiting is not for ordering, but for multiple clones.
>
> > so given rq-based doesn't support multiple targets. As you know, from
> > dm_table_set_type:
> >
> > /*
> > * Request-based dm supports only tables that have a single target now.
> > * To support multiple targets, request splitting support is needed,
> > * and that needs lots of changes in the block-layer.
> > * (e.g. request completion process for partial completion.)
> > */
>
> This comment is about multiple targets.
> The special code for barrier is for single target whose
> num_flush_requests > 1. Different thing.

Yes, I need to not send mail just before going to bed..

> > I think we need to at least benchmark the performance of dm-mpath
> > without any of this extra, soon to be unnecessary, code.
>
> If there will be no need for supporting a request-based target
> with num_flush_requests > 1, the special handling of flush
> can be removed.
>
> And since there is no such target in the current tree,
> I don't object if you remove that part of code for good reason.

OK, certainly something to keep in mind. But _really_ knowing the
multipath FLUSH+FUA performance difference (extra special-case code vs
none) requires a full FLUSH conversion of request-based DM anyway.

In general, request-based DM's barrier/flush code does carry a certain
maintenance overhead. It is quite a bit of distracting code in the core
DM which isn't buying us anything.. so we _could_ just remove it and
never look back (until we have some specific need for num_flush_requests
> 1 in rq-based DM).

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-30-2010, 04:45 AM
"Jun'ichi Nomura"
 
Default dm: support REQ_FLUSH directly

Hi Mike,

(08/27/10 23:13), Mike Snitzer wrote:
>> If there will be no need for supporting a request-based target
>> with num_flush_requests > 1, the special handling of flush
>> can be removed.
>>
>> And since there is no such target in the current tree,
>> I don't object if you remove that part of code for good reason.
>
> OK, certainly something to keep in mind. But _really_ knowing the
> multipath FLUSH+FUA performance difference (extra special-case code vs
> none) requires a full FLUSH conversion of request-based DM anyway.
>
> In general, request-based DM's barrier/flush code does carry a certain
> maintenance overhead. It is quite a bit of distracting code in the core
> DM which isn't buying us anything.. so we _could_ just remove it and
> never look back (until we have some specific need for num_flush_requests
>> 1 in rq-based DM).

So, I'm not objecting to your idea.
Could you please create a patch to remove that?

Thanks,
--
Jun'ichi Nomura, NEC Corporation

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-30-2010, 08:33 AM
Tejun Heo
 
Default dm: support REQ_FLUSH directly

On 08/30/2010 06:45 AM, Jun'ichi Nomura wrote:
> Hi Mike,
>
> (08/27/10 23:13), Mike Snitzer wrote:
>>> If there will be no need for supporting a request-based target
>>> with num_flush_requests > 1, the special handling of flush
>>> can be removed.
>>>
>>> And since there is no such target in the current tree,
>>> I don't object if you remove that part of code for good reason.
>>
>> OK, certainly something to keep in mind. But _really_ knowing the
>> multipath FLUSH+FUA performance difference (extra special-case code vs
>> none) requires a full FLUSH conversion of request-based DM anyway.
>>
>> In general, request-based DM's barrier/flush code does carry a certain
>> maintenance overhead. It is quite a bit of distracting code in the core
>> DM which isn't buying us anything.. so we _could_ just remove it and
>> never look back (until we have some specific need for num_flush_requests
>>> 1 in rq-based DM).
>
> So, I'm not objecting to your idea.
> Could you please create a patch to remove that?

I did that yesterday. Will post the patch soon.

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-30-2010, 12:43 PM
Mike Snitzer
 
Default dm: support REQ_FLUSH directly

On Mon, Aug 30 2010 at 4:33am -0400,
Tejun Heo <tj@kernel.org> wrote:

> On 08/30/2010 06:45 AM, Jun'ichi Nomura wrote:
> > Hi Mike,
> >
> > (08/27/10 23:13), Mike Snitzer wrote:
> >>> If there will be no need for supporting a request-based target
> >>> with num_flush_requests > 1, the special handling of flush
> >>> can be removed.
> >>>
> >>> And since there is no such target in the current tree,
> >>> I don't object if you remove that part of code for good reason.
> >>
> >> OK, certainly something to keep in mind. But _really_ knowing the
> >> multipath FLUSH+FUA performance difference (extra special-case code vs
> >> none) requires a full FLUSH conversion of request-based DM anyway.
> >>
> >> In general, request-based DM's barrier/flush code does carry a certain
> >> maintenance overhead. It is quite a bit of distracting code in the core
> >> DM which isn't buying us anything.. so we _could_ just remove it and
> >> never look back (until we have some specific need for num_flush_requests
> >>> 1 in rq-based DM).
> >
> > So, I'm not objecting to your idea.
> > Could you please create a patch to remove that?
>
> I did that yesterday. Will post the patch soon.

I did it yesterday also, mine builds on your previous DM patchset...

I'll review your recent patchset, from today, to compare and will share
my findings.

I was hoping we could get the current request-based code working with
your new FLUSH+FUA work without removing support for num_flush_requests
(yet). And then layer in the removal to give us the before and after so
we would know the overhead associated with keeping/dropping
num_flush_requests. But like I said earlier "we _could_ just remove it
and never look back".

Thanks,
Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 08-30-2010, 12:45 PM
Tejun Heo
 
Default dm: support REQ_FLUSH directly

Hello,

On 08/30/2010 02:43 PM, Mike Snitzer wrote:
> I did it yesterday also, mine builds on your previous DM patchset...
>
> I'll review your recent patchset, from today, to compare and will share
> my findings.

Thanks. :-)

> I was hoping we could get the current request-based code working with
> your new FLUSH+FUA work without removing support for num_flush_requests
> (yet). And then layer in the removal to give us the before and after so
> we would know the overhead associated with keeping/dropping
> num_flush_requests. But like I said earlier "we _could_ just remove it
> and never look back".

I tried but it's not very easy because the original implementation
depended on the block layer suppressing other requests while flush
sequence is in progress. The painful part was that block layer no
longer sorts requeued flush requests in front of other front inserted
requests, so explicit queue suppressing can't be implemented simply.
Another route would be adding a separate wait/wakeup logic for flushes
(someone posted a demo patch for that which was almost there but not
fully), but it seemed like a aimless effort to build a new facility to
rip it out in the next patch. After all, the whole thing seemed
somewhat pointless given that writes can't be routed to multiple
targets (if writes can't target multiple devices, flushes won't need
to either).

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 09:45 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org