From patchwork Tue Dec 8 04:26:20 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Snitzer X-Patchwork-Id: 65635 X-Patchwork-Delegate: agk@redhat.com Received: from hormel.redhat.com (hormel1.redhat.com [209.132.177.33]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id nB84QNuD019408 for ; Tue, 8 Dec 2009 04:26:23 GMT Received: from listman.util.phx.redhat.com (listman.util.phx.redhat.com [10.8.4.110]) by hormel.redhat.com (Postfix) with ESMTP id 4AEAD619F3D; Mon, 7 Dec 2009 23:26:23 -0500 (EST) Received: from int-mx01.intmail.prod.int.phx2.redhat.com (nat-pool.util.phx.redhat.com [10.8.5.200]) by listman.util.phx.redhat.com (8.13.1/8.13.1) with ESMTP id nB84QKLO001410 for ; Mon, 7 Dec 2009 23:26:20 -0500 Received: from localhost (dhcp-100-19-150.bos.redhat.com [10.16.19.150]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id nB84QK23032687 for ; Mon, 7 Dec 2009 23:26:20 -0500 From: Mike Snitzer To: dm-devel@redhat.com Date: Mon, 7 Dec 2009 23:26:20 -0500 Message-Id: <1260246380-32576-1-git-send-email-snitzer@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-loop: dm-devel@redhat.com Subject: [dm-devel] [PATCH] dm snapshot: remove try_again goto from snapshot_merge_process() X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.5 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com Index: linux-rhel6/drivers/md/dm-snap.c =================================================================== --- linux-rhel6.orig/drivers/md/dm-snap.c +++ linux-rhel6/drivers/md/dm-snap.c @@ -766,8 +766,8 @@ static int init_hash_tables(struct dm_sn static void flush_bios(struct bio *bio); static void error_bios(struct bio *bio); -static int __origin_write(struct list_head *snapshots, - sector_t sector, struct bio *bio); +static int origin_write_extent(struct dm_snapshot *merging_snap, + sector_t sector, chunk_t size); static void merge_callback(int read_err, unsigned long write_err, void *context); @@ -785,10 +785,8 @@ static u64 read_pending_exception_done_c static void snapshot_merge_process(struct dm_snapshot *s) { - int r, i, linear_chunks; - chunk_t old_chunk, new_chunk, n; - struct origin *o; - int must_wait; + int i, linear_chunks; + chunk_t old_chunk, new_chunk; struct dm_io_region src, dest; sector_t io_size; u64 previous_count; @@ -832,49 +830,13 @@ static void snapshot_merge_process(struc src.sector = chunk_to_sector(s->store, new_chunk); src.count = dest.count; - /* - * Reallocate the other snapshots: - * - * The chunk size of the merging snapshot may be larger than the chunk - * size of some other snapshot. So we may need to reallocate multiple - * chunks in a snapshot. - * - * We don't do linking of pending exceptions and waiting for the last - * one --- that would complicate code too much and it would also be - * bug-prone. - * - * Instead, we try to scan all the overlapping exceptions in all - * non-merging snapshots and if something was reallocated then wait - * for any pending exception to complete. Retry after the wait, until - * all exceptions are done. - * - * This may seem ineffective, but in practice, people hardly use more - * than one or two snapshots. In case of two snapshots (one merging and - * one non-merging) with the same chunksize, wait and wakeup is done - * only once. - */ - -test_again: + /* Reallocate the other snapshots */ previous_count = read_pending_exception_done_count(); - must_wait = 0; - - /* - * Merging snapshot already has the origin's __minimum_chunk_size() - * stored in split_io (see: snapshot_merge_resume); avoid rediscovery - */ - BUG_ON(!s->ti->split_io); - down_read(&_origins_lock); - o = __lookup_origin(s->origin->bdev); - for (n = 0; n < io_size; n += s->ti->split_io) { - r = __origin_write(&o->snapshots, dest.sector + n, NULL); - if (r == DM_MAPIO_SUBMITTED) - must_wait = 1; - } - up_read(&_origins_lock); - if (must_wait) { + while (origin_write_extent(s, dest.sector, io_size)) { wait_event(_pending_exception_done, read_pending_exception_done_count() != previous_count); - goto test_again; + /* Retry after the wait, until all exceptions are done. */ + previous_count = read_pending_exception_done_count(); } down_write(&s->lock); @@ -1984,6 +1946,55 @@ static int do_origin(struct dm_dev *orig } /* + * Trigger exceptions in all non-merging snapshots for the + * specified extent: + * + * The chunk size of the merging snapshot may be larger than the chunk + * size of some other snapshot. So we may need to reallocate multiple + * chunks in a snapshot. + * + * We don't do linking of pending exceptions and waiting for the last + * one --- that would complicate code too much and it would also be + * bug-prone. + * + * Instead, we try to scan all the overlapping exceptions in all + * non-merging snapshots and if something was reallocated then wait + * for any pending exception to complete. Retry after the wait, until + * all exceptions are done. + * + * This may seem ineffective, but in practice, people hardly use more + * than one or two snapshots. In case of two snapshots (one merging and + * one non-merging) with the same chunksize, wait and wakeup is done + * only once. + */ +static int origin_write_extent(struct dm_snapshot *merging_snap, + sector_t sector, chunk_t size) +{ + int r, must_wait = 0; + chunk_t n; + struct origin *o; + + /* 'size' must be a multiple of the 'merging_snap's chunk_size */ + BUG_ON(size % merging_snap->store->chunk_size); + /* + * Merging snapshot already has the origin's __minimum_chunk_size() + * stored in split_io (see: snapshot_merge_resume); avoid rediscovery + */ + BUG_ON(!merging_snap->ti->split_io); + + down_read(&_origins_lock); + o = __lookup_origin(merging_snap->origin->bdev); + for (n = 0; n < size; n += merging_snap->ti->split_io) { + r = __origin_write(&o->snapshots, sector + n, NULL); + if (r == DM_MAPIO_SUBMITTED) + must_wait = 1; + } + up_read(&_origins_lock); + + return must_wait; +} + +/* * Origin: maps a linear range of a device, with hooks for snapshotting. */