Date: Sun, 29 Oct 2017 11:36:32 +0100 From: Stefan Esser <se@freebsd.org> To: Kirk McKusick <mckusick@mckusick.com>, Mark Johnston <markj@FreeBSD.org> Cc: freebsd-fs@FreeBSD.org Subject: Re: softdep as a mount(8) option Message-ID: <a19ed958-7b48-6c11-b945-d53dec8a77fa@freebsd.org> In-Reply-To: <201710282225.v9SMPDCZ074228@chez.mckusick.com> References: <201710282225.v9SMPDCZ074228@chez.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------24E4D2D2320EE5A929E27EF5 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Am 29.10.17 um 00:25 schrieb Kirk McKusick: >> Date: Fri, 27 Oct 2017 11:39:00 -0400 >> From: Mark Johnston <markj@FreeBSD.org> >> To: freebsd-fs@FreeBSD.org >> Subject: softdep as a mount(8) option >> >> Hi, >> >> I'd like to finally enable the use of SU (not SU+J) on some small UFS >> filesystems. The fact that SU is enabled using a flag in the superblock >> poses a problem for me, however: the systems containing these >> filesystems may at any time be repurposed to run a kernel that supports >> SU but contains bugs[*] that render it unusable. I therefore can't >> persistently enable SU in these systems. >> >> I'm wondering if it would be possible to enable SU using a mount >> option rather than with a persistent flag. fsck_ffs conditionalizes some >> of its logic on whether SU is configured - is this necessary for >> correctness? That is, if I run fsck on an unclean filesystem that had >> been mounted with SU, and fsck runs as though SU hadn't been configured, >> what problems might arise? >> >> [*] These bugs are a result of local modifications and aren't in >> FreeBSD. > > While it is safe and possible to add soft-updates (but not journalled > soft updates) as a mount option, it means that fsck will not know that > soft updates were in use, so it will always run in full (slow) mode at > boot time. This is why I have not added it as an option. 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 ... --------------24E4D2D2320EE5A929E27EF5 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"); --------------24E4D2D2320EE5A929E27EF5--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a19ed958-7b48-6c11-b945-d53dec8a77fa>