DF-2763 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 | --- a/sys/kern/vfs_jops.c +++ b/sys/kern/vfs_jops.c @@ -159,6 +159,14 @@ mp = ap->a_head.a_ops->head.vv_mount; KKASSERT(mp); + /* + * Serialize the journal lifecycle (attach/detach + mnt_jlist + * install/remove) against concurrent mountctl operations and + * against dounmount(), which holds mnt_token across + * journal_remove_all_journals(). + */ + lwkt_gettoken(&mp->mnt_token); + if (mp->mnt_vn_journal_ops == NULL) { switch(ap->a_op) { case MOUNTCTL_INSTALL_VFS_JOURNAL: @@ -226,6 +234,7 @@ break; } } + lwkt_reltoken(&mp->mnt_token); return (error); } @@ -245,7 +254,13 @@ static void journal_detach(struct mount *mp) { - KKASSERT(mp->mnt_jbitmap != NULL); + /* + * Idempotent: concurrent removes (or a remove straddling a sleep in + * journal_destroy(), which drops mnt_token) can race us to the + * teardown. If the bitmap is already gone so is the ops vector. + */ + if (mp->mnt_jbitmap == NULL) + return; if (mp->mnt_vn_journal_ops) vfs_rm_vnodeops(mp, &journal_vnode_vops, &mp->mnt_vn_journal_ops); kfree(mp->mnt_jbitmap, M_JOURNAL); @@ -427,6 +442,8 @@ while ((jo = TAILQ_FIRST(&mp->mnt_jlist)) != NULL) { journal_destroy(mp, jo, flags); } + if (mp->mnt_vn_journal_ops) + journal_detach(mp); } static int @@ -542,6 +559,12 @@ * also allows someone observing the raw records to clearly see * when parallel transactions occur. */ + /* + * Serialize the streamid allocation against other jreclist's and + * against journal_detach() freeing mnt_jbitmap. This section does + * not block, so the token is held throughout. + */ + lwkt_gettoken(&mp->mnt_token); streamid = mp->mnt_streamid; count = 0; while (mp->mnt_jbitmap[streamid >> 3] & (1 << (streamid & 7))) { @@ -549,13 +572,16 @@ streamid = JREC_STREAMID_JMIN; if (++count == JREC_STREAMID_JMAX - JREC_STREAMID_JMIN) { kprintf("jreclist_init: all streamid's in use! sleeping\n"); + lwkt_reltoken(&mp->mnt_token); tsleep(jreclist, 0, "jsidfl", hz * 10); + lwkt_gettoken(&mp->mnt_token); count = 0; } } mp->mnt_jbitmap[streamid >> 3] |= 1 << (streamid & 7); mp->mnt_streamid = streamid; jreclist->streamid = streamid; + lwkt_reltoken(&mp->mnt_token); /* * Now initialize a stream on each journal. @@ -608,9 +634,13 @@ } /* - * Clear the streamid so it can be reused. + * Clear the streamid so it can be reused. Serialized against + * journal_detach() freeing mnt_jbitmap. */ - mp->mnt_jbitmap[jreclist->streamid >> 3] &= ~(1 << (jreclist->streamid & 7)); + lwkt_gettoken(&mp->mnt_token); + if (mp->mnt_jbitmap) + mp->mnt_jbitmap[jreclist->streamid >> 3] &= ~(1 << (jreclist->streamid & 7)); + lwkt_reltoken(&mp->mnt_token); } /* @@ -1315,11 +1345,18 @@ jreclist_undo_file(&jreclist, ap->a_tnch->ncp->nc_vp, JRUNDO_ALL|JRUNDO_GETVP|JRUNDO_CONDLINK, 0, -1); } + /* + * PATH1 must be captured BEFORE the underlying rename: cache_rename() + * relinks fnch->ncp to the target name in-place, so a post-op walk + * records the target path twice and the source path is lost. + */ + TAILQ_FOREACH(jrec, &jreclist.list, user_entry) { + jrecord_write_path(jrec, JLEAF_PATH1, ap->a_fnch->ncp); + } error = vop_journal_operate_ap(&ap->a_head); if (error == 0) { TAILQ_FOREACH(jrec, &jreclist.list, user_entry) { jrecord_write_cred(jrec, NULL, ap->a_cred); - jrecord_write_path(jrec, JLEAF_PATH1, ap->a_fnch->ncp); jrecord_write_path(jrec, JLEAF_PATH2, ap->a_tnch->ncp); } } |