From b58c6388a61db3df6185f0527dfe6bff829997bd Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Mon, 24 Mar 2008 23:50:23 +0000 Subject: [PATCH] HAMMER 35/many: Stabilization pass, cleanups * Fix a buffer load race which could result in an assertion or panic related to a referenced HAMMER buffer with a NULL bp. The problem was that the loading flag must be used when releasing the buffer as well as when acquiring the buffer. Change the loading flag to a loading count. * Do not lose flush requests. The flush request now stays flagged until the buffer is able to be flushed. * Fix stale blockmap offsets cached in hammer_buffer. Clear the cached offset when freeing a big block from the blockmap. NOTE: We do not yet try to index buffers based on the blockmap offset but we should. * Remove the old write ordering code in preparation for redoing the algorithm. * General code cleanups. --- sys/vfs/hammer/hammer.h | 14 ++--- sys/vfs/hammer/hammer_freemap.c | 3 +- sys/vfs/hammer/hammer_io.c | 78 +++++++------------------- sys/vfs/hammer/hammer_ondisk.c | 98 +++++++++++++++++++++------------ sys/vfs/hammer/hammer_reblock.c | 14 +++-- sys/vfs/hammer/hammer_undo.c | 7 +-- 6 files changed, 104 insertions(+), 110 deletions(-) diff --git a/sys/vfs/hammer/hammer.h b/sys/vfs/hammer/hammer.h index bf8e3c640a..e0897dc17f 100644 --- a/sys/vfs/hammer/hammer.h +++ b/sys/vfs/hammer/hammer.h @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer.h,v 1.42 2008/03/20 06:08:40 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer.h,v 1.43 2008/03/24 23:50:23 dillon Exp $ */ /* * This header file contains structures used internally by the HAMMERFS @@ -261,7 +261,7 @@ struct worklist { LIST_ENTRY(worklist) node; }; -TAILQ_HEAD(hammer_io_list, hammer_io); +/*TAILQ_HEAD(hammer_dep_list, hammer_dep);*/ struct hammer_io { struct worklist worklist; @@ -269,15 +269,14 @@ struct hammer_io { enum hammer_io_type type; struct buf *bp; int64_t offset; - TAILQ_ENTRY(hammer_io) entry; /* based on modified flag */ - struct hammer_io_list *entry_list; - struct hammer_io_list deplist; + int loading; /* loading/unloading interlock */ u_int modified : 1; /* bp's data was modified */ u_int released : 1; /* bp released (w/ B_LOCKED set) */ u_int running : 1; /* bp write IO in progress */ u_int waiting : 1; /* someone is waiting on us */ - u_int loading : 1; /* ondisk is loading */ u_int validated : 1; /* ondisk has been validated */ + u_int flush : 1; /* flush on last release */ + u_int waitdep : 1; /* flush waits for dependancies */ }; typedef struct hammer_io *hammer_io_t; @@ -552,6 +551,7 @@ hammer_volume_t hammer_get_volume(hammer_mount_t hmp, int32_t vol_no, int *errorp); hammer_buffer_t hammer_get_buffer(hammer_mount_t hmp, hammer_off_t buf_offset, int isnew, int *errorp); +void hammer_uncache_buffer(struct hammer_mount *hmp, hammer_off_t off); int hammer_ref_volume(hammer_volume_t volume); int hammer_ref_buffer(hammer_buffer_t buffer); @@ -648,7 +648,7 @@ int hammer_ioctl(hammer_inode_t ip, u_long com, caddr_t data, int fflag, void hammer_io_init(hammer_io_t io, enum hammer_io_type type); int hammer_io_read(struct vnode *devvp, struct hammer_io *io); int hammer_io_new(struct vnode *devvp, struct hammer_io *io); -void hammer_io_release(struct hammer_io *io, int flush); +void hammer_io_release(struct hammer_io *io); void hammer_io_flush(struct hammer_io *io); int hammer_io_checkflush(hammer_io_t io); void hammer_io_clear_modify(struct hammer_io *io); diff --git a/sys/vfs/hammer/hammer_freemap.c b/sys/vfs/hammer/hammer_freemap.c index 12864376bb..3df96b34a5 100644 --- a/sys/vfs/hammer/hammer_freemap.c +++ b/sys/vfs/hammer/hammer_freemap.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer_freemap.c,v 1.4 2008/03/19 20:18:17 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer_freemap.c,v 1.5 2008/03/24 23:50:23 dillon Exp $ */ /* @@ -151,6 +151,7 @@ hammer_freemap_free(hammer_transaction_t trans, hammer_off_t phys_offset, kprintf("hammer_freemap_free %016llx\n", phys_offset); + hammer_uncache_buffer(trans->hmp, phys_offset); *errorp = 0; ondisk = trans->rootvol->ondisk; diff --git a/sys/vfs/hammer/hammer_io.c b/sys/vfs/hammer/hammer_io.c index 52942e01e5..4b073ea801 100644 --- a/sys/vfs/hammer/hammer_io.c +++ b/sys/vfs/hammer/hammer_io.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer_io.c,v 1.22 2008/03/19 20:18:17 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer_io.c,v 1.23 2008/03/24 23:50:23 dillon Exp $ */ /* * IO Primitives and buffer cache management @@ -62,7 +62,6 @@ void hammer_io_init(hammer_io_t io, enum hammer_io_type type) { io->type = type; - TAILQ_INIT(&io->deplist); } /* @@ -80,7 +79,7 @@ hammer_io_disassociate(hammer_io_structure_t iou, int elseit) { struct buf *bp = iou->io.bp; - KKASSERT(TAILQ_EMPTY(&iou->io.deplist) && iou->io.modified == 0); + KKASSERT(iou->io.modified == 0); buf_dep_init(bp); iou->io.bp = NULL; bp->b_flags &= ~B_LOCKED; @@ -125,15 +124,6 @@ hammer_io_wait(hammer_io_t io) } } -void -hammer_io_waitdep(hammer_io_t io) -{ - while (TAILQ_FIRST(&io->deplist)) { - kprintf("waitdep %p\n", io); - tsleep(io, 0, "hmrdep", hz); - } -} - /* * Load bp for a HAMMER structure. The io is exclusively locked by the * caller. @@ -209,7 +199,7 @@ hammer_io_new(struct vnode *devvp, struct hammer_io *io) * are still other references. */ void -hammer_io_release(struct hammer_io *io, int flush) +hammer_io_release(struct hammer_io *io) { struct buf *bp; @@ -220,7 +210,7 @@ hammer_io_release(struct hammer_io *io, int flush) /* * If flush is 2 wait for dependancies */ - while (flush == 2 && TAILQ_FIRST(&io->deplist)) { + while (io->waitdep && TAILQ_FIRST(&io->deplist)) { hammer_io_wait(TAILQ_FIRST(&io->deplist)); } #endif @@ -231,13 +221,13 @@ hammer_io_release(struct hammer_io *io, int flush) * * The flush will fail if any dependancies are present. */ - if (io->modified && (flush || bp->b_flags & B_LOCKED)) + if (io->modified && (io->flush || (bp->b_flags & B_LOCKED))) hammer_io_flush(io); /* * If flush is 2 we wait for the IO to complete. */ - if (flush == 2 && io->running) { + if (io->waitdep && io->running) { hammer_io_wait(io); } @@ -245,8 +235,7 @@ hammer_io_release(struct hammer_io *io, int flush) * Actively or passively release the buffer. Modified IOs with * dependancies cannot be released. */ - if (flush && io->modified == 0 && io->running == 0) { - KKASSERT(TAILQ_EMPTY(&io->deplist)); + if (io->flush && io->modified == 0 && io->running == 0) { if (io->released) { regetblk(bp); BUF_KERNPROC(bp); @@ -254,7 +243,7 @@ hammer_io_release(struct hammer_io *io, int flush) } hammer_io_disassociate((hammer_io_structure_t)io, 1); } else if (io->modified) { - if (io->released == 0 && TAILQ_EMPTY(&io->deplist)) { + if (io->released == 0) { io->released = 1; bdwrite(bp); } @@ -278,10 +267,10 @@ hammer_io_flush(struct hammer_io *io) /* * Can't flush if the IO isn't modified or if it has dependancies. */ - if (io->modified == 0) - return; - if (TAILQ_FIRST(&io->deplist)) + if (io->modified == 0) { + io->flush = 0; return; + } KKASSERT(io->bp); @@ -304,6 +293,7 @@ hammer_io_flush(struct hammer_io *io) * ondisk while we are blocked blocks waiting for us. */ io->modified = 0; /* force interlock */ + io->flush = 0; bp = io->bp; if (io->released) { @@ -331,24 +321,17 @@ hammer_io_flush(struct hammer_io *io) /* * Mark a HAMMER structure as undergoing modification. Return 1 when applying * a non-NULL ordering dependancy for the first time, 0 otherwise. - * - * list can be NULL, indicating that a structural modification is being made - * without creating an ordering dependancy. */ static __inline -int -hammer_io_modify(hammer_io_t io, struct hammer_io_list *list) +void +hammer_io_modify(hammer_io_t io) { - int r; - /* * Shortcut if nothing to do. */ KKASSERT(io->lock.refs != 0 && io->bp != NULL); - if (io->modified && io->released == 0 && - (io->entry_list || list == NULL)) { - return(0); - } + if (io->modified && io->released == 0) + return; hammer_lock_ex(&io->lock); io->modified = 1; @@ -358,28 +341,14 @@ hammer_io_modify(hammer_io_t io, struct hammer_io_list *list) io->released = 0; KKASSERT(io->modified != 0); } - if (io->entry_list == NULL) { - io->entry_list = list; - if (list) { - TAILQ_INSERT_TAIL(list, io, entry); - r = 1; - } else { - r = 0; - } - } else { - /* only one dependancy is allowed */ - KKASSERT(list == NULL || io->entry_list == list); - r = 0; - } hammer_unlock(&io->lock); - return(r); } void hammer_modify_volume(hammer_transaction_t trans, hammer_volume_t volume, void *base, int len) { - hammer_io_modify(&volume->io, NULL); + hammer_io_modify(&volume->io); if (len) { intptr_t rel_offset = (intptr_t)base - (intptr_t)volume->ondisk; @@ -399,7 +368,7 @@ void hammer_modify_buffer(hammer_transaction_t trans, hammer_buffer_t buffer, void *base, int len) { - hammer_io_modify(&buffer->io, NULL); + hammer_io_modify(&buffer->io); if (len) { intptr_t rel_offset = (intptr_t)base - (intptr_t)buffer->ondisk; KKASSERT((rel_offset & ~(intptr_t)HAMMER_BUFMASK) == 0); @@ -468,18 +437,13 @@ hammer_io_complete(struct buf *bp) KKASSERT(iou->io.released == 1); + /* XXX DEP REMOVE */ + /* - * If this was a write and the modified bit is still clear we can - * remove ourselves from the dependancy list. - * * If no lock references remain and we can acquire the IO lock and * someone at some point wanted us to flush (B_LOCKED test), then * try to dispose of the IO. */ - if (iou->io.modified == 0 && iou->io.entry_list) { - TAILQ_REMOVE(iou->io.entry_list, &iou->io, entry); - iou->io.entry_list = NULL; - } iou->io.running = 0; if (iou->io.waiting) { iou->io.waiting = 0; @@ -581,8 +545,6 @@ hammer_io_checkwrite(struct buf *bp) { union hammer_io_structure *iou = (void *)LIST_FIRST(&bp->b_dep); - KKASSERT(TAILQ_EMPTY(&iou->io.deplist)); - /* * We are called from the kernel on delayed-write buffers, and * called from hammer_io_flush() on flush requests. There should diff --git a/sys/vfs/hammer/hammer_ondisk.c b/sys/vfs/hammer/hammer_ondisk.c index ebc5f81fc7..5a08da9136 100644 --- a/sys/vfs/hammer/hammer_ondisk.c +++ b/sys/vfs/hammer/hammer_ondisk.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer_ondisk.c,v 1.34 2008/03/19 20:18:17 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer_ondisk.c,v 1.35 2008/03/24 23:50:23 dillon Exp $ */ /* * Manage HAMMER's on-disk structures. These routines are primarily @@ -291,12 +291,13 @@ hammer_unload_volume(hammer_volume_t volume, void *data __unused) */ RB_SCAN(hammer_buf_rb_tree, &volume->rb_bufs_root, NULL, hammer_unload_buffer, NULL); - hammer_io_waitdep(&volume->io); /* * Release our buffer and flush anything left in the buffer cache. */ - hammer_io_release(&volume->io, 2); + volume->io.flush = 1; + volume->io.waitdep = 1; + hammer_io_release(&volume->io); /* * There should be no references on the volume, no clusters, and @@ -430,27 +431,22 @@ hammer_get_root_volume(struct hammer_mount *hmp, int *errorp) static int hammer_load_volume(hammer_volume_t volume) { - struct hammer_volume_ondisk *ondisk; int error; hammer_lock_ex(&volume->io.lock); KKASSERT(volume->io.loading == 0); - volume->io.loading = 1; + ++volume->io.loading; if (volume->ondisk == NULL) { error = hammer_io_read(volume->devvp, &volume->io); - if (error) { - volume->io.loading = 0; - hammer_unlock(&volume->io.lock); - return (error); - } - volume->ondisk = ondisk = (void *)volume->io.bp->b_data; + if (error == 0) + volume->ondisk = (void *)volume->io.bp->b_data; } else { error = 0; } - volume->io.loading = 0; + --volume->io.loading; hammer_unlock(&volume->io.lock); - return(0); + return(error); } /* @@ -464,17 +460,21 @@ hammer_load_volume(hammer_volume_t volume) void hammer_rel_volume(hammer_volume_t volume, int flush) { + if (flush) + volume->io.flush = 1; + crit_enter(); if (volume->io.lock.refs == 1) { + ++volume->io.loading; hammer_lock_ex(&volume->io.lock); if (volume->io.lock.refs == 1) { volume->ondisk = NULL; - hammer_io_release(&volume->io, flush); - } else if (flush) { - hammer_io_flush(&volume->io); + hammer_io_release(&volume->io); } + --volume->io.loading; hammer_unlock(&volume->io.lock); } hammer_unref(&volume->io.lock); + crit_exit(); } /************************************************************************ @@ -575,16 +575,14 @@ static int hammer_load_buffer(hammer_buffer_t buffer, int isnew) { hammer_volume_t volume; - void *ondisk; int error; /* * Load the buffer's on-disk info */ volume = buffer->volume; + ++buffer->io.loading; hammer_lock_ex(&buffer->io.lock); - KKASSERT(buffer->io.loading == 0); - buffer->io.loading = 1; if (buffer->ondisk == NULL) { if (isnew) { @@ -592,12 +590,8 @@ hammer_load_buffer(hammer_buffer_t buffer, int isnew) } else { error = hammer_io_read(volume->devvp, &buffer->io); } - if (error) { - buffer->io.loading = 0; - hammer_unlock(&buffer->io.lock); - return (error); - } - buffer->ondisk = ondisk = (void *)buffer->io.bp->b_data; + if (error == 0) + buffer->ondisk = (void *)buffer->io.bp->b_data; } else if (isnew) { error = hammer_io_new(volume->devvp, &buffer->io); } else { @@ -607,7 +601,7 @@ hammer_load_buffer(hammer_buffer_t buffer, int isnew) hammer_modify_buffer(NULL, buffer, NULL, 0); /* additional initialization goes here */ } - buffer->io.loading = 0; + --buffer->io.loading; hammer_unlock(&buffer->io.lock); return (error); } @@ -663,31 +657,67 @@ void hammer_rel_buffer(hammer_buffer_t buffer, int flush) { hammer_volume_t volume; + int freeme = 0; + if (flush) + buffer->io.flush = 1; + crit_enter(); if (buffer->io.lock.refs == 1) { + ++buffer->io.loading; /* force interlock check */ hammer_lock_ex(&buffer->io.lock); if (buffer->io.lock.refs == 1) { - hammer_io_release(&buffer->io, flush); + hammer_io_release(&buffer->io); + hammer_flush_buffer_nodes(buffer); + KKASSERT(TAILQ_EMPTY(&buffer->clist)); if (buffer->io.bp == NULL && buffer->io.lock.refs == 1) { - hammer_flush_buffer_nodes(buffer); - KKASSERT(TAILQ_EMPTY(&buffer->clist)); + /* + * Final cleanup + */ volume = buffer->volume; RB_REMOVE(hammer_buf_rb_tree, &volume->rb_bufs_root, buffer); buffer->volume = NULL; /* sanity */ - --hammer_count_buffers; - kfree(buffer, M_HAMMER); hammer_rel_volume(volume, 0); - return; + freeme = 1; } - } else if (flush) { - hammer_io_flush(&buffer->io); } + --buffer->io.loading; hammer_unlock(&buffer->io.lock); } hammer_unref(&buffer->io.lock); + crit_exit(); + if (freeme) { + --hammer_count_buffers; + kfree(buffer, M_HAMMER); + } +} + +/* + * Remove the zoneX translation cache for a buffer given its zone-2 offset. + */ +void +hammer_uncache_buffer(hammer_mount_t hmp, hammer_off_t buf_offset) +{ + hammer_volume_t volume; + hammer_buffer_t buffer; + int vol_no; + int error; + + buf_offset &= ~HAMMER_BUFMASK64; + KKASSERT((buf_offset & HAMMER_ZONE_RAW_BUFFER) == + HAMMER_ZONE_RAW_BUFFER); + vol_no = HAMMER_VOL_DECODE(buf_offset); + volume = hammer_get_volume(hmp, vol_no, &error); + KKASSERT(volume != 0); + KKASSERT(buf_offset < volume->maxbuf_off); + + buffer = RB_LOOKUP(hammer_buf_rb_tree, &volume->rb_bufs_root, + buf_offset); + if (buffer) + buffer->zoneX_offset = 0; + hammer_rel_volume(volume, 0); } /* diff --git a/sys/vfs/hammer/hammer_reblock.c b/sys/vfs/hammer/hammer_reblock.c index 159b81187e..2f9fa6de68 100644 --- a/sys/vfs/hammer/hammer_reblock.c +++ b/sys/vfs/hammer/hammer_reblock.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer_reblock.c,v 1.4 2008/03/20 06:08:40 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer_reblock.c,v 1.5 2008/03/24 23:50:23 dillon Exp $ */ /* * HAMMER reblocker - This code frees up fragmented physical space @@ -306,8 +306,10 @@ hammer_reblock_record(struct hammer_ioc_reblock *reblock, hammer_blockmap_free(cursor->trans, elm->leaf.rec_offset, sizeof(*nrec)); - kprintf("REBLOCK RECD %016llx -> %016llx\n", - elm->leaf.rec_offset, nrec_offset); + if (hammer_debug_general & 0x4000) { + kprintf("REBLOCK RECD %016llx -> %016llx\n", + elm->leaf.rec_offset, nrec_offset); + } hammer_modify_node(cursor->trans, cursor->node, &elm->leaf.rec_offset, sizeof(hammer_off_t)); @@ -377,8 +379,10 @@ hammer_reblock_node(struct hammer_ioc_reblock *reblock, hammer_delete_node(cursor->trans, onode); - kprintf("REBLOCK NODE %016llx -> %016llx\n", - onode->node_offset, nnode->node_offset); + if (hammer_debug_general & 0x4000) { + kprintf("REBLOCK NODE %016llx -> %016llx\n", + onode->node_offset, nnode->node_offset); + } cursor->node = nnode; hammer_rel_node(onode); diff --git a/sys/vfs/hammer/hammer_undo.c b/sys/vfs/hammer/hammer_undo.c index 7256b89e8a..ca79243e55 100644 --- a/sys/vfs/hammer/hammer_undo.c +++ b/sys/vfs/hammer/hammer_undo.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/vfs/hammer/hammer_undo.c,v 1.2 2008/03/19 20:18:17 dillon Exp $ + * $DragonFly: src/sys/vfs/hammer/hammer_undo.c,v 1.3 2008/03/24 23:50:23 dillon Exp $ */ /* @@ -93,9 +93,7 @@ hammer_generate_undo(hammer_transaction_t trans, hammer_off_t zone1_off, bytes = ((len + 7) & ~7) + sizeof(struct hammer_fifo_undo) + sizeof(struct hammer_fifo_tail); - root_volume = hammer_get_root_volume(trans->hmp, &error); - if (error) - return(error); + root_volume = trans->rootvol; ondisk = root_volume->ondisk; undomap = &ondisk->vol0_blockmap[HAMMER_ZONE_UNDO_INDEX]; @@ -171,7 +169,6 @@ again: if (buffer) hammer_rel_buffer(buffer, 0); - hammer_rel_volume(root_volume, 0); return(error); } -- 2.41.0