From: Milan Broz <mbroz@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>,
device-mapper development <dm-devel@redhat.com>,
Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [RFC PATCH 00/20] dm-crypt: parallel processing
Date: Wed, 22 Aug 2012 12:28:41 +0200 [thread overview]
Message-ID: <5034B459.1000107@redhat.com> (raw)
In-Reply-To: <20120821182340.GA24861@google.com>
On 08/21/2012 08:23 PM, Tejun Heo wrote:
> Hello,
>
> (cc'ing Jens and Vivek, hi!)
>
> On Tue, Aug 21, 2012 at 11:37:43AM +0200, Milan Broz wrote:
>> Better adding cc to Tejun here, I still think there are several things
>> which perhaps should be done through kernel wq...
>>
>> (I would prefer to use kernel wq as well btw.)
>
> What do you mean by kernel wq? One of the system_*_wq's? If not,
> from scanning the patch names, it seems like it's converting to
> unbound workqueue from bound one.
I meant just extend bound workqueue as you mentioned below.
...
>>> 2) Could be kernel workqueue used/fixed here instead? Basically all it needs
>>> is to prefer submitting CPU, if it is busy just move work to another CPU.
>
> The problem, I suppose, is that w/ wq, it's either bound or completely
> unbound. If bound, the local CPU can become the bottleneck. If
> unbound, wq doesn't discern local and remote at all and thus loses any
> benefit from locality association.
>
> It would be nice if workqueue can somehow accomodate the situation
> better - maybe by migrating the worker to the issuing CPU before
> setting it loose so that the scheduler needs to migrate it away
> explicitly. Maybe we can do it opportunistically - e.g. record which
> CPU an unbound worker was on before entering idle and queue to local
> one if it exists. It wouldn't be trivial to implement tho. I'll
> think more about it.
Yes, something like this.
dmcrypt basically should have parameter which says how it will use workqueue(s)
IMHO three basic cases:
1) just use bound 1 wq per device. (this mode is usual for specific cases,
some people have several dmcrypt devices with RAID on top - it was workaround
to 1 thread per dmcrypt device in older kernel. This increased throughput
but with recent kernel it does exact opposite... So this mode provides
workaround for them.)
2) Use all CPU possible just prefer local CPU if available
(something between bound and unbound wq)
3) the same as 2) just with limited # of CPU per crypt device.
(but Mikulas' code uses also some batching of request - not sure how
to incorporate this)
Whatever, if logic is implemented in workqueue code, others can use t as well.
I would really prefer not to have "too smart" dmcrypt...
(Someone mentioned btrfs on top of it with all workqueues - how it can behave nicely
if every layer will try to implement own smart logic.)
Anyway, thanks for discussion, this is exactly what was missing here :)
Milan
next prev parent reply other threads:[~2012-08-22 10:28 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-21 9:08 [RFC PATCH 00/20] dm-crypt: parallel processing Milan Broz
2012-08-21 9:09 ` [PATCH 01/20] dm-crypt: remove per-cpu structure Mikulas Patocka
2012-08-21 9:09 ` [PATCH 02/20] dm-crypt: use unbound workqueue for request processing Mikulas Patocka
2012-08-21 9:09 ` [PATCH 03/20] dm-crypt: remove completion restart Mikulas Patocka
2012-08-21 9:09 ` [PATCH 04/20] dm-crypt: use encryption threads Mikulas Patocka
2012-08-21 9:09 ` [PATCH 05/20] dm-crypt: Unify spinlock Mikulas Patocka
2012-08-21 9:09 ` [PATCH 06/20] dm-crypt: Introduce an option that sets the number of threads Mikulas Patocka
2012-08-21 9:09 ` [PATCH 07/20] dm-crypt: don't use write queue Mikulas Patocka
2012-08-21 9:09 ` [PATCH 08/20] dm-crypt: simplify io queue Mikulas Patocka
2012-08-21 9:09 ` [PATCH 09/20] dm-crypt: unify io_queue and crypt_queue Mikulas Patocka
2012-08-21 9:09 ` [PATCH 10/20] dm-crypt: don't allocate pages for a partial request Mikulas Patocka
2012-08-21 9:09 ` [PATCH 11/20] dm-crypt: avoid deadlock in mempools Mikulas Patocka
2012-08-21 9:09 ` [PATCH 12/20] dm-crypt: simplify cc_pending Mikulas Patocka
2012-08-21 9:09 ` [PATCH 13/20] dm-crypt merge convert_context and dm_crypt_io Mikulas Patocka
2012-08-21 9:09 ` [PATCH 14/20] dm-crypt: move error handling to crypt_convert Mikulas Patocka
2012-08-21 9:09 ` [PATCH 15/20] dm-crypt: remove io_pending field Mikulas Patocka
2012-08-21 9:09 ` [PATCH 16/20] dm-crypt: small changes Mikulas Patocka
2012-08-21 9:09 ` [PATCH 17/20] dm-crypt: move temporary values to stack Mikulas Patocka
2012-08-21 9:09 ` [PATCH 18/20] dm-crypt: offload writes to thread Mikulas Patocka
2012-08-21 9:09 ` [PATCH 19/20] dm-crypt: retain write ordering Mikulas Patocka
2012-08-21 9:09 ` [PATCH 20/20] dm-crypt: sort writes Mikulas Patocka
2012-08-21 10:57 ` Alasdair G Kergon
2012-08-21 13:39 ` Mikulas Patocka
2012-08-21 9:37 ` [RFC PATCH 00/20] dm-crypt: parallel processing Milan Broz
2012-08-21 18:23 ` Tejun Heo
2012-08-21 19:26 ` Vivek Goyal
2012-08-22 10:28 ` Milan Broz [this message]
2012-08-23 20:15 ` Tejun Heo
2012-08-21 13:32 ` 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=5034B459.1000107@redhat.com \
--to=mbroz@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@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).