Date: Sun, 29 Oct 2017 11:56:57 +0100 From: Stefan Esser <se@freebsd.org> To: Kirk McKusick <mckusick@mckusick.com>, Mark Johnston <markj@FreeBSD.org> Cc: freebsd-fs@FreeBSD.org Subject: [PATCH] Re: softdep as a mount(8) option Message-ID: <0bc1fba0-cda6-064a-add5-02f702e533f7@freebsd.org> In-Reply-To: <a19ed958-7b48-6c11-b945-d53dec8a77fa@freebsd.org> References: <201710282225.v9SMPDCZ074228@chez.mckusick.com> <a19ed958-7b48-6c11-b945-d53dec8a77fa@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------772410BE6C4FC62BABB50287 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Am 29.10.17 um 11:36 schrieb Stefan Esser: > Hi Kirk, > > I had implemented this feature more than 10 years ago and even got your > reply, that this was a better solution than the mount option, when I > discussed the implementation with you at that time. (That was shortly > after soft-updates made it into FreeBSD - a long time ago ...) > > But despite your agreement that I should commit that change (for the > next major FreeBSD release), it was later considered a violation of POLA > and I kept (and maintained it) on my system only. > > > I had answered the same question regarding FSCK more than 10 years ago > in the following archived message: > > https://lists.freebsd.org/pipermail/freebsd-hackers/2005-December/014865.html > > > My solution was very simple: The mount option sets the soft-updetes flag > in the super-block, if a file system is mounted RW with soft-updates > enabled. If it is not cleanly unmounted, then FSCK will correctly check > it even on a system that does not support soft-updates in the kernel > (but in FSCK, where it's not optional). > > When such file system is mounted on a system without my patch, it is > indistinguishable from a drive that had the soft-updates flag set via > tunefs. > > In fact, my patch made the kernel do the equivalent of tunefs -j on > each mount, depending on the mount option. The only special case is that > mount -u must not change the soft-updates state on a file-system that > is mounted RW (but tunefs cannot do this, too), since I did not want to > implement the required flushing of all buffers before switching the > mode. > > I have stopped using UFS (have switched to ZFS) a few years ago, but > have maintained my changes in my local version of -CURRENT to this date. > > (For that reason I had no interest in SU+J and there might be a need to > adapt the code to correctly work with SU+J ...) > > Regards, STefan > > PS: I have attached my patch to this mail, will be stripped by the > mail-list, but may be of interest to the direct recipients ... I noticed that patch missed one non-critical part, which allows to update the soft-updates state with mount -u on a R/O mounted file- system (e.g. / when booting in single-user mode). Seems that the patch made it to the list, so I'm attaching the full diff (now with the part missed in the previous version) again. Maybe it is of use to somebody ... Regards, STefan --------------772410BE6C4FC62BABB50287 Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0"; name="soft-updates-option.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="soft-updates-option.diff" Index: sys/ufs/ffs/ffs_vfsops.c =================================================================== --- sys/ufs/ffs/ffs_vfsops.c (Revision 325051) +++ sys/ufs/ffs/ffs_vfsops.c (Arbeitskopie) @@ -138,7 +138,7 @@ */ static const char *ffs_opts[] = { "acls", "async", "noatime", "noclusterr", "noclusterw", "noexec", "export", "force", "from", "groupquota", - "multilabel", "nfsv4acls", "fsckpid", "snapshot", "nosuid", "suiddir", + "multilabel", "nfsv4acls", "fsckpid", "snapshot", "softdep", "nosuid", "suiddir", "nosymfollow", "sync", "union", "userquota", NULL }; static int @@ -237,17 +237,26 @@ ump = VFSTOUFS(mp); fs = ump->um_fs; devvp = ump->um_devvp; if (fsckpid == -1 && ump->um_fsckpid > 0) { if ((error = ffs_flushfiles(mp, WRITECLOSE, td)) != 0 || (error = ffs_sbupdate(ump, MNT_WAIT, 0)) != 0) return (error); g_topology_lock(); + if (fs->fs_ronly == 0) { /* - * Return to normal read-only mode. + * Preserve current softdep status, + * unless mounted read-only. */ + MNT_ILOCK(mp); + if (fs->fs_flags & FS_DOSOFTDEP) { + mp->mnt_flag |= MNT_SOFTDEP; + } else { + mp->mnt_flag &= ~MNT_SOFTDEP; + } + MNT_IUNLOCK(mp); error = g_access(ump->um_cp, 0, -1, 0); g_topology_unlock(); ump->um_fsckpid = 0; } if (fs->fs_ronly == 0 && vfs_flagopt(mp->mnt_optnew, "ro", NULL, 0)) { @@ -266,7 +265,8 @@ flags = WRITECLOSE; if (mp->mnt_flag & MNT_FORCE) flags |= FORCECLOSE; - if (MOUNTEDSOFTDEP(mp)) { +// if (MOUNTEDSOFTDEP(mp)) { + if (fs->fs_flags & FS_DOSOFTDEP) { error = softdep_flushfiles(mp, flags, td); } else { error = ffs_flushfiles(mp, flags, td); @@ -346,6 +346,15 @@ (fs->fs_flags & FS_DOSOFTDEP))) { printf("WARNING: %s was not properly " "dismounted\n", fs->fs_fsmnt); + /* + * Preserve softdep state if unclean + */ + MNT_ILOCK(mp); + if ((fs->fs_flags & FS_DOSOFTDEP) == 0) + mp->mnt_flag &= ~MNT_SOFTDEP; + else + mp->mnt_flag |= MNT_SOFTDEP; + MNT_IUNLOCK(mp); } else { vfs_mount_error(mp, "R/W mount of %s denied. %s.%s", @@ -373,7 +380,7 @@ MNT_IUNLOCK(mp); fs->fs_mtime = time_second; /* check to see if we need to start softdep */ - if ((fs->fs_flags & FS_DOSOFTDEP) && + if ((mp->mnt_flag & MNT_SOFTDEP) && (error = softdep_mount(devvp, mp, fs, td->td_ucred))){ vn_finished_write(mp); return (error); @@ -851,6 +858,15 @@ (fs->fs_flags & FS_DOSOFTDEP))) { printf("WARNING: %s was not properly dismounted\n", fs->fs_fsmnt); + /* + * Preserve softdep state if unclean. + */ + MNT_ILOCK(mp); + if ((fs->fs_flags & FS_DOSOFTDEP) == 0) + mp->mnt_flag &= ~MNT_SOFTDEP; + else + mp->mnt_flag |= MNT_SOFTDEP; + MNT_IUNLOCK(mp); } else { vfs_mount_error(mp, "R/W mount of %s denied. %s%s", fs->fs_fsmnt, "Filesystem is not clean - run fsck.", @@ -875,6 +889,11 @@ fs->fs_pendingblocks = 0; fs->fs_pendinginodes = 0; } + /* Clear softdep flag in superblock, if not a softdep mount. */ + MNT_ILOCK(mp); + if ((mp->mnt_flag & MNT_SOFTDEP) == 0) + fs->fs_flags &= ~FS_DOSOFTDEP; + MNT_IUNLOCK(mp); if ((fs->fs_flags & FS_GJOURNAL) != 0) { #ifdef UFS_GJOURNAL /* @@ -1071,7 +1088,7 @@ if (ronly == 0) { fs->fs_mtime = time_second; - if ((fs->fs_flags & FS_DOSOFTDEP) && + if ((mp->mnt_flag & MNT_SOFTDEP) && (error = softdep_mount(devvp, mp, fs, cred)) != 0) { free(fs->fs_csp, M_UFSMNT); ffs_flushfiles(mp, FORCECLOSE, td); Index: sbin/mount/mntopts.h =================================================================== --- sbin/mount/mntopts.h (Revision 325058) +++ sbin/mount/mntopts.h (Arbeitskopie) @@ -52,6 +52,7 @@ #define MOPT_NOCLUSTERW { "clusterw", 1, MNT_NOCLUSTERW, 0 } #define MOPT_SUIDDIR { "suiddir", 0, MNT_SUIDDIR, 0 } #define MOPT_SNAPSHOT { "snapshot", 0, MNT_SNAPSHOT, 0 } +#define MOPT_SOFTDEP { "softdep", 0, MNT_SOFTDEP, 0 } #define MOPT_MULTILABEL { "multilabel", 0, MNT_MULTILABEL, 0 } #define MOPT_ACLS { "acls", 0, MNT_ACLS, 0 } #define MOPT_NFS4ACLS { "nfsv4acls", 0, MNT_NFS4ACLS, 0 } Index: sbin/mount/mount.c =================================================================== --- sbin/mount/mount.c (Revision 325058) +++ sbin/mount/mount.c (Arbeitskopie) @@ -958,6 +958,7 @@ if (flags & MNT_NOCLUSTERW) res = catopt(res, "noclusterw"); if (flags & MNT_NOSYMFOLLOW) res = catopt(res, "nosymfollow"); if (flags & MNT_SUIDDIR) res = catopt(res, "suiddir"); + if (flags & MNT_SOFTDEP) res = catopt(res, "softdep"); if (flags & MNT_MULTILABEL) res = catopt(res, "multilabel"); if (flags & MNT_ACLS) res = catopt(res, "acls"); if (flags & MNT_NFS4ACLS) res = catopt(res, "nfsv4acls"); Index: sys/sys/mount.h =================================================================== --- sys/sys/mount.h (Revision 325058) +++ sys/sys/mount.h (Arbeitskopie) @@ -337,6 +337,7 @@ MNT_SYNCHRONOUS | MNT_UNION | MNT_ASYNC | \ MNT_NOATIME | \ MNT_NOSYMFOLLOW | MNT_IGNORE | \ + MNT_SOFTDEP | \ MNT_NOCLUSTERR | MNT_NOCLUSTERW | MNT_SUIDDIR | \ MNT_ACLS | MNT_USER | MNT_NFS4ACLS | \ MNT_AUTOMOUNTED) --------------772410BE6C4FC62BABB50287--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0bc1fba0-cda6-064a-add5-02f702e533f7>