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 03-12-2009, 10:51 AM
Milan Broz
 
Default dm crypt: wait for possible unfinished endio() call in destructor

dm crypt: Wait for possible unfinished endio() call in destructor

When user set dm-crypt over loop device and the loop thread processing
bios calls bio_endio later than the dm-crypt mapping is destroyed
(including mempool for dm io request), the endio can cause this OOPs:

(mempool_free from already destroyed mempool).

[ 70.381058] EIP is at mempool_free+0x12/0x6b
...
[ 70.381058] Process loop0 (pid: 4268, ti=cf3b2000 task=cf1cc1f0 task.ti=cf3b2000)
...
[ 70.381058] Call Trace:
[ 70.381058] [<d0d76601>] ? crypt_dec_pending+0x5e/0x62 [dm_crypt]
[ 70.381058] [<d0d767b8>] ? crypt_endio+0xa2/0xaa [dm_crypt]
[ 70.381058] [<d0d76716>] ? crypt_endio+0x0/0xaa [dm_crypt]
[ 70.381058] [<c01a2f24>] ? bio_endio+0x2b/0x2e
[ 70.381058] [<d0806530>] ? dec_pending+0x224/0x23b [dm_mod]
[ 70.381058] [<d08066e4>] ? clone_endio+0x79/0xa4 [dm_mod]
[ 70.381058] [<d080666b>] ? clone_endio+0x0/0xa4 [dm_mod]
[ 70.381058] [<c01a2f24>] ? bio_endio+0x2b/0x2e
[ 70.381058] [<c02bad86>] ? loop_thread+0x380/0x3b7
[ 70.381058] [<c02ba8a1>] ? do_lo_send_aops+0x0/0x165
[ 70.381058] [<c013754f>] ? autoremove_wake_function+0x0/0x33
[ 70.381058] [<c02baa06>] ? loop_thread+0x0/0x3b7

Fix it by adding reference counter into crypt config and wait till
all endio operations finishes.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
drivers/md/dm-crypt.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 35bda49..6cef632 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -18,6 +18,7 @@
#include <linux/crypto.h>
#include <linux/workqueue.h>
#include <linux/backing-dev.h>
+#include <linux/delay.h>
#include <asm/atomic.h>
#include <linux/scatterlist.h>
#include <asm/page.h>
@@ -95,6 +96,8 @@ struct crypt_config {
struct workqueue_struct *io_queue;
struct workqueue_struct *crypt_queue;

+ atomic_t pending;
+
/*
* crypto related data
*/
@@ -566,6 +569,7 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
}

mempool_free(io, cc->io_pool);
+ atomic_dec(&cc->pending);
}

/*
@@ -1113,6 +1117,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
goto bad_crypt_queue;
}

+ atomic_set(&cc->pending, 0);
+
ti->private = cc;
return 0;

@@ -1149,6 +1155,9 @@ static void crypt_dtr(struct dm_target *ti)
destroy_workqueue(cc->io_queue);
destroy_workqueue(cc->crypt_queue);

+ while (atomic_read(&cc->pending))
+ msleep(1);
+
if (cc->req)
mempool_free(cc->req, cc->req_pool);

@@ -1171,8 +1180,11 @@ static void crypt_dtr(struct dm_target *ti)
static int crypt_map(struct dm_target *ti, struct bio *bio,
union map_info *map_context)
{
+ struct crypt_config *cc = ti->private;
struct dm_crypt_io *io;

+ atomic_inc(&cc->pending);
+
io = crypt_io_alloc(ti, bio, bio->bi_sector - ti->begin);

if (bio_data_dir(io->base_bio) == READ)


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-13-2009, 12:53 PM
Alasdair G Kergon
 
Default dm crypt: wait for possible unfinished endio() call in destructor

Latest version of this patch. Please would some people test and/or
review it asap so we can get it fixed in 2.6.29?

Alasdair

From: Milan Broz <mbroz@redhat.com>

The following oops has been reported when dm-crypt runs over a loop device.

...
[ 70.381058] Process loop0 (pid: 4268, ti=cf3b2000 task=cf1cc1f0 task.ti=cf3b2000)
...
[ 70.381058] Call Trace:
[ 70.381058] [<d0d76601>] ? crypt_dec_pending+0x5e/0x62 [dm_crypt]
[ 70.381058] [<d0d767b8>] ? crypt_endio+0xa2/0xaa [dm_crypt]
[ 70.381058] [<d0d76716>] ? crypt_endio+0x0/0xaa [dm_crypt]
[ 70.381058] [<c01a2f24>] ? bio_endio+0x2b/0x2e
[ 70.381058] [<d0806530>] ? dec_pending+0x224/0x23b [dm_mod]
[ 70.381058] [<d08066e4>] ? clone_endio+0x79/0xa4 [dm_mod]
[ 70.381058] [<d080666b>] ? clone_endio+0x0/0xa4 [dm_mod]
[ 70.381058] [<c01a2f24>] ? bio_endio+0x2b/0x2e
[ 70.381058] [<c02bad86>] ? loop_thread+0x380/0x3b7
[ 70.381058] [<c02ba8a1>] ? do_lo_send_aops+0x0/0x165
[ 70.381058] [<c013754f>] ? autoremove_wake_function+0x0/0x33
[ 70.381058] [<c02baa06>] ? loop_thread+0x0/0x3b7

When a table is being replaced, it waits for I/O to complete
before destroying the mempool, but the endio function doesn't
call mempool_free() until after completing the bio.

Fix it by swapping the order of those two operations.

The same problem occurs in dm.c with referenced after dec_pending.
Again, we swap the order.

Signed-off-by: Milan Broz <mbroz@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>
---
drivers/md/dm-crypt.c | 17 ++++++++++-------
drivers/md/dm.c | 32 +++++++++++++++++++-------------
2 files changed, 29 insertions(+), 20 deletions(-)

Index: linux-2.6.29-rc7/drivers/md/dm-crypt.c
================================================== =================
--- linux-2.6.29-rc7.orig/drivers/md/dm-crypt.c 2009-03-13 11:06:18.000000000 +0000
+++ linux-2.6.29-rc7/drivers/md/dm-crypt.c 2009-03-13 11:07:46.000000000 +0000
@@ -568,19 +568,22 @@ static void crypt_inc_pending(struct dm_
static void crypt_dec_pending(struct dm_crypt_io *io)
{
struct crypt_config *cc = io->target->private;
+ struct bio *base_bio = io->base_bio;
+ struct dm_crypt_io *base_io = io->base_io;
+ int error = io->error;

if (!atomic_dec_and_test(&io->pending))
return;

- if (likely(!io->base_io))
- bio_endio(io->base_bio, io->error);
+ mempool_free(io, cc->io_pool);
+
+ if (likely(!base_io))
+ bio_endio(base_bio, error);
else {
- if (io->error && !io->base_io->error)
- io->base_io->error = io->error;
- crypt_dec_pending(io->base_io);
+ if (error && !base_io->error)
+ base_io->error = error;
+ crypt_dec_pending(base_io);
}
-
- mempool_free(io, cc->io_pool);
}

/*
Index: linux-2.6.29-rc7/drivers/md/dm.c
================================================== =================
--- linux-2.6.29-rc7.orig/drivers/md/dm.c 2009-03-09 22:12:09.000000000 +0000
+++ linux-2.6.29-rc7/drivers/md/dm.c 2009-03-13 12:34:19.000000000 +0000
@@ -525,9 +525,12 @@ static int __noflush_suspending(struct m
static void dec_pending(struct dm_io *io, int error)
{
unsigned long flags;
+ int io_error;
+ struct bio *bio;
+ struct mapped_device *md = io->md;

/* Push-back supersedes any I/O errors */
- if (error && !(io->error > 0 && __noflush_suspending(io->md)))
+ if (error && !(io->error > 0 && __noflush_suspending(md)))
io->error = error;

if (atomic_dec_and_test(&io->io_count)) {
@@ -537,24 +540,27 @@ static void dec_pending(struct dm_io *io
* This must be handled before the sleeper on
* suspend queue merges the pushback list.
*/
- spin_lock_irqsave(&io->md->pushback_lock, flags);
- if (__noflush_suspending(io->md))
- bio_list_add(&io->md->pushback, io->bio);
+ spin_lock_irqsave(&md->pushback_lock, flags);
+ if (__noflush_suspending(md))
+ bio_list_add(&md->pushback, io->bio);
else
/* noflush suspend was interrupted. */
io->error = -EIO;
- spin_unlock_irqrestore(&io->md->pushback_lock, flags);
+ spin_unlock_irqrestore(&md->pushback_lock, flags);
}

end_io_acct(io);

- if (io->error != DM_ENDIO_REQUEUE) {
- trace_block_bio_complete(io->md->queue, io->bio);
+ io_error = io->error;
+ bio = io->bio;

- bio_endio(io->bio, io->error);
- }
+ free_io(md, io);

- free_io(io->md, io);
+ if (io_error != DM_ENDIO_REQUEUE) {
+ trace_block_bio_complete(md->queue, bio);
+
+ bio_endio(bio, io_error);
+ }
}
}

@@ -562,6 +568,7 @@ static void clone_endio(struct bio *bio,
{
int r = 0;
struct dm_target_io *tio = bio->bi_private;
+ struct dm_io *io = tio->io;
struct mapped_device *md = tio->io->md;
dm_endio_fn endio = tio->ti->type->end_io;

@@ -585,15 +592,14 @@ static void clone_endio(struct bio *bio,
}
}

- dec_pending(tio->io, error);
-
/*
* Store md for cleanup instead of tio which is about to get freed.
*/
bio->bi_private = md->bs;

- bio_put(bio);
free_tio(md, tio);
+ bio_put(bio);
+ dec_pending(io, error);
}

static sector_t max_io_len(struct mapped_device *md,

--
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 04:43 AM.

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