| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Pine.LNX.4.64.1303041711070.28666@file.rdu.redhat.com>
Date: Mon, 4 Mar 2013 17:15:45 -0500 (EST)
From: Mikulas Patocka <mpatocka@...hat.com>
To: Paul Taysom <taysom@...gle.com>
cc: Paul Taysom <taysom@...omium.org>, agk@...hat.com,
dm-devel@...hat.com, neilb@...e.de, linux-raid@...r.kernel.org,
linux-kernel@...r.kernel.org,
Mandeep Singh Baines <msb@...omium.org>,
Olof Johansson <olofj@...omium.org>
Subject: Re: [PATCH] md: dm-verity: Fix to avoid a deadlock in dm-bufio
On Mon, 4 Mar 2013, Paul Taysom wrote:
> On Mon, Mar 4, 2013 at 9:42 AM, Alasdair G Kergon <agk@...hat.com> wrote:
> > On Mon, Mar 04, 2013 at 08:45:48AM -0800, Paul Taysom wrote:
> >> @@ -449,8 +468,14 @@ static void verity_prefetch_io(struct dm_verity *v, struct dm_verity_io *io)
> >> hash_block_end = v->hash_blocks - 1;
> >> }
> >> no_prefetch_cluster:
> >> - dm_bufio_prefetch(v->bufio, hash_block_start,
> >> - hash_block_end - hash_block_start + 1);
> >> + vw = kmalloc(sizeof(*vw), GFP_KERNEL);
> >
> > kmalloc? mempool? ...
> >
> > Alasdair
> >
> The use of mempool would be a separate patch that would have to be
> measured for performance impact.
> -Paul
>
You don't have to use mempool. Just avoid prefetching if there is not
enough memory for the prefetch structure.
I reworked the patch, is uses an allocation that can fail and it generates
just one workqueue entry for one request (the original patch generated one
workqueue entry for hash tree level).
Please test the patch and if it works and performs well, let's submit it.
Mikulas
---
Changed the dm-verity prefetching to use a worker thread to avoid
a deadlock in dm-bufio.
If generic_make_request is called recursively, it queues the I/O
request on the current->bio_list without making the I/O request
and returns. The routine making the recursive call cannot wait
for the I/O to complete.
The deadlock occurred when one thread grabbed the bufio_client
mutex and waited for an I/O to complete but the I/O was queued
on another thread's current->bio_list and it was waiting to get
the mutex held by the first thread.
The fix allows only one I/O request from dm-verity to dm-bufio
per thread. To do this, the prefetch requests were queued on worker
threads.
In addition to avoiding the deadlock, this fix made a slight
improvement in performance.
seconds_kernel_to_login:
with prefetch: 8.43s
without prefetch: 9.2s
worker prefetch: 8.28s
Signed-off-by: Paul Taysom <taysom@...omium.org>
Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
Cc: stable@...nel.org
---
drivers/md/dm-bufio.c | 2 ++
drivers/md/dm-verity.c | 37 +++++++++++++++++++++++++++++++++----
2 files changed, 35 insertions(+), 4 deletions(-)
Index: linux-3.8-fast/drivers/md/dm-verity.c
===================================================================
--- linux-3.8-fast.orig/drivers/md/dm-verity.c 2013-03-04 22:49:20.000000000 +0100
+++ linux-3.8-fast/drivers/md/dm-verity.c 2013-03-04 23:10:45.000000000 +0100
@@ -93,6 +93,13 @@ struct dm_verity_io {
*/
};
+struct dm_verity_prefetch_work {
+ struct work_struct work;
+ struct dm_verity *v;
+ sector_t block;
+ unsigned n_blocks;
+};
+
static struct shash_desc *io_hash_desc(struct dm_verity *v, struct dm_verity_io *io)
{
return (struct shash_desc *)(io + 1);
@@ -424,15 +431,18 @@ static void verity_end_io(struct bio *bi
* The root buffer is not prefetched, it is assumed that it will be cached
* all the time.
*/
-static void verity_prefetch_io(struct dm_verity *v, struct dm_verity_io *io)
+static void verity_prefetch_io(struct work_struct *work)
{
+ struct dm_verity_prefetch_work *pw =
+ container_of(work, struct dm_verity_prefetch_work, work);
+ struct dm_verity *v = pw->v;
int i;
for (i = v->levels - 2; i >= 0; i--) {
sector_t hash_block_start;
sector_t hash_block_end;
- verity_hash_at_level(v, io->block, i, &hash_block_start, NULL);
- verity_hash_at_level(v, io->block + io->n_blocks - 1, i, &hash_block_end, NULL);
+ verity_hash_at_level(v, pw->block, i, &hash_block_start, NULL);
+ verity_hash_at_level(v, pw->block + pw->n_blocks - 1, i, &hash_block_end, NULL);
if (!i) {
unsigned cluster = ACCESS_ONCE(dm_verity_prefetch_cluster);
@@ -452,6 +462,25 @@ no_prefetch_cluster:
dm_bufio_prefetch(v->bufio, hash_block_start,
hash_block_end - hash_block_start + 1);
}
+
+ kfree(pw);
+}
+
+static void verity_submit_prefetch(struct dm_verity *v, struct dm_verity_io *io)
+{
+ struct dm_verity_prefetch_work *pw;
+
+ pw = kmalloc(sizeof(struct dm_verity_prefetch_work),
+ GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
+
+ if (!pw)
+ return;
+
+ INIT_WORK(&pw->work, verity_prefetch_io);
+ pw->v = v;
+ pw->block = io->block;
+ pw->n_blocks = io->n_blocks;
+ queue_work(v->verify_wq, &pw->work);
}
/*
@@ -498,7 +527,7 @@ static int verity_map(struct dm_target *
memcpy(io->io_vec, bio_iovec(bio),
io->io_vec_size * sizeof(struct bio_vec));
- verity_prefetch_io(v, io);
+ verity_submit_prefetch(v, io);
generic_make_request(bio);
Index: linux-3.8-fast/drivers/md/dm-bufio.c
===================================================================
--- linux-3.8-fast.orig/drivers/md/dm-bufio.c 2013-03-04 23:03:14.000000000 +0100
+++ linux-3.8-fast/drivers/md/dm-bufio.c 2013-03-04 23:04:19.000000000 +0100
@@ -1026,6 +1026,8 @@ void dm_bufio_prefetch(struct dm_bufio_c
{
struct blk_plug plug;
+ BUG_ON(dm_bufio_in_request());
+
blk_start_plug(&plug);
dm_bufio_lock(c);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists