All of lore.kernel.org
 help / color / mirror / Atom feed
* A target to just process bios background?
@ 2014-03-04  1:30 Akira Hayakawa
  2014-03-06 22:57 ` Mikulas Patocka
  0 siblings, 1 reply; 4+ messages in thread
From: Akira Hayakawa @ 2014-03-04  1:30 UTC (permalink / raw)
  To: dm-devel

Hi,

Is it meaningless to submit the split and cloned bios in background
using workqueue?

device-mapper doesn't go to next split before the .map hook returns
(and generic_make_request() returns only in case of DM_MAPIO_REMAPPED).
So, quitting from .map hook and going into next split fast sounds to me
effective at least in terms of CPU usage (in multicore system).

is this discussed before?

A target as tiny as linear or flakey can be thought:
- it has work_struct in per_bio_data
- .map hook queue_work the work into private wq.
- and then return DM_MAPIO_SUBMITTED

is this implemented before?

I think this target will make people happy if they
want to see what if the bio submission is done background
without changing their code but only stacking a dm target.

I am sorry if I am confused.

-- 
Akira

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A target to just process bios background?
  2014-03-04  1:30 A target to just process bios background? Akira Hayakawa
@ 2014-03-06 22:57 ` Mikulas Patocka
  2014-03-10  8:18   ` Akira Hayakawa
  0 siblings, 1 reply; 4+ messages in thread
From: Mikulas Patocka @ 2014-03-06 22:57 UTC (permalink / raw)
  To: Akira Hayakawa; +Cc: dm-devel



On Tue, 4 Mar 2014, Akira Hayakawa wrote:

> Hi,
> 
> Is it meaningless to submit the split and cloned bios in background
> using workqueue?
> 
> device-mapper doesn't go to next split before the .map hook returns
> (and generic_make_request() returns only in case of DM_MAPIO_REMAPPED).
> So, quitting from .map hook and going into next split fast sounds to me
> effective at least in terms of CPU usage (in multicore system).
> 
> is this discussed before?
> 
> A target as tiny as linear or flakey can be thought:
> - it has work_struct in per_bio_data
> - .map hook queue_work the work into private wq.
> - and then return DM_MAPIO_SUBMITTED
> 
> is this implemented before?
> 
> I think this target will make people happy if they
> want to see what if the bio submission is done background
> without changing their code but only stacking a dm target.
> 
> I am sorry if I am confused.

Hi

Such target doesn't exist and I wouldn't recommend to make it. If you 
add the bio to a queue and return immediatelly from the map routine, the 
caller sends another bio. So, in the end, you end up with extreme amount 
of bios on the queue and very bad responsiveness.

Suppose, for example, that you have a system with 16GB memory. 20% can be 
marked dirty (that's the default value for /proc/sys/vm/dirty_ratio), and 
if you use 4k bios, you may have 838860 bios on the queue. Any process 
that tries to write anything to disk freezes until all of them are 
written.

In fact, dm-crypt behaves this way - and we have a bug report that it 
causes out-of-memory crashes when massive amounts of bios are added to the 
queue.

dm-mirror also behaves this way, but only for write bios - you can load a 
mirror target with both legs pointing to the same device if you want to 
see how does it behave.

Mikulas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A target to just process bios background?
  2014-03-06 22:57 ` Mikulas Patocka
@ 2014-03-10  8:18   ` Akira Hayakawa
  2014-03-16 23:46     ` Akira Hayakawa
  0 siblings, 1 reply; 4+ messages in thread
From: Akira Hayakawa @ 2014-03-10  8:18 UTC (permalink / raw)
  To: dm-devel

Hi, Mikulas,

> Such target doesn't exist and I wouldn't recommend to make it. If you 
> add the bio to a queue and return immediatelly from the map routine, the 
> caller sends another bio. So, in the end, you end up with extreme amount 
> of bios on the queue and very bad responsiveness.
I see. It was just a question. Don't worry, dm-writeboost doesn't contain such a code.

By the way, I want to ask you a question related to this topic.

calling dm_io() in sync mode under .map hook causes deadlock.
To avoid that, for example, dm-snap-persistent.c executes it in a different thread.

        /*
         * Issue the synchronous I/O from a different thread
         * to avoid generic_make_request recursion.
         */
        INIT_WORK_ONSTACK(&req.work, do_metadata);
        queue_work(ps->metadata_wq, &req.work);
        flush_workqueue(ps->metadata_wq);
        destroy_work_on_stack(&req.work);

If we always queue the bio and executes it in a different thread
we never be suffered from this problem and that's why I call this topic is "related".

The cause of the deadlock is that the context has only one bio_list,
the bio submitted under sync dm_io() never be executed the caller bio completes.
Thus, deadlock.

If I understand correctly, any sync I/O causes deadlock if it's called under .map hook.
And blkdev_issue_flush() seems to be sync (submitting and wait for the completion after that) but
never cause deadlock in my environment.

Is it not allowed to call blkdev_issue_flush() directly under .map?
No target uses the function in device-mapper.

--
Akira

On Thu, 6 Mar 2014 17:57:21 -0500 (EST)
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Tue, 4 Mar 2014, Akira Hayakawa wrote:
> 
> > Hi,
> > 
> > Is it meaningless to submit the split and cloned bios in background
> > using workqueue?
> > 
> > device-mapper doesn't go to next split before the .map hook returns
> > (and generic_make_request() returns only in case of DM_MAPIO_REMAPPED).
> > So, quitting from .map hook and going into next split fast sounds to me
> > effective at least in terms of CPU usage (in multicore system).
> > 
> > is this discussed before?
> > 
> > A target as tiny as linear or flakey can be thought:
> > - it has work_struct in per_bio_data
> > - .map hook queue_work the work into private wq.
> > - and then return DM_MAPIO_SUBMITTED
> > 
> > is this implemented before?
> > 
> > I think this target will make people happy if they
> > want to see what if the bio submission is done background
> > without changing their code but only stacking a dm target.
> > 
> > I am sorry if I am confused.
> 
> Hi
> 
> Such target doesn't exist and I wouldn't recommend to make it. If you 
> add the bio to a queue and return immediatelly from the map routine, the 
> caller sends another bio. So, in the end, you end up with extreme amount 
> of bios on the queue and very bad responsiveness.
> 
> Suppose, for example, that you have a system with 16GB memory. 20% can be 
> marked dirty (that's the default value for /proc/sys/vm/dirty_ratio), and 
> if you use 4k bios, you may have 838860 bios on the queue. Any process 
> that tries to write anything to disk freezes until all of them are 
> written.
> 
> In fact, dm-crypt behaves this way - and we have a bug report that it 
> causes out-of-memory crashes when massive amounts of bios are added to the 
> queue.
> 
> dm-mirror also behaves this way, but only for write bios - you can load a 
> mirror target with both legs pointing to the same device if you want to 
> see how does it behave.
> 
> Mikulas
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


-- 
Akira Hayakawa <hayakawa@valinux.co.jp>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A target to just process bios background?
  2014-03-10  8:18   ` Akira Hayakawa
@ 2014-03-16 23:46     ` Akira Hayakawa
  0 siblings, 0 replies; 4+ messages in thread
From: Akira Hayakawa @ 2014-03-16 23:46 UTC (permalink / raw)
  To: device-mapper development

Hi,

> If I understand correctly, any sync I/O causes deadlock if it's called under .map hook.
> And blkdev_issue_flush() seems to be sync (submitting and wait for the completion after that) but
> never cause deadlock in my environment.
Sorry. I was really confused. That causes deadlock definitely.

> Is it not allowed to call blkdev_issue_flush() directly under .map?
> No target uses the function in device-mapper.
blkdev_issue_flush() is not used in device-mapper at present but instead
some targets uses dm_io with WRITE_FLUSH such as

- flush_header() in dm-log.c
- mirror_flush() in dm-raid1.c

these codes
- set the sector and count of dm_io_region to zero
- set the write buffer is NULL
- and then submits with WRITE_FLUSH flag using dm_io() routine.

the dm-insitu-compression target newly proposed by Shaohua
only uses blkdev_issue_flush() in its insitu_comp_flush_dirty_meta()
but it's called on background and thus, sane.

--
Akira

On Mon, 10 Mar 2014 17:18:07 +0900
Akira Hayakawa <hayakawa@valinux.co.jp> wrote:

> Hi, Mikulas,
> 
> > Such target doesn't exist and I wouldn't recommend to make it. If you 
> > add the bio to a queue and return immediatelly from the map routine, the 
> > caller sends another bio. So, in the end, you end up with extreme amount 
> > of bios on the queue and very bad responsiveness.
> I see. It was just a question. Don't worry, dm-writeboost doesn't contain such a code.
> 
> By the way, I want to ask you a question related to this topic.
> 
> calling dm_io() in sync mode under .map hook causes deadlock.
> To avoid that, for example, dm-snap-persistent.c executes it in a different thread.
> 
>         /*
>          * Issue the synchronous I/O from a different thread
>          * to avoid generic_make_request recursion.
>          */
>         INIT_WORK_ONSTACK(&req.work, do_metadata);
>         queue_work(ps->metadata_wq, &req.work);
>         flush_workqueue(ps->metadata_wq);
>         destroy_work_on_stack(&req.work);
> 
> If we always queue the bio and executes it in a different thread
> we never be suffered from this problem and that's why I call this topic is "related".
> 
> The cause of the deadlock is that the context has only one bio_list,
> the bio submitted under sync dm_io() never be executed the caller bio completes.
> Thus, deadlock.
> 
> If I understand correctly, any sync I/O causes deadlock if it's called under .map hook.
> And blkdev_issue_flush() seems to be sync (submitting and wait for the completion after that) but
> never cause deadlock in my environment.
> 
> Is it not allowed to call blkdev_issue_flush() directly under .map?
> No target uses the function in device-mapper.
> 
> --
> Akira
> 
> On Thu, 6 Mar 2014 17:57:21 -0500 (EST)
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > 
> > 
> > On Tue, 4 Mar 2014, Akira Hayakawa wrote:
> > 
> > > Hi,
> > > 
> > > Is it meaningless to submit the split and cloned bios in background
> > > using workqueue?
> > > 
> > > device-mapper doesn't go to next split before the .map hook returns
> > > (and generic_make_request() returns only in case of DM_MAPIO_REMAPPED).
> > > So, quitting from .map hook and going into next split fast sounds to me
> > > effective at least in terms of CPU usage (in multicore system).
> > > 
> > > is this discussed before?
> > > 
> > > A target as tiny as linear or flakey can be thought:
> > > - it has work_struct in per_bio_data
> > > - .map hook queue_work the work into private wq.
> > > - and then return DM_MAPIO_SUBMITTED
> > > 
> > > is this implemented before?
> > > 
> > > I think this target will make people happy if they
> > > want to see what if the bio submission is done background
> > > without changing their code but only stacking a dm target.
> > > 
> > > I am sorry if I am confused.
> > 
> > Hi
> > 
> > Such target doesn't exist and I wouldn't recommend to make it. If you 
> > add the bio to a queue and return immediatelly from the map routine, the 
> > caller sends another bio. So, in the end, you end up with extreme amount 
> > of bios on the queue and very bad responsiveness.
> > 
> > Suppose, for example, that you have a system with 16GB memory. 20% can be 
> > marked dirty (that's the default value for /proc/sys/vm/dirty_ratio), and 
> > if you use 4k bios, you may have 838860 bios on the queue. Any process 
> > that tries to write anything to disk freezes until all of them are 
> > written.
> > 
> > In fact, dm-crypt behaves this way - and we have a bug report that it 
> > causes out-of-memory crashes when massive amounts of bios are added to the 
> > queue.
> > 
> > dm-mirror also behaves this way, but only for write bios - you can load a 
> > mirror target with both legs pointing to the same device if you want to 
> > see how does it behave.
> > 
> > Mikulas
> > 
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
> 
> 
> -- 
> Akira Hayakawa <hayakawa@valinux.co.jp>
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-03-16 23:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-04  1:30 A target to just process bios background? Akira Hayakawa
2014-03-06 22:57 ` Mikulas Patocka
2014-03-10  8:18   ` Akira Hayakawa
2014-03-16 23:46     ` Akira Hayakawa

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.