From b970c5eb6938666505ba3eca093607ad3dec40ff Mon Sep 17 00:00:00 2001 From: delphij Date: Wed, 13 May 2015 22:52:35 +0000 Subject: [PATCH] Fix bug with freebsd-update(8) that does not ensure the previous upgrade was completed. [EN-15:04] Fix deadlock on reboot with UFS tuned with SU+J. [EN-15:05] Approved by: so git-svn-id: svn://svn.freebsd.org/base/releng/10.1@282873 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- UPDATING | 8 ++ sys/conf/newvers.sh | 2 +- sys/ufs/ffs/ffs_softdep.c | 118 +++++++++++++++------- sys/ufs/ffs/ffs_vfsops.c | 16 ++- sys/ufs/ffs/softdep.h | 2 + usr.sbin/freebsd-update/freebsd-update.8 | 29 ++++-- usr.sbin/freebsd-update/freebsd-update.sh | 25 ++++- 7 files changed, 150 insertions(+), 50 deletions(-) diff --git a/UPDATING b/UPDATING index b7befecb6..cea7e638c 100644 --- a/UPDATING +++ b/UPDATING @@ -16,6 +16,14 @@ from older versions of FreeBSD, try WITHOUT_CLANG to bootstrap to the tip of stable/10, and then rebuild without this option. The bootstrap process from older version of current is a bit fragile. +20150513: p10 FreeBSD-EN-15:04.freebsd-update + FreeBSD-EN-15:05.ufs + + Fix bug with freebsd-update(8) that does not ensure the previous + upgrade was completed. [EN-15:04] + + Fix deadlock on reboot with UFS tuned with SU+J. [EN-15:05] + 20150407: p9 FreeBSD-SA-15:04.igmp [revised] FreeBSD-SA-15:07.ntp FreeBSD-SA-15:08.bsdinstall diff --git a/sys/conf/newvers.sh b/sys/conf/newvers.sh index 8321ae105..d37108486 100644 --- a/sys/conf/newvers.sh +++ b/sys/conf/newvers.sh @@ -32,7 +32,7 @@ TYPE="FreeBSD" REVISION="10.1" -BRANCH="RELEASE-p9" +BRANCH="RELEASE-p10" if [ "X${BRANCH_OVERRIDE}" != "X" ]; then BRANCH=${BRANCH_OVERRIDE} fi diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c index f958f5cba..2ce5caca0 100644 --- a/sys/ufs/ffs/ffs_softdep.c +++ b/sys/ufs/ffs/ffs_softdep.c @@ -735,9 +735,10 @@ static struct malloc_type *memtype[] = { static void check_clear_deps(struct mount *); static void softdep_error(char *, int); static int softdep_process_worklist(struct mount *, int); -static int softdep_waitidle(struct mount *); +static int softdep_waitidle(struct mount *, int); static void drain_output(struct vnode *); static struct buf *getdirtybuf(struct buf *, struct rwlock *, int); +static int check_inodedep_free(struct inodedep *); static void clear_remove(struct mount *); static void clear_inodedeps(struct mount *); static void unlinked_inodedep(struct mount *, struct inodedep *); @@ -1377,6 +1378,10 @@ softdep_flush(addr) mp = (struct mount *)addr; ump = VFSTOUFS(mp); atomic_add_int(&stat_flush_threads, 1); + ACQUIRE_LOCK(ump); + ump->softdep_flags &= ~FLUSH_STARTING; + wakeup(&ump->softdep_flushtd); + FREE_LOCK(ump); if (print_threads) { if (stat_flush_threads == 1) printf("Running %s at pid %d\n", bufdaemonproc->p_comm, @@ -1389,7 +1394,7 @@ softdep_flush(addr) VFSTOUFS(mp)->softdep_jblocks->jb_suspended)) kthread_suspend_check(); ACQUIRE_LOCK(ump); - if ((ump->softdep_flags & FLUSH_CLEANUP) == 0) + if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) msleep(&ump->softdep_flushtd, LOCK_PTR(ump), PVM, "sdflush", hz / 2); ump->softdep_flags &= ~FLUSH_CLEANUP; @@ -1419,11 +1424,9 @@ worklist_speedup(mp) ump = VFSTOUFS(mp); LOCK_OWNED(ump); - if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) { + if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) ump->softdep_flags |= FLUSH_CLEANUP; - if (ump->softdep_flushtd->td_wchan == &ump->softdep_flushtd) - wakeup(&ump->softdep_flushtd); - } + wakeup(&ump->softdep_flushtd); } static int @@ -1468,14 +1471,10 @@ softdep_speedup(ump) TAILQ_INSERT_TAIL(&softdepmounts, sdp, sd_next); FREE_GBLLOCK(&lk); if ((altump->softdep_flags & - (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) { + (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) altump->softdep_flags |= FLUSH_CLEANUP; - altump->um_softdep->sd_cleanups++; - if (altump->softdep_flushtd->td_wchan == - &altump->softdep_flushtd) { - wakeup(&altump->softdep_flushtd); - } - } + altump->um_softdep->sd_cleanups++; + wakeup(&altump->softdep_flushtd); FREE_LOCK(altump); } } @@ -1887,8 +1886,8 @@ softdep_flushworklist(oldmnt, countp, td) struct thread *td; { struct vnode *devvp; - int count, error = 0; struct ufsmount *ump; + int count, error; /* * Alternately flush the block device associated with the mount @@ -1897,6 +1896,7 @@ softdep_flushworklist(oldmnt, countp, td) * are found. */ *countp = 0; + error = 0; ump = VFSTOUFS(oldmnt); devvp = ump->um_devvp; while ((count = softdep_process_worklist(oldmnt, 1)) > 0) { @@ -1904,36 +1904,47 @@ softdep_flushworklist(oldmnt, countp, td) vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); error = VOP_FSYNC(devvp, MNT_WAIT, td); VOP_UNLOCK(devvp, 0); - if (error) + if (error != 0) break; } return (error); } +#define SU_WAITIDLE_RETRIES 20 static int -softdep_waitidle(struct mount *mp) +softdep_waitidle(struct mount *mp, int flags __unused) { struct ufsmount *ump; - int error; - int i; + struct vnode *devvp; + struct thread *td; + int error, i; ump = VFSTOUFS(mp); + devvp = ump->um_devvp; + td = curthread; + error = 0; ACQUIRE_LOCK(ump); - for (i = 0; i < 10 && ump->softdep_deps; i++) { + for (i = 0; i < SU_WAITIDLE_RETRIES && ump->softdep_deps != 0; i++) { ump->softdep_req = 1; - if (ump->softdep_on_worklist) - panic("softdep_waitidle: work added after flush."); - msleep(&ump->softdep_deps, LOCK_PTR(ump), PVM, "softdeps", 1); + KASSERT((flags & FORCECLOSE) == 0 || + ump->softdep_on_worklist == 0, + ("softdep_waitidle: work added after flush")); + msleep(&ump->softdep_deps, LOCK_PTR(ump), PVM | PDROP, + "softdeps", 10 * hz); + vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); + error = VOP_FSYNC(devvp, MNT_WAIT, td); + VOP_UNLOCK(devvp, 0); + if (error != 0) + break; + ACQUIRE_LOCK(ump); } ump->softdep_req = 0; - FREE_LOCK(ump); - error = 0; - if (i == 10) { + if (i == SU_WAITIDLE_RETRIES && error == 0 && ump->softdep_deps != 0) { error = EBUSY; printf("softdep_waitidle: Failed to flush worklist for %p\n", mp); } - + FREE_LOCK(ump); return (error); } @@ -1990,7 +2001,7 @@ retry_flush: error = EBUSY; } if (!error) - error = softdep_waitidle(oldmnt); + error = softdep_waitidle(oldmnt, flags); if (!error) { if (oldmnt->mnt_kern_flag & MNTK_UNMOUNT) { retry = 0; @@ -2490,9 +2501,18 @@ softdep_mount(devvp, mp, fs, cred) /* * Start our flushing thread in the bufdaemon process. */ + ACQUIRE_LOCK(ump); + ump->softdep_flags |= FLUSH_STARTING; + FREE_LOCK(ump); kproc_kthread_add(&softdep_flush, mp, &bufdaemonproc, &ump->softdep_flushtd, 0, 0, "softdepflush", "%s worker", mp->mnt_stat.f_mntonname); + ACQUIRE_LOCK(ump); + while ((ump->softdep_flags & FLUSH_STARTING) != 0) { + msleep(&ump->softdep_flushtd, LOCK_PTR(ump), PVM, "sdstart", + hz / 2); + } + FREE_LOCK(ump); /* * When doing soft updates, the counters in the * superblock may have gotten out of sync. Recomputation @@ -7629,17 +7649,13 @@ check_inode_unwritten(inodedep) return (1); } -/* - * Try to free an inodedep structure. Return 1 if it could be freed. - */ static int -free_inodedep(inodedep) +check_inodedep_free(inodedep) struct inodedep *inodedep; { LOCK_OWNED(VFSTOUFS(inodedep->id_list.wk_mp)); - if ((inodedep->id_state & (ONWORKLIST | UNLINKED)) != 0 || - (inodedep->id_state & ALLCOMPLETE) != ALLCOMPLETE || + if ((inodedep->id_state & ALLCOMPLETE) != ALLCOMPLETE || !LIST_EMPTY(&inodedep->id_dirremhd) || !LIST_EMPTY(&inodedep->id_pendinghd) || !LIST_EMPTY(&inodedep->id_bufwait) || @@ -7654,6 +7670,21 @@ free_inodedep(inodedep) inodedep->id_nlinkdelta != 0 || inodedep->id_savedino1 != NULL) return (0); + return (1); +} + +/* + * Try to free an inodedep structure. Return 1 if it could be freed. + */ +static int +free_inodedep(inodedep) + struct inodedep *inodedep; +{ + + LOCK_OWNED(VFSTOUFS(inodedep->id_list.wk_mp)); + if ((inodedep->id_state & (ONWORKLIST | UNLINKED)) != 0 || + !check_inodedep_free(inodedep)) + return (0); if (inodedep->id_state & ONDEPLIST) LIST_REMOVE(inodedep, id_deps); LIST_REMOVE(inodedep, id_hash); @@ -13838,7 +13869,8 @@ softdep_check_suspend(struct mount *mp, { struct bufobj *bo; struct ufsmount *ump; - int error; + struct inodedep *inodedep; + int error, unlinked; bo = &devvp->v_bufobj; ASSERT_BO_WLOCKED(bo); @@ -13899,6 +13931,20 @@ softdep_check_suspend(struct mount *mp, break; } + unlinked = 0; + if (MOUNTEDSUJ(mp)) { + for (inodedep = TAILQ_FIRST(&ump->softdep_unlinked); + inodedep != NULL; + inodedep = TAILQ_NEXT(inodedep, id_unlinked)) { + if ((inodedep->id_state & (UNLINKED | UNLINKLINKS | + UNLINKONLIST)) != (UNLINKED | UNLINKLINKS | + UNLINKONLIST) || + !check_inodedep_free(inodedep)) + continue; + unlinked++; + } + } + /* * Reasons for needing more work before suspend: * - Dirty buffers on devvp. @@ -13908,8 +13954,8 @@ softdep_check_suspend(struct mount *mp, error = 0; if (bo->bo_numoutput > 0 || bo->bo_dirty.bv_cnt > 0 || - softdep_depcnt != 0 || - ump->softdep_deps != 0 || + softdep_depcnt != unlinked || + ump->softdep_deps != unlinked || softdep_accdepcnt != ump->softdep_accdeps || secondary_writes != 0 || mp->mnt_secondary_writes != 0 || diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 41e29c02f..881da62f8 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -1502,8 +1502,11 @@ ffs_sync(mp, waitfor) if (fs->fs_fmod != 0 && fs->fs_ronly != 0 && ump->um_fsckpid == 0) panic("%s: ffs_sync: modification on read-only filesystem", fs->fs_fsmnt); - if (waitfor == MNT_LAZY) - return (ffs_sync_lazy(mp)); + if (waitfor == MNT_LAZY) { + if (!rebooting) + return (ffs_sync_lazy(mp)); + waitfor = MNT_NOWAIT; + } /* * Write back each (modified) inode. @@ -1560,7 +1563,7 @@ loop: /* * Force stale filesystem control information to be flushed. */ - if (waitfor == MNT_WAIT) { + if (waitfor == MNT_WAIT || rebooting) { if ((error = softdep_flushworklist(ump->um_mountp, &count, td))) allerror = error; /* Flushed work items may create new vnodes to clean */ @@ -1577,9 +1580,12 @@ loop: if (bo->bo_numoutput > 0 || bo->bo_dirty.bv_cnt > 0) { BO_UNLOCK(bo); vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - if ((error = VOP_FSYNC(devvp, waitfor, td)) != 0) - allerror = error; + error = VOP_FSYNC(devvp, waitfor, td); VOP_UNLOCK(devvp, 0); + if (MOUNTEDSOFTDEP(mp) && (error == 0 || error == EAGAIN)) + error = ffs_sbupdate(ump, waitfor, 0); + if (error != 0) + allerror = error; if (allerror == 0 && waitfor == MNT_WAIT) goto loop; } else if (suspend != 0) { diff --git a/sys/ufs/ffs/softdep.h b/sys/ufs/ffs/softdep.h index a17e4ea55..ce9327935 100644 --- a/sys/ufs/ffs/softdep.h +++ b/sys/ufs/ffs/softdep.h @@ -1063,6 +1063,8 @@ struct mount_softdeps { */ #define FLUSH_EXIT 0x0001 /* time to exit */ #define FLUSH_CLEANUP 0x0002 /* need to clear out softdep structures */ +#define FLUSH_STARTING 0x0004 /* flush thread not yet started */ + /* * Keep the old names from when these were in the ufsmount structure. */ diff --git a/usr.sbin/freebsd-update/freebsd-update.8 b/usr.sbin/freebsd-update/freebsd-update.8 index f372587ee..02b9b008a 100644 --- a/usr.sbin/freebsd-update/freebsd-update.8 +++ b/usr.sbin/freebsd-update/freebsd-update.8 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd July 14, 2010 +.Dd March 2, 2015 .Dt FREEBSD-UPDATE 8 .Os FreeBSD .Sh NAME @@ -36,10 +36,12 @@ .Op Fl b Ar basedir .Op Fl d Ar workdir .Op Fl f Ar conffile +.Op Fl F .Op Fl k Ar KEY .Op Fl r Ar newrelease .Op Fl s Ar server .Op Fl t Ar address +.Op Fl -not-running-from-cron .Cm command ... .Sh DESCRIPTION The @@ -54,16 +56,16 @@ by the .Fx Release Engineering Team, e.g., .Fx -7.3-RELEASE and +9.3-RELEASE and .Fx -8.0-RELEASE, but not +10.1-RELEASE, but not .Fx -6.3-STABLE or +9.3-STABLE or .Fx -9.0-CURRENT. +11-CURRENT. .Sh OPTIONS The following options are supported: -.Bl -tag -width "-f conffile" +.Bl -tag -width "-r newrelease" .It Fl b Ar basedir Operate on a system mounted at .Ar basedir . @@ -81,6 +83,10 @@ Read configuration options from .Ar conffile . (default: .Pa /etc/freebsd-update.conf ) +.It Fl F +Force +.Nm Cm fetch +to proceed where it normally would not, such as an unfinished upgrade .It Fl k Ar KEY Trust an RSA key with SHA256 of .Ar KEY . @@ -98,12 +104,21 @@ Mail output of command, if any, to .Ar address . (default: root, or as given in the configuration file.) +.It Fl -not-running-from-cron +Force +.Nm Cm fetch +to proceed when there is no controlling tty. +This is for use by automated scripts and orchestration tools. +Please do not run +.Nm Cm fetch +from crontab or similar using this flag, see: +.Nm Cm cron .El .Sh COMMANDS The .Cm command can be any one of the following: -.Bl -tag -width "-f conffile" +.Bl -tag -width "rollback" .It Cm fetch Based on the currently installed world and the configuration options set, fetch all available binary updates. diff --git a/usr.sbin/freebsd-update/freebsd-update.sh b/usr.sbin/freebsd-update/freebsd-update.sh index 63cb14ced..bacdfa720 100644 --- a/usr.sbin/freebsd-update/freebsd-update.sh +++ b/usr.sbin/freebsd-update/freebsd-update.sh @@ -43,12 +43,15 @@ Options: (default: /var/db/freebsd-update/) -f conffile -- Read configuration options from conffile (default: /etc/freebsd-update.conf) + -F -- Force a fetch operation to proceed -k KEY -- Trust an RSA key with SHA256 hash of KEY -r release -- Target for upgrade (e.g., 6.2-RELEASE) -s server -- Server from which to fetch updates (default: update.FreeBSD.org) -t address -- Mail output of cron command, if any, to address (default: root) + --not-running-from-cron + -- Run without a tty, for use by automated tools Commands: fetch -- Fetch updates from server cron -- Sleep rand(3600) seconds, fetch updates, and send an @@ -399,6 +402,12 @@ init_params () { # No commands specified yet COMMANDS="" + + # Force fetch to proceed + FORCEFETCH=0 + + # Run without a TTY + NOTTYOK=0 } # Parse the command line @@ -411,6 +420,12 @@ parse_cmdline () { if [ ! -z "${CONFFILE}" ]; then usage; fi shift; CONFFILE="$1" ;; + -F) + FORCEFETCH=1 + ;; + --not-running-from-cron) + NOTTYOK=1 + ;; # Configuration file equivalents -b) @@ -665,6 +680,14 @@ fetch_check_params () { echo "(Did you mean 'upgrade' instead?)" exit 1 fi + + # Check that we have updates ready to install + if [ -f ${BDHASH}-install/kerneldone -a $FORCEFETCH -eq 0 ]; then + echo "You have a partially completed upgrade pending" + echo "Run '$0 install' first." + echo "Run '$0 fetch -F' to proceed anyway." + exit 1 + fi } # Perform sanity checks etc. before fetching upgrades. @@ -3202,7 +3225,7 @@ get_params () { # Fetch command. Make sure that we're being called # interactively, then run fetch_check_params and fetch_run cmd_fetch () { - if [ ! -t 0 ]; then + if [ ! -t 0 -a $NOTTYOK -eq 0 ]; then echo -n "`basename $0` fetch should not " echo "be run non-interactively." echo "Run `basename $0` cron instead." -- 2.42.0