Re: [Cluster-devel] [PATCH v3 2/2] fs: remove mandatory file locking support
On Fri, Aug 20, 2021 at 12:39:19PM -0400, Jeff Layton wrote: > We added CONFIG_MANDATORY_FILE_LOCKING in 2015, and soon after turned it > off in Fedora and RHEL8. Several other distros have followed suit. > > I've heard of one problem in all that time: Someone migrated from an > older distro that supported "-o mand" to one that didn't, and the host > had a fstab entry with "mand" in it which broke on reboot. They didn't > actually _use_ mandatory locking so they just removed the mount option > and moved on. > > This patch rips out mandatory locking support wholesale from the kernel, > along with the Kconfig option and the Documentation file. It also > changes the mount code to ignore the "mand" mount option instead of > erroring out, and to throw a big, ugly warning. > > Signed-off-by: Jeff Layton This patch leaves a number of unused variables and labels behind, as can be seen in allmodcoinfig builds for various architectures. Do you expect those to be fixed individually, or do you plan to fix those up yourself ? Guenter > --- > .../filesystems/mandatory-locking.rst | 188 -- > fs/9p/vfs_file.c | 12 -- > fs/Kconfig| 10 - > fs/afs/flock.c| 4 - > fs/ceph/locks.c | 3 - > fs/gfs2/file.c| 3 - > fs/locks.c| 116 +-- > fs/namei.c| 4 +- > fs/namespace.c| 29 +-- > fs/nfs/file.c | 4 - > fs/nfsd/nfs4state.c | 13 -- > fs/nfsd/vfs.c | 15 -- > fs/ocfs2/locks.c | 4 - > fs/open.c | 8 +- > fs/read_write.c | 7 - > fs/remap_range.c | 10 - > include/linux/fs.h| 84 > mm/mmap.c | 6 - > mm/nommu.c| 3 - > 19 files changed, 14 insertions(+), 509 deletions(-) > delete mode 100644 Documentation/filesystems/mandatory-locking.rst > > diff --git a/Documentation/filesystems/mandatory-locking.rst > b/Documentation/filesystems/mandatory-locking.rst > deleted file mode 100644 > index 9ce73544a8f0.. > --- a/Documentation/filesystems/mandatory-locking.rst > +++ /dev/null > @@ -1,188 +0,0 @@ > -.. SPDX-License-Identifier: GPL-2.0 > - > -= > -Mandatory File Locking For The Linux Operating System > -= > - > - Andy Walker > - > -15 April 1996 > - > - (Updated September 2007) > - > -0. Why you should avoid mandatory locking > -- > - > -The Linux implementation is prey to a number of difficult-to-fix race > -conditions which in practice make it not dependable: > - > - - The write system call checks for a mandatory lock only once > - at its start. It is therefore possible for a lock request to > - be granted after this check but before the data is modified. > - A process may then see file data change even while a mandatory > - lock was held. > - - Similarly, an exclusive lock may be granted on a file after > - the kernel has decided to proceed with a read, but before the > - read has actually completed, and the reading process may see > - the file data in a state which should not have been visible > - to it. > - - Similar races make the claimed mutual exclusion between lock > - and mmap similarly unreliable. > - > -1. What is mandatory locking? > --- > - > -Mandatory locking is kernel enforced file locking, as opposed to the more > usual > -cooperative file locking used to guarantee sequential access to files among > -processes. File locks are applied using the flock() and fcntl() system calls > -(and the lockf() library routine which is a wrapper around fcntl().) It is > -normally a process' responsibility to check for locks on a file it wishes to > -update, before applying its own lock, updating the file and unlocking it > again. > -The most commonly used example of this (and in the case of sendmail, the most > -troublesome) is access to a user's mailbox. The mail user agent and the mail > -transfer agent must guard against updating the mailbox at the same time, and > -prevent reading the mailbox while it is being updated. > - > -In a perfect world all processes would use and honour a cooperative, or > -"advisory" locking scheme. However, the world isn't perfect, and there's > -a lot of poorly written code out there. > - > -In trying to address this problem, the designers of System V
Re: [Cluster-devel] [PATCH v3 2/2] fs: remove mandatory file locking support
On Fri, Aug 20, 2021 at 12:39:19PM -0400, Jeff Layton wrote:
> diff --git a/fs/locks.c b/fs/locks.c
> @@ -2857,8 +2744,7 @@ static void lock_get_status(struct seq_file *f, struct
> file_lock *fl,
> seq_puts(f, "POSIX ");
>
> seq_printf(f, " %s ",
> - (inode == NULL) ? "*NOINODE*" :
> - mandatory_lock(inode) ? "MANDATORY" : "ADVISORY ");
> + (inode == NULL) ? "*NOINODE*" : "ADVISORY ");
Huh?
if (fl->fl_file != NULL)
inode = locks_inode(fl->fl_file);
So basically that's fl->fl_file ? "ADVISORY" : "*NOINODE*"?
How could that trigger, though? With locks_mandatory_area() gone, I don't
see how FL_POSIX file_lock with NULL ->fl_file could be ever found...
Confused...
Why is locks_copy_conflock() exported (hell, non-static), BTW?
> diff --git a/fs/namespace.c b/fs/namespace.c
> -#ifdef CONFIG_MANDATORY_FILE_LOCKING
> -static bool may_mandlock(void)
> +static void warn_mandlock(void)
> {
> - pr_warn_once("==\n"
> - "WARNING: the mand mount option is being deprecated and\n"
> - " will be removed in v5.15!\n"
> -
> "==\n");
> - return capable(CAP_SYS_ADMIN);
> + pr_warn_once("===\n"
> + "WARNING: The mand mount option has been deprecated and\n"
> + " and is ignored by this kernel. Remove the mand\n"
> + " option from the mount to silence this warning.\n"
> +
> "===\n");
> }
> -#else
> -static inline bool may_mandlock(void)
> -{
> - pr_warn("VFS: \"mand\" mount option not supported");
> - return false;
> -}
> -#endif
Is there any point in having the previous patch not folded into this one?
[Cluster-devel] [PATCH v3 2/2] fs: remove mandatory file locking support
We added CONFIG_MANDATORY_FILE_LOCKING in 2015, and soon after turned it off in Fedora and RHEL8. Several other distros have followed suit. I've heard of one problem in all that time: Someone migrated from an older distro that supported "-o mand" to one that didn't, and the host had a fstab entry with "mand" in it which broke on reboot. They didn't actually _use_ mandatory locking so they just removed the mount option and moved on. This patch rips out mandatory locking support wholesale from the kernel, along with the Kconfig option and the Documentation file. It also changes the mount code to ignore the "mand" mount option instead of erroring out, and to throw a big, ugly warning. Signed-off-by: Jeff Layton --- .../filesystems/mandatory-locking.rst | 188 -- fs/9p/vfs_file.c | 12 -- fs/Kconfig| 10 - fs/afs/flock.c| 4 - fs/ceph/locks.c | 3 - fs/gfs2/file.c| 3 - fs/locks.c| 116 +-- fs/namei.c| 4 +- fs/namespace.c| 29 +-- fs/nfs/file.c | 4 - fs/nfsd/nfs4state.c | 13 -- fs/nfsd/vfs.c | 15 -- fs/ocfs2/locks.c | 4 - fs/open.c | 8 +- fs/read_write.c | 7 - fs/remap_range.c | 10 - include/linux/fs.h| 84 mm/mmap.c | 6 - mm/nommu.c| 3 - 19 files changed, 14 insertions(+), 509 deletions(-) delete mode 100644 Documentation/filesystems/mandatory-locking.rst diff --git a/Documentation/filesystems/mandatory-locking.rst b/Documentation/filesystems/mandatory-locking.rst deleted file mode 100644 index 9ce73544a8f0.. --- a/Documentation/filesystems/mandatory-locking.rst +++ /dev/null @@ -1,188 +0,0 @@ -.. SPDX-License-Identifier: GPL-2.0 - -= -Mandatory File Locking For The Linux Operating System -= - - Andy Walker - - 15 April 1996 - -(Updated September 2007) - -0. Why you should avoid mandatory locking -- - -The Linux implementation is prey to a number of difficult-to-fix race -conditions which in practice make it not dependable: - - - The write system call checks for a mandatory lock only once - at its start. It is therefore possible for a lock request to - be granted after this check but before the data is modified. - A process may then see file data change even while a mandatory - lock was held. - - Similarly, an exclusive lock may be granted on a file after - the kernel has decided to proceed with a read, but before the - read has actually completed, and the reading process may see - the file data in a state which should not have been visible - to it. - - Similar races make the claimed mutual exclusion between lock - and mmap similarly unreliable. - -1. What is mandatory locking? --- - -Mandatory locking is kernel enforced file locking, as opposed to the more usual -cooperative file locking used to guarantee sequential access to files among -processes. File locks are applied using the flock() and fcntl() system calls -(and the lockf() library routine which is a wrapper around fcntl().) It is -normally a process' responsibility to check for locks on a file it wishes to -update, before applying its own lock, updating the file and unlocking it again. -The most commonly used example of this (and in the case of sendmail, the most -troublesome) is access to a user's mailbox. The mail user agent and the mail -transfer agent must guard against updating the mailbox at the same time, and -prevent reading the mailbox while it is being updated. - -In a perfect world all processes would use and honour a cooperative, or -"advisory" locking scheme. However, the world isn't perfect, and there's -a lot of poorly written code out there. - -In trying to address this problem, the designers of System V UNIX came up -with a "mandatory" locking scheme, whereby the operating system kernel would -block attempts by a process to write to a file that another process holds a -"read" -or- "shared" lock on, and block attempts to both read and write to a -file that a process holds a "write " -or- "exclusive" lock on. - -The System V mandatory locking scheme was intended to have as little impact as -possible on existing user code. The scheme is based on marking
