Attachment 304319 Details for Bug 444049 – Fix read-realloc race in RHEL 4

[patch] Fix read-realloc race in RHEL 4

linux-2.6.9-dm-snapshot-fix-read-realloc-race.patch (text/plain), 6.06 KB, created by Mikuláš Patočka on 2008-05-01 16:17:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mikuláš Patočka

Created: 2008-05-01 16:17:28 UTC

Size: 6.06 KB

patch

obsolete

>diff -X diff-exclude -u -r -p linux-2.6.9-mark-buffer-dirty/drivers/md/dm-snap.c linux-2.6.9-dm-snap-1/drivers/md/dm-snap.c
>--- linux-2.6.9-mark-buffer-dirty/drivers/md/dm-snap.c	2008-04-22 06:30:15.000000000 +0200
>+++ linux-2.6.9-dm-snap-1/drivers/md/dm-snap.c	2008-05-01 17:59:40.000000000 +0200
>@@ -38,6 +38,11 @@
>  */
> #define SNAPSHOT_PAGES 256
> 
>+/*
>+ * The size of mempool
>+ */
>+#define MIN_IOS 256
>+
> struct pending_exception {
> 	struct exception e;
> 
>@@ -88,6 +93,13 @@ static kmem_cache_t *exception_cache;
> static kmem_cache_t *pending_cache;
> static mempool_t *pending_pool;
> 
>+struct read_track {
>+	struct hlist_node read_list;
>+	chunk_t chunk;
>+};
>+
>+static kmem_cache_t *read_track_cache;
>+
> /*
>  * One of these per registered origin, held in the snapshot_origins hash
>  */
>@@ -434,6 +446,7 @@ static int set_chunk_size(struct dm_snap
> static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
> {
> 	struct dm_snapshot *s;
>+	int i;
> 	int r = -EINVAL;
> 	char persistent;
> 	char *origin_path;
>@@ -515,11 +528,22 @@ static int snapshot_ctr(struct dm_target
> 		goto bad5;
> 	}
> 
>+	s->read_track_pool = mempool_create(MIN_IOS, mempool_alloc_slab, mempool_free_slab, read_track_cache);
>+	if (!s->read_track_pool) {
>+		ti->error = "Could not allocate mempool for read tracking";
>+		goto bad6;
>+	}
>+
>+	for (i = 0; i < READ_TRACK_HASH_SIZE; i++) {
>+		INIT_HLIST_HEAD(&s->read_track_hash[i]);
>+	}
>+	spin_lock_init(&s->read_track_lock);
>+
> 	/* Metadata must only be loaded into one table at once */
> 	r = s->store.read_metadata(&s->store);
> 	if (r < 0) {
> 		ti->error = "Failed to read snapshot metadata";
>-		goto bad6;
>+		goto bad7;
> 	} else if (r > 0) {
> 		s->valid = 0;
> 		DMWARN("Snapshot is marked invalid.");
>@@ -530,7 +554,7 @@ static int snapshot_ctr(struct dm_target
> 	if (register_snapshot(s)) {
> 		r = -EINVAL;
> 		ti->error = "Cannot register snapshot origin";
>-		goto bad6;
>+		goto bad7;
> 	}
> 
> 	ti->private = s;
>@@ -538,6 +562,9 @@ static int snapshot_ctr(struct dm_target
> 
> 	return 0;
> 
>+ bad7:
>+	mempool_destroy(s->read_track_pool);
>+
>  bad6:
> 	kcopyd_client_destroy(s->kcopyd_client);
> 
>@@ -561,6 +588,7 @@ static int snapshot_ctr(struct dm_target
> 
> static void snapshot_dtr(struct dm_target *ti)
> {
>+	int i;
> 	struct dm_snapshot *s = (struct dm_snapshot *) ti->private;
> 
> 	/* Prevent further origin writes from using this snapshot. */
>@@ -575,6 +603,12 @@ static void snapshot_dtr(struct dm_targe
> 	/* Deallocate memory used */
> 	s->store.destroy(&s->store);
> 
>+	for (i = 0; i < READ_TRACK_HASH_SIZE; i++) {
>+		BUG_ON(!hlist_empty(&s->read_track_hash[i]));
>+	}
>+
>+	mempool_destroy(s->read_track_pool);
>+
> 	dm_put_device(ti, s->origin);
> 	dm_put_device(ti, s->cow);
> 
>@@ -895,13 +929,6 @@ static int snapshot_map(struct dm_target
> 		if (copy_needed)
> 			start_copy(pe);
> 	} else {
>-		/*
>-		 * FIXME: this read path scares me because we
>-		 * always use the origin when we have a pending
>-		 * exception.  However I can't think of a
>-		 * situation where this is wrong - ejt.
>-		 */
>-
> 		/* Do reads */
> 		down_read(&s->lock);
> 
>@@ -912,10 +939,17 @@ static int snapshot_map(struct dm_target
> 
> 		/* See if it it has been remapped */
> 		e = lookup_exception(&s->complete, chunk);
>-		if (e)
>+		if (e) {
> 			remap_exception(s, e, bio);
>-		else
>+		} else {
>+			struct read_track *rt = mempool_alloc(s->read_track_pool, GFP_NOIO);
>+			map_context->ptr = rt;
> 			bio->bi_bdev = s->origin->bdev;
>+			spin_lock_irq(&s->read_track_lock);
>+			rt->chunk = chunk;
>+			hlist_add_head(&rt->read_list, &s->read_track_hash[READ_TRACK_HASH(rt->chunk)]);
>+			spin_unlock_irq(&s->read_track_lock);
>+		}
> 
> 		up_read(&s->lock);
> 	}
>@@ -923,6 +957,23 @@ static int snapshot_map(struct dm_target
> 	return r;
> }
> 
>+static int snapshot_end_io(struct dm_target *ti, struct bio *bio,
>+			   int error, union map_info *map_context)
>+{
>+	struct dm_snapshot *s = ti->private;
>+	struct read_track *rt = map_context->ptr;
>+
>+	if (rt) {
>+		unsigned long flags;
>+		spin_lock_irqsave(&s->read_track_lock, flags);
>+		hlist_del(&rt->read_list);
>+		spin_unlock_irqrestore(&s->read_track_lock, flags);
>+		mempool_free(rt, s->read_track_pool);
>+	}
>+
>+	return 0;
>+}
>+
> static void snapshot_resume(struct dm_target *ti)
> {
> 	struct dm_snapshot *s = (struct dm_snapshot *) ti->private;
>@@ -1203,6 +1254,7 @@ static struct target_type snapshot_targe
> 	.ctr     = snapshot_ctr,
> 	.dtr     = snapshot_dtr,
> 	.map     = snapshot_map,
>+	.end_io  = snapshot_end_io,
> 	.resume  = snapshot_resume,
> 	.status  = snapshot_status,
> };
>@@ -1250,16 +1302,26 @@ static int __init dm_snapshot_init(void)
> 		goto bad4;
> 	}
> 
>+	read_track_cache = kmem_cache_create("read_track",
>+				sizeof(struct read_track), 0, 0, NULL, NULL);
>+	if (!read_track_cache) {
>+		DMERR("Couldn't create read track cache.");
>+		r = -ENOMEM;
>+		goto bad5;
>+	}
>+
> 	pending_pool = mempool_create(128, mempool_alloc_slab,
> 				      mempool_free_slab, pending_cache);
> 	if (!pending_pool) {
> 		DMERR("Couldn't create pending pool.");
> 		r = -ENOMEM;
>-		goto bad5;
>+		goto bad6;
> 	}
> 
> 	return 0;
> 
>+      bad6:
>+	kmem_cache_destroy(read_track_cache);
>       bad5:
> 	kmem_cache_destroy(pending_cache);
>       bad4:
>@@ -1289,6 +1351,7 @@ static void __exit dm_snapshot_exit(void
> 	mempool_destroy(pending_pool);
> 	kmem_cache_destroy(pending_cache);
> 	kmem_cache_destroy(exception_cache);
>+	kmem_cache_destroy(read_track_cache);
> }
> 
> /* Module hooks */
>diff -X diff-exclude -u -r -p linux-2.6.9-mark-buffer-dirty/drivers/md/dm-snap.h linux-2.6.9-dm-snap-1/drivers/md/dm-snap.h
>--- linux-2.6.9-mark-buffer-dirty/drivers/md/dm-snap.h	2008-04-22 06:30:15.000000000 +0200
>+++ linux-2.6.9-dm-snap-1/drivers/md/dm-snap.h	2008-05-01 17:44:30.000000000 +0200
>@@ -82,6 +82,9 @@ struct exception_store {
> 	void *context;
> };
> 
>+#define READ_TRACK_HASH_SIZE	16
>+#define READ_TRACK_HASH(x)	((unsigned long)(x) & (READ_TRACK_HASH_SIZE - 1))
>+
> struct dm_snapshot {
> 	struct rw_semaphore lock;
> 	struct dm_table *table;
>@@ -116,6 +119,10 @@ struct dm_snapshot {
> 	struct exception_store store;
> 
> 	struct kcopyd_client *kcopyd_client;
>+
>+	mempool_t *read_track_pool;
>+	spinlock_t read_track_lock;
>+	struct hlist_head read_track_hash[READ_TRACK_HASH_SIZE];
> };
> 
> /*

Actions: View | Diff

Attachments on bug 444049: 304319