From: Mikulas Patocka <mpatocka@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Vivek Goyal <vgoyal@redhat.com>,
Mike Snitzer <snitzer@redhat.com>,
Milan Broz <gmazyland@gmail.com>,
dm-devel@redhat.com, Andi Kleen <andi@firstfloor.org>,
dm-crypt@saout.de, linux-kernel@vger.kernel.org,
Christoph Hellwig <hch@infradead.org>,
Christian Schmidt <schmidt@digadd.de>
Subject: Re: dm-crypt parallelization patches
Date: Tue, 9 Apr 2013 16:32:28 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.64.1304091624110.19672@file.rdu.redhat.com> (raw)
In-Reply-To: <20130409195259.GL6186@mtj.dyndns.org>
On Tue, 9 Apr 2013, Tejun Heo wrote:
> On Tue, Apr 09, 2013 at 03:42:16PM -0400, Mikulas Patocka wrote:
> > If I drop ifdefs, it doesn't compile (because other cgroup stuff it
> > missing).
> >
> > So I enabled bio cgroups.
> >
> > bio_associate_current can't be used, because by the time we allocate the
> > outgoing write bio, we are no longer in the process that submitted the
> > original bio.
>
> Oh, I suppose it'd need some massaging to selectively turn off the
> cgroup part.
>
> > Anyway, I tried to reproduce in dm-crypt what bio_associate_current does -
>
> and we probably need to change that to bio_associate_task().
Generally, we shouldn't associate bios with "current" task in device
mapper targets. For example suppose that we have two stacked dm-crypt
targets:
In the "current" process pointer in lower dm-crypt target's request
function always points to the workqueue of the upper dm-crypt target that
submits the bios. So if we associate the bio with "current" in the lower
target, we are associating it with a preallocated workqueue and we already
lost the information who submitted it.
You should associate a bio with a task when you create the bio and "md"
and "dm" midlayers should just forward this association to lower layer
bios.
> > in the submitting process I record "ioc" and "css" fields in "dm_crypt_io"
> > structure and set these fields on all outgoing bios. It has no effect on
> > performance, it is as bad as if I hadn't done it.
>
> A good way to verify that the tagging is correct would be configuring
> io limits in block cgroup and see whether the limits are correctly
> applied when going through dm-crypt (please test with direct-io or
> reads, writeback is horribly broken, sorry).working correctly, maybe
> plugging is the overriding factor?
>
> Thanks.
It doesn't work because device mapper on the underlying layers ignores
bi_ioc and bi_css.
If I make device mapper forward bi_ioc and bi_css to outgoing bios, it
improves performance (from 2:30 to 1:30), but it is still far from
perfect.
Mikulas
---
dm: forward cgroup context
This patch makes dm forward associated cgroup context to cloned bios.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm.c | 9 +++++++++
fs/bio.c | 2 ++
2 files changed, 11 insertions(+)
Index: linux-3.8.6-fast/drivers/md/dm.c
===================================================================
--- linux-3.8.6-fast.orig/drivers/md/dm.c 2013-04-09 22:00:36.000000000 +0200
+++ linux-3.8.6-fast/drivers/md/dm.c 2013-04-09 22:19:40.000000000 +0200
@@ -453,6 +453,10 @@ static void free_io(struct mapped_device
static void free_tio(struct mapped_device *md, struct dm_target_io *tio)
{
+#ifdef CONFIG_BLK_CGROUP
+ tio->clone.bi_ioc = NULL;
+ tio->clone.bi_css = NULL;
+#endif
bio_put(&tio->clone);
}
@@ -1124,6 +1128,11 @@ static struct dm_target_io *alloc_tio(st
clone = bio_alloc_bioset(GFP_NOIO, nr_iovecs, ci->md->bs);
tio = container_of(clone, struct dm_target_io, clone);
+#ifdef CONFIG_BLK_CGROUP
+ tio->clone.bi_ioc = ci->bio->bi_ioc;
+ tio->clone.bi_css = ci->bio->bi_css;
+#endif
+
tio->io = ci->io;
tio->ti = ti;
memset(&tio->info, 0, sizeof(tio->info));
next prev parent reply other threads:[~2013-04-09 20:32 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Pine.LNX.4.64.1303252051520.9745@file.rdu.redhat.com>
2013-03-26 12:27 ` [dm-devel] dm-crypt performance Alasdair G Kergon
2013-03-26 20:05 ` Milan Broz
2013-03-26 20:28 ` Mike Snitzer
2013-03-26 20:58 ` Milan Broz
2013-03-28 18:53 ` Tejun Heo
2013-03-28 19:33 ` Vivek Goyal
2013-03-28 19:44 ` Tejun Heo
2013-03-28 20:38 ` Vivek Goyal
2013-03-28 20:45 ` Tejun Heo
2013-04-09 17:51 ` dm-crypt parallelization patches Mikulas Patocka
2013-04-09 17:57 ` Tejun Heo
2013-04-09 18:08 ` Mikulas Patocka
2013-04-09 18:10 ` Tejun Heo
2013-04-09 18:42 ` Vivek Goyal
2013-04-09 18:57 ` Tejun Heo
2013-04-09 19:13 ` Vivek Goyal
2013-04-09 19:42 ` Mikulas Patocka
2013-04-09 19:52 ` Tejun Heo
2013-04-09 20:32 ` Mikulas Patocka [this message]
2013-04-09 21:02 ` Tejun Heo
2013-04-09 21:03 ` Tejun Heo
2013-04-09 21:07 ` Vivek Goyal
2013-04-09 21:18 ` Mikulas Patocka
2013-04-10 19:24 ` Vivek Goyal
2013-04-10 23:42 ` [PATCH] make dm and dm-crypt forward cgroup context (was: dm-crypt parallelization patches) Mikulas Patocka
2013-04-10 23:50 ` Tejun Heo
2013-04-11 19:49 ` [PATCH v2] " Mikulas Patocka
2013-04-11 19:52 ` Tejun Heo
2013-04-11 20:00 ` Tejun Heo
2013-04-12 0:06 ` Mikulas Patocka
2013-04-12 0:22 ` Tejun Heo
2013-04-12 5:59 ` [PATCH v2] make dm and dm-crypt forward cgroup context Milan Broz
2013-04-12 18:17 ` [PATCH v2] make dm and dm-crypt forward cgroup context (was: dm-crypt parallelization patches) Mikulas Patocka
2013-04-12 18:01 ` Mikulas Patocka
2013-04-12 18:29 ` Tejun Heo
2013-04-15 13:02 ` Mikulas Patocka
2013-04-16 17:24 ` Tejun Heo
2013-04-16 19:41 ` Mikulas Patocka
2013-04-18 16:47 ` Mike Snitzer
2013-04-18 17:03 ` Tejun Heo
2013-05-22 18:50 ` Mike Snitzer
2013-05-22 19:48 ` Tejun Heo
2013-04-09 18:36 ` dm-crypt parallelization patches Vivek Goyal
2013-04-09 18:08 ` [dm-devel] dm-crypt performance Mikulas Patocka
2013-04-09 18:59 ` [dm-crypt] " Milan Broz
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=Pine.LNX.4.64.1304091624110.19672@file.rdu.redhat.com \
--to=mpatocka@redhat.com \
--cc=andi@firstfloor.org \
--cc=axboe@kernel.dk \
--cc=dm-crypt@saout.de \
--cc=dm-devel@redhat.com \
--cc=gmazyland@gmail.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=schmidt@digadd.de \
--cc=snitzer@redhat.com \
--cc=tj@kernel.org \
--cc=vgoyal@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 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).