[PATCH] locks: rename file-private locks to file-description locks

[PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

File-private locks have been merged into Linux for v3.15, and *now*
people are commenting that the name and macro definitions for the new
file-private locks suck.

...and I can't even disagree. The names and command macros do suck.

We're going to have to live with these for a long time, so it's
important that we be happy with the names before we're stuck with them.

The consensus on the lists so far is that they should be rechristened as
"file-description locks".

This patch makes the following changes that I think are necessary before
v3.15 ships:

1) rename the command macros to their new names. These end up in the uapi
   headers and so are part of the external-facing API. It turns out that
   glibc doesn't actually use the fcntl.h uapi header, but it's hard to
   be sure that something else won't. Changing it now is safest.

2) make the the /proc/locks output display these as type "FDLOCK"

The rest of the renaming can wait until v3.16, since everything else
isn't visible outside of the kernel.

Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 arch/arm/kernel/sys_oabi-compat.c |  6 +++---
 fs/compat.c                       | 14 +++++++-------
 fs/fcntl.c                        | 12 ++++++------
 fs/locks.c                        | 14 +++++++-------
 include/uapi/asm-generic/fcntl.h  | 20 ++++++++++----------
 security/selinux/hooks.c          |  6 +++---
 6 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index 702bd329d9d0..d92aa1277e40 100644
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -203,9 +203,9 @@ asmlinkage long sys_oabi_fcntl64(unsigned int fd, unsigned int cmd,
 	int ret;
 
 	switch (cmd) {
-	case F_GETLKP:
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_GETLK:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 	case F_GETLK64:
 	case F_SETLK64:
 	case F_SETLKW64:
diff --git a/fs/compat.c b/fs/compat.c
index ca926ad0430c..4933d5b32ace 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -457,9 +457,9 @@ COMPAT_SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 	case F_GETLK64:
 	case F_SETLK64:
 	case F_SETLKW64:
-	case F_GETLKP:
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_GETLK:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 		ret = get_compat_flock64(&f, compat_ptr(arg));
 		if (ret != 0)
 			break;
@@ -468,7 +468,7 @@ COMPAT_SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 		conv_cmd = convert_fcntl_cmd(cmd);
 		ret = sys_fcntl(fd, conv_cmd, (unsigned long)&f);
 		set_fs(old_fs);
-		if ((conv_cmd == F_GETLK || conv_cmd == F_GETLKP) && ret == 0) {
+		if ((conv_cmd == F_GETLK || conv_cmd == F_FD_GETLK) && ret == 0) {
 			/* need to return lock information - see above for commentary */
 			if (f.l_start > COMPAT_LOFF_T_MAX)
 				ret = -EOVERFLOW;
@@ -493,9 +493,9 @@ COMPAT_SYSCALL_DEFINE3(fcntl, unsigned int, fd, unsigned int, cmd,
 	case F_GETLK64:
 	case F_SETLK64:
 	case F_SETLKW64:
-	case F_GETLKP:
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_GETLK:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 		return -EINVAL;
 	}
 	return compat_sys_fcntl64(fd, cmd, arg);
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 9ead1596399a..a62d9ba3874b 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -274,15 +274,15 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
 		break;
 #if BITS_PER_LONG != 32
 	/* 32-bit arches must use fcntl64() */
-	case F_GETLKP:
+	case F_FD_GETLK:
 #endif
 	case F_GETLK:
 		err = fcntl_getlk(filp, cmd, (struct flock __user *) arg);
 		break;
 #if BITS_PER_LONG != 32
 	/* 32-bit arches must use fcntl64() */
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 #endif
 		/* Fallthrough */
 	case F_SETLK:
@@ -399,13 +399,13 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 	
 	switch (cmd) {
 	case F_GETLK64:
-	case F_GETLKP:
+	case F_FD_GETLK:
 		err = fcntl_getlk64(f.file, cmd, (struct flock64 __user *) arg);
 		break;
 	case F_SETLK64:
 	case F_SETLKW64:
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 		err = fcntl_setlk64(fd, f.file, cmd,
 				(struct flock64 __user *) arg);
 		break;
diff --git a/fs/locks.c b/fs/locks.c
index b380f5543614..bfe5df7aa685 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1941,7 +1941,7 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock __user *l)
 	if (error)
 		goto out;
 
-	if (cmd == F_GETLKP) {
+	if (cmd == F_FD_GETLK) {
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2076,7 +2076,7 @@ again:
 	 * FL_FILE_PVT flag and override the owner.
 	 */
 	switch (cmd) {
-	case F_SETLKP:
+	case F_FD_SETLK:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2085,7 +2085,7 @@ again:
 		file_lock->fl_flags |= FL_FILE_PVT;
 		file_lock->fl_owner = (fl_owner_t)filp;
 		break;
-	case F_SETLKPW:
+	case F_FD_SETLKW:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2143,7 +2143,7 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l)
 	if (error)
 		goto out;
 
-	if (cmd == F_GETLKP) {
+	if (cmd == F_FD_GETLK) {
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2211,7 +2211,7 @@ again:
 	 * FL_FILE_PVT flag and override the owner.
 	 */
 	switch (cmd) {
-	case F_SETLKP:
+	case F_FD_SETLK:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2220,7 +2220,7 @@ again:
 		file_lock->fl_flags |= FL_FILE_PVT;
 		file_lock->fl_owner = (fl_owner_t)filp;
 		break;
-	case F_SETLKPW:
+	case F_FD_SETLKW:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2413,7 +2413,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 		if (fl->fl_flags & FL_ACCESS)
 			seq_printf(f, "ACCESS");
 		else if (IS_FILE_PVT(fl))
-			seq_printf(f, "FLPVT ");
+			seq_printf(f, "FDLOCK");
 		else
 			seq_printf(f, "POSIX ");
 
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index a9b13f8b3595..a0154a9b28c0 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -133,20 +133,20 @@
 #endif
 
 /*
- * fd "private" POSIX locks.
+ * File-description locks.
  *
- * Usually POSIX locks held by a process are released on *any* close and are
+ * Usually record locks held by a process are released on *any* close and are
  * not inherited across a fork().
  *
- * These cmd values will set locks that conflict with normal POSIX locks, but
- * are "owned" by the opened file, not the process. This means that they are
- * inherited across fork() like BSD (flock) locks, and they are only released
- * automatically when the last reference to the the open file against which
- * they were acquired is put.
+ * These cmd values will set locks that conflict with process-associated
+ * record  locks, but are "owned" by the opened file description, not the
+ * process. This means that they are inherited across fork() like BSD (flock)
+ * locks, and they are only released automatically when the last reference to
+ * the the open file against which they were acquired is put.
  */
-#define F_GETLKP	36
-#define F_SETLKP	37
-#define F_SETLKPW	38
+#define F_FD_GETLK	36
+#define F_FD_SETLK	37
+#define F_FD_SETLKW	38
 
 #define F_OWNER_TID	0
 #define F_OWNER_PID	1
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b4beb77967b1..4e4fa02e9b17 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3317,9 +3317,9 @@ static int selinux_file_fcntl(struct file *file, unsigned int cmd,
 	case F_GETLK:
 	case F_SETLK:
 	case F_SETLKW:
-	case F_GETLKP:
-	case F_SETLKP:
-	case F_SETLKPW:
+	case F_FD_GETLK:
+	case F_FD_SETLK:
+	case F_FD_SETLKW:
 #if BITS_PER_LONG == 32
 	case F_GETLK64:
 	case F_SETLK64:
-- 
1.9.0

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
> File-private locks have been merged into Linux for v3.15, and *now*
> people are commenting that the name and macro definitions for the new
> file-private locks suck.
> 
> ....and I can't even disagree. The names and command macros do suck.
> 
> We're going to have to live with these for a long time, so it's
> important that we be happy with the names before we're stuck with them.
> 
> The consensus on the lists so far is that they should be rechristened as
> "file-description locks".
> 
> This patch makes the following changes that I think are necessary before
> v3.15 ships:
> 
> 1) rename the command macros to their new names. These end up in the uapi
>    headers and so are part of the external-facing API. It turns out that
>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
>    be sure that something else won't. Changing it now is safest.
> 
> 2) make the the /proc/locks output display these as type "FDLOCK"
> 
> The rest of the renaming can wait until v3.16, since everything else
> isn't visible outside of the kernel.

I'm sorry I didn't chime in on this earlier, but I really prefer the
(somewhat bad) current naming ("private") to the
ridiculously-confusing use of "FD" to mean "file descriptION" when
everybody is used to it meaning "file descriptOR". The potential for
confusion that these are "file descriptOR locks" (they're not) is much
more of a problem, IMO, than the confusion about what "private" means
(since it doesn't have an established meaning in this context.

Thus my vote is for leaving things the way the kernel did it already.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 04:02 PM, Rich Felker wrote:
> On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
>> File-private locks have been merged into Linux for v3.15, and *now*
>> people are commenting that the name and macro definitions for the new
>> file-private locks suck.
>>
>> ....and I can't even disagree. The names and command macros do suck.
>>
>> We're going to have to live with these for a long time, so it's
>> important that we be happy with the names before we're stuck with them.
>>
>> The consensus on the lists so far is that they should be rechristened as
>> "file-description locks".
>>
>> This patch makes the following changes that I think are necessary before
>> v3.15 ships:
>>
>> 1) rename the command macros to their new names. These end up in the uapi
>>    headers and so are part of the external-facing API. It turns out that
>>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
>>    be sure that something else won't. Changing it now is safest.
>>
>> 2) make the the /proc/locks output display these as type "FDLOCK"
>>
>> The rest of the renaming can wait until v3.16, since everything else
>> isn't visible outside of the kernel.
> 
> I'm sorry I didn't chime in on this earlier, but I really prefer the
> (somewhat bad) current naming ("private") to the
> ridiculously-confusing use of "FD" to mean "file descriptION" when
> everybody is used to it meaning "file descriptOR". The potential for
> confusion that these are "file descriptOR locks" (they're not) is much
> more of a problem, IMO, than the confusion about what "private" means
> (since it doesn't have an established meaning in this context.
> 
> Thus my vote is for leaving things the way the kernel did it already.

There's at least two problems to solve here:

1) "File private locks" is _meaningless_ as a term. Elsewhere
   (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
   I suggested various alternatives. "File-handle locks [*]" was my
   initial preference, and I also suggested "file-description locks"
   and noted the drawbacks of that term. I think it's insufficient
   to say "stick with the existing poor name"--if you have
   something better, then please propose it. (Note by the way
   that for nearly a decade now, the open(2) man page has followed
   POSIX in using the term "open file description. Full disclosure:
   of course, I'm responsible for that change in the man page.)

2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
   that are visually very close to the traditional POSIX lock names 
   (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
   when someone mistypes in code and/or misses such a misttyping
   when reading code. That really must be fixed.

Cheers,

Michael

[*] "File-handle locks" was considered by Jeff to be a little
confusing because of the term elsewhere, such as NFS. I take 
the point, though I'd still prefer it over "File-handle locks".

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 03:45 PM, Jeff Layton wrote:

[...]

> - * These cmd values will set locks that conflict with normal POSIX locks, but
> - * are "owned" by the opened file, not the process. This means that they are
> - * inherited across fork() like BSD (flock) locks, and they are only released
> - * automatically when the last reference to the the open file against which
> - * they were acquired is put.
> + * These cmd values will set locks that conflict with process-associated
> + * record  locks, but are "owned" by the opened file description, not the
> + * process. This means that they are inherited across fork() like BSD (flock)
> + * locks, and they are only released automatically when the last reference to
> + * the the open file against which they were acquired is put.


(Pre-existing) typo: s/the the/the/



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Stefan (metze) Metzmacher]

Am 21.04.2014 15:45, schrieb Jeff Layton:
> File-private locks have been merged into Linux for v3.15, and *now*
> people are commenting that the name and macro definitions for the new
> file-private locks suck.
> 
> ...and I can't even disagree. The names and command macros do suck.
> 
> We're going to have to live with these for a long time, so it's
> important that we be happy with the names before we're stuck with them.
> 
> The consensus on the lists so far is that they should be rechristened as
> "file-description locks".
> 
> This patch makes the following changes that I think are necessary before
> v3.15 ships:
> 
> 1) rename the command macros to their new names. These end up in the uapi
>    headers and so are part of the external-facing API. It turns out that
>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
>    be sure that something else won't. Changing it now is safest.
> 
> 2) make the the /proc/locks output display these as type "FDLOCK"
> 
> The rest of the renaming can wait until v3.16, since everything else
> isn't visible outside of the kernel.
> 
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

Reviewed-by: Stefan Metzmacher <metze@samba.org>

Re: [PATCH] locks: rename file-private locks to file-description locks

[Christoph Hellwig]

On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
> 
> There's at least two problems to solve here:
> 
> 1) "File private locks" is _meaningless_ as a term. Elsewhere
>    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),

It's indeed not a very good choice, but the new name is even worse.
Just call them non-broken locks? :)  Or not give them a name an just
append a 2 to the fcntls? :)

> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>    that are visually very close to the traditional POSIX lock names 
>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>    when someone mistypes in code and/or misses such a misttyping
>    when reading code. That really must be fixed.

I don't think so.  They also should have a name very similar because
they have the same semantics with a major bug fixed.  In fact I can't
think of anyone who would actually want the old behavior.

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
> On 04/21/2014 04:02 PM, Rich Felker wrote:
> > On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
> >> File-private locks have been merged into Linux for v3.15, and *now*
> >> people are commenting that the name and macro definitions for the new
> >> file-private locks suck.
> >>
> >> ....and I can't even disagree. The names and command macros do suck.
> >>
> >> We're going to have to live with these for a long time, so it's
> >> important that we be happy with the names before we're stuck with them.
> >>
> >> The consensus on the lists so far is that they should be rechristened as
> >> "file-description locks".
> >>
> >> This patch makes the following changes that I think are necessary before
> >> v3.15 ships:
> >>
> >> 1) rename the command macros to their new names. These end up in the uapi
> >>    headers and so are part of the external-facing API. It turns out that
> >>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
> >>    be sure that something else won't. Changing it now is safest.
> >>
> >> 2) make the the /proc/locks output display these as type "FDLOCK"
> >>
> >> The rest of the renaming can wait until v3.16, since everything else
> >> isn't visible outside of the kernel.
> > 
> > I'm sorry I didn't chime in on this earlier, but I really prefer the
> > (somewhat bad) current naming ("private") to the
> > ridiculously-confusing use of "FD" to mean "file descriptION" when
> > everybody is used to it meaning "file descriptOR". The potential for
> > confusion that these are "file descriptOR locks" (they're not) is much
> > more of a problem, IMO, than the confusion about what "private" means
> > (since it doesn't have an established meaning in this context.
> > 
> > Thus my vote is for leaving things the way the kernel did it already.
> 
> There's at least two problems to solve here:
> 
> 1) "File private locks" is _meaningless_ as a term. Elsewhere

That's the benefit of it: it doesn't clash with any
already-established meaning. I agree it's less than ideal, but all the
alternatives I've seen so far are worse.

>    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
>    I suggested various alternatives. "File-handle locks [*]" was my

This is also bad. "Handle" also has a defined meaning in POSIX. See
XSH 2.5.1:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html

>    initial preference, and I also suggested "file-description locks"
>    and noted the drawbacks of that term. I think it's insufficient
>    to say "stick with the existing poor name"--if you have
>    something better, then please propose it. (Note by the way
>    that for nearly a decade now, the open(2) man page has followed
>    POSIX in using the term "open file description. Full disclosure:
>    of course, I'm responsible for that change in the man page.)

I'm well aware of that. The problem is that the proposed API is using
the two-letter abbreviation FD, which ALWAYS means file descriptor and
NEVER means file description (in existing usage) to mean file
description. That's what's wrong.

> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>    that are visually very close to the traditional POSIX lock names 
>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>    when someone mistypes in code and/or misses such a misttyping
>    when reading code. That really must be fixed.

I agree, but I don't think making it worse is a solution.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

On Mon, 21 Apr 2014 09:09:27 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
> > 
> > There's at least two problems to solve here:
> > 
> > 1) "File private locks" is _meaningless_ as a term. Elsewhere
> >    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
> 
> It's indeed not a very good choice, but the new name is even worse.
> Just call them non-broken locks? :)  Or not give them a name an just
> append a 2 to the fcntls? :)
> 

I think we'll need to give them a name, if only to make it possible to
document this stuff.

I'm in Jeremy's camp on this one. I don't really care what that name
*is*. I just need to know what it is so I can finish up the docs and
make any changes to the interface that are necessary.

> > 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >    that are visually very close to the traditional POSIX lock names 
> >    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >    when someone mistypes in code and/or misses such a misttyping
> >    when reading code. That really must be fixed.
> 
> I don't think so.  They also should have a name very similar because
> they have the same semantics with a major bug fixed.  In fact I can't
> think of anyone who would actually want the old behavior.
> 

On this point, I agree with Michael. It would be easy to mix up the
names when scanning by eye, so I think there is some value in making
these more visually distinct. I rather like the idea of changing
F_SETLKP to F_*_SETLK. The question is what to put in place of the
wildcard there, and that sort of hinges on the name...

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

On Mon, 21 Apr 2014 12:10:04 -0400
Rich Felker <dalias@libc.org> wrote:

> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
> > On 04/21/2014 04:02 PM, Rich Felker wrote:
> > > On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
> > >> File-private locks have been merged into Linux for v3.15, and *now*
> > >> people are commenting that the name and macro definitions for the new
> > >> file-private locks suck.
> > >>
> > >> ....and I can't even disagree. The names and command macros do suck.
> > >>
> > >> We're going to have to live with these for a long time, so it's
> > >> important that we be happy with the names before we're stuck with them.
> > >>
> > >> The consensus on the lists so far is that they should be rechristened as
> > >> "file-description locks".
> > >>
> > >> This patch makes the following changes that I think are necessary before
> > >> v3.15 ships:
> > >>
> > >> 1) rename the command macros to their new names. These end up in the uapi
> > >>    headers and so are part of the external-facing API. It turns out that
> > >>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
> > >>    be sure that something else won't. Changing it now is safest.
> > >>
> > >> 2) make the the /proc/locks output display these as type "FDLOCK"
> > >>
> > >> The rest of the renaming can wait until v3.16, since everything else
> > >> isn't visible outside of the kernel.
> > > 
> > > I'm sorry I didn't chime in on this earlier, but I really prefer the
> > > (somewhat bad) current naming ("private") to the
> > > ridiculously-confusing use of "FD" to mean "file descriptION" when
> > > everybody is used to it meaning "file descriptOR". The potential for
> > > confusion that these are "file descriptOR locks" (they're not) is much
> > > more of a problem, IMO, than the confusion about what "private" means
> > > (since it doesn't have an established meaning in this context.
> > > 
> > > Thus my vote is for leaving things the way the kernel did it already.
> > 
> > There's at least two problems to solve here:
> > 
> > 1) "File private locks" is _meaningless_ as a term. Elsewhere
> 
> That's the benefit of it: it doesn't clash with any
> already-established meaning. I agree it's less than ideal, but all the
> alternatives I've seen so far are worse.
> 
> >    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
> >    I suggested various alternatives. "File-handle locks [*]" was my
> 
> This is also bad. "Handle" also has a defined meaning in POSIX. See
> XSH 2.5.1:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> 

Not to mention that "filehandle" has a different meaning altogether in
NFS parlance. I think we should avoid "handle" altogether in the name.

> >    initial preference, and I also suggested "file-description locks"
> >    and noted the drawbacks of that term. I think it's insufficient
> >    to say "stick with the existing poor name"--if you have
> >    something better, then please propose it. (Note by the way
> >    that for nearly a decade now, the open(2) man page has followed
> >    POSIX in using the term "open file description. Full disclosure:
> >    of course, I'm responsible for that change in the man page.)
> 
> I'm well aware of that. The problem is that the proposed API is using
> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> NEVER means file description (in existing usage) to mean file
> description. That's what's wrong.
> 

Fair enough. Assuming we kept "file-description locks" as a name, what
would you propose as new macro names?

> > 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >    that are visually very close to the traditional POSIX lock names 
> >    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >    when someone mistypes in code and/or misses such a misttyping
> >    when reading code. That really must be fixed.
> 
> I agree, but I don't think making it worse is a solution.
> 

-- 
Jeff Layton <jlayton@redhat.com>

RE: [Nfs-ganesha-devel] [PATCH] locks: rename file-private locks to file-description locks

[Frank Filz]

> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages)
> wrote:
> >
> > There's at least two problems to solve here:
> >
> > 1) "File private locks" is _meaningless_ as a term. Elsewhere
> >
> >
> (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=16
> 8
> > 5376),
> 
> It's indeed not a very good choice, but the new name is even worse.
> Just call them non-broken locks? :)  Or not give them a name an just
append
> a 2 to the fcntls? :)

The folder name I've been archiving e-mails on this subject in is called
"good locks"...

I'd also be happy with "non-broken locks"...

I could also be happy with private locks (since in addition to resolving
broken POSIX behavior, they will ALSO allow user space servers to keep
separate sets of locks for each client lock owner - if I remember, the
addition of that capability prompted the private name).

:-)

> > 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >    that are visually very close to the traditional POSIX lock names
> >    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >    when someone mistypes in code and/or misses such a misttyping
> >    when reading code. That really must be fixed.
> 
> I don't think so.  They also should have a name very similar because they
> have the same semantics with a major bug fixed.  In fact I can't think of
> anyone who would actually want the old behavior.

I'm nervous about such close names, but it's easy enough to use grep or
cscope to check a code base for typos in this case.

Frank

Re: [PATCH] locks: rename file-private locks to file-description locks

[Andy Lutomirski]

On 04/21/2014 09:45 AM, Jeff Layton wrote:
> On Mon, 21 Apr 2014 12:10:04 -0400
> Rich Felker <dalias@libc.org> wrote:
>> I'm well aware of that. The problem is that the proposed API is using
>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>> NEVER means file description (in existing usage) to mean file
>> description. That's what's wrong.
>>
> 
> Fair enough. Assuming we kept "file-description locks" as a name, what
> would you propose as new macro names?

F_OFD_...?  F_OPENFILE_...?

If you said "file description" to me, I'd assume you made a typo.  If,
on the other hand, you said "open file" or "open file description" or,
ugh, "struct file", I think I'd understand.

--Andy

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

Jeff,
On 04/21/2014 06:45 PM, Jeff Layton wrote:
> On Mon, 21 Apr 2014 12:10:04 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
>> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
>>> On 04/21/2014 04:02 PM, Rich Felker wrote:
>>>> On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
[...]
>>>    initial preference, and I also suggested "file-description locks"
>>>    and noted the drawbacks of that term. I think it's insufficient
>>>    to say "stick with the existing poor name"--if you have
>>>    something better, then please propose it. (Note by the way
>>>    that for nearly a decade now, the open(2) man page has followed
>>>    POSIX in using the term "open file description. Full disclosure:
>>>    of course, I'm responsible for that change in the man page.)
>>
>> I'm well aware of that. The problem is that the proposed API is using
>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>> NEVER means file description (in existing usage) to mean file
>> description. That's what's wrong.
>>
> 
> Fair enough. Assuming we kept "file-description locks" as a name, what
> would you propose as new macro names?

I assume you meant, "assume we kept the term 'file-private locks'..."
In that case, at least make the constants something like

F_FP_SETLK
F_FP_SETLKW
F_FP_GETLK

so that they are not confused with the traditional constants.

Cheer,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

Christoph,

On 04/21/2014 06:09 PM, Christoph Hellwig wrote:
> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
>>
>> There's at least two problems to solve here:
>>
>> 1) "File private locks" is _meaningless_ as a term. Elsewhere
>>    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
> 
> It's indeed not a very good choice, but the new name is even worse.
> Just call them non-broken locks? :)  Or not give them a name an just
> append a 2 to the fcntls? :)

As Jeff points out, they must have a name, or we can't have sensible
discussions about them ;-).

>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>    that are visually very close to the traditional POSIX lock names 
>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>    when someone mistypes in code and/or misses such a misttyping
>>    when reading code. That really must be fixed.
> 
> I don't think so.  They also should have a name very similar because
> they have the same semantics with a major bug fixed.  In fact I can't
> think of anyone who would actually want the old behavior.

They should have a name that is similar, but not so similar that 
one is easily confused for the other. Some people will inevitably
want the other behavior. Other people will be working on alternatively
legacy and new applications. We should choose some constant names
that minimize the chance of silly mistakes. Names that differ by 
just a single letter (in one case, inside the name) are a poor 
choice from that perspective.

But, solving the naming of the constants is somewhat orthogonal
to solving the name of the entity. At the least let's have something
more visually distinctive, even if we stay with the current 
terminology. (See my other mail, just sent.)

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

On Mon, 21 Apr 2014 20:18:50 +0200
"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:

> Jeff,
> On 04/21/2014 06:45 PM, Jeff Layton wrote:
> > On Mon, 21 Apr 2014 12:10:04 -0400
> > Rich Felker <dalias@libc.org> wrote:
> > 
> >> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
> >>> On 04/21/2014 04:02 PM, Rich Felker wrote:
> >>>> On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
> [...]
> >>>    initial preference, and I also suggested "file-description locks"
> >>>    and noted the drawbacks of that term. I think it's insufficient
> >>>    to say "stick with the existing poor name"--if you have
> >>>    something better, then please propose it. (Note by the way
> >>>    that for nearly a decade now, the open(2) man page has followed
> >>>    POSIX in using the term "open file description. Full disclosure:
> >>>    of course, I'm responsible for that change in the man page.)
> >>
> >> I'm well aware of that. The problem is that the proposed API is using
> >> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> >> NEVER means file description (in existing usage) to mean file
> >> description. That's what's wrong.
> >>
> > 
> > Fair enough. Assuming we kept "file-description locks" as a name, what
> > would you propose as new macro names?
> 
> I assume you meant, "assume we kept the term 'file-private locks'..."
> In that case, at least make the constants something like
> 
> F_FP_SETLK
> F_FP_SETLKW
> F_FP_GETLK
> 
> so that they are not confused with the traditional constants.
> 
> Cheer,
> 

Actually no, I was asking how Rich would name the constants if we use
the name "file-description locks" (as per the patch I posted this
morning), since his objection was the use if *_FD_* names.

I would assume that if we stick with "file-private locks" as the name,
then we'll still change the constants to a form like *_FP_*.

Also, to be clear...Frank is correct that the name "file-private" came
from allowing the locks to be "private" to a particular open file
description. Though I agree that it's a crappy name at best...

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 06:10 PM, Rich Felker wrote:
> On Mon, Apr 21, 2014 at 04:23:54PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 04/21/2014 04:02 PM, Rich Felker wrote:
>>> On Mon, Apr 21, 2014 at 09:45:35AM -0400, Jeff Layton wrote:
>>>> File-private locks have been merged into Linux for v3.15, and *now*
>>>> people are commenting that the name and macro definitions for the new
>>>> file-private locks suck.
>>>>
>>>> ....and I can't even disagree. The names and command macros do suck.
>>>>
>>>> We're going to have to live with these for a long time, so it's
>>>> important that we be happy with the names before we're stuck with them.
>>>>
>>>> The consensus on the lists so far is that they should be rechristened as
>>>> "file-description locks".
>>>>
>>>> This patch makes the following changes that I think are necessary before
>>>> v3.15 ships:
>>>>
>>>> 1) rename the command macros to their new names. These end up in the uapi
>>>>    headers and so are part of the external-facing API. It turns out that
>>>>    glibc doesn't actually use the fcntl.h uapi header, but it's hard to
>>>>    be sure that something else won't. Changing it now is safest.
>>>>
>>>> 2) make the the /proc/locks output display these as type "FDLOCK"
>>>>
>>>> The rest of the renaming can wait until v3.16, since everything else
>>>> isn't visible outside of the kernel.
>>>
>>> I'm sorry I didn't chime in on this earlier, but I really prefer the
>>> (somewhat bad) current naming ("private") to the
>>> ridiculously-confusing use of "FD" to mean "file descriptION" when
>>> everybody is used to it meaning "file descriptOR". The potential for
>>> confusion that these are "file descriptOR locks" (they're not) is much
>>> more of a problem, IMO, than the confusion about what "private" means
>>> (since it doesn't have an established meaning in this context.
>>>
>>> Thus my vote is for leaving things the way the kernel did it already.
>>
>> There's at least two problems to solve here:
>>
>> 1) "File private locks" is _meaningless_ as a term. Elsewhere
> 
> That's the benefit of it: it doesn't clash with any
> already-established meaning. I agree it's less than ideal, but all the
> alternatives I've seen so far are worse.
> 
>>    (http://thread.gmane.org/gmane.network.samba.internals/76414/focus=1685376),
>>    I suggested various alternatives. "File-handle locks [*]" was my
> 
> This is also bad. "Handle" also has a defined meaning in POSIX. See
> XSH 2.5.1:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
> 
>>    initial preference, and I also suggested "file-description locks"
>>    and noted the drawbacks of that term. I think it's insufficient
>>    to say "stick with the existing poor name"--if you have
>>    something better, then please propose it. (Note by the way
>>    that for nearly a decade now, the open(2) man page has followed
>>    POSIX in using the term "open file description. Full disclosure:
>>    of course, I'm responsible for that change in the man page.)
> 
> I'm well aware of that. The problem is that the proposed API is using
> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> NEVER means file description (in existing usage) to mean file
> description. That's what's wrong.

So, can you *please* answer this question: what do you call (i.e., 
what  everyday technical language term do use for) the thing
that sits between a file descriptor and an i-node? 

(Please don't say 'struct file' -- that is not is an implementation 
detail, and does not qualify as the kind of term that I could use 
when documenting this feature in man pages.)

POSIX uses (or invented, I am not sure which) the term file description
for a good reason: it is unambiguous, and therefore precise. I do agree
that there's a risk of confusion between 'open file descriptor" and 
'and file description'--it's the same kind of risk as between English 
terms such as 'arbitrator' and 'arbitration' (and any number of other
examples), and as language speakers we deal with this every day.

The real problem is that people are less familiar with the 
term 'open file description'. Instead people dance around with 
a number of other ambiguous terms such as 'file handle' and 'open
file table entry' and (if thinking as an implementer) 'struct file',
and sometimes need to check with one another that their differing
terminologies mean the same thing. POSIX did come up with a solution;
I think there is value in following POSIX, and making the terminology
more widespread: an 'open file description' is the thing referred
to by an 'open file descriptor'.

And just to beat this donkey further... Consider the problem of
documenting (and talking about) this new feature. Suppose we call them
'file-private locks' (FPLs). Now, suppose you are given the problem 
of explaining to someone what these new FPLs are associated with 
(and this was the point of my question above). What are you going to 
say? Are you going to quietly pretend there isn't an entity between 
open file descriptors and inodes, and just explain things in terms 
of the semantics? (I think that approach does programmers a disservice, 
since understanding that there is an entity there greatly helps
people in terms of understanding what really is going on). Or
are you going to say that 'file-private locks' are associated with
XXX, where XXX is (take your pick): 'file handle' or 'open file 
table entry' or some other term of your preference. For my part, 
I'll follow POSIX and say that FPLs refer to 'open file
descriptions'. But when I start writing funny sentences like that,
I end up asking myself: why did we introduce this new term
'file-private lock'? Why not just name it after the entity with
which it is associated: 'file handle', 'file description',
or <insert your preference here>.

>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>    that are visually very close to the traditional POSIX lock names 
>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>    when someone mistypes in code and/or misses such a misttyping
>>    when reading code. That really must be fixed.
> 
> I agree, but I don't think making it worse is a solution.

I don't agree that it's making it worse. The real problem here is 
that people use no good unambiguous term for the thing between a file
descriptor and an inode. POSIX provides us with a solution that may
not seem perfect, but it is unambiguous, and I think it might feel
more comfortable if we used it often enough.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Christoph Hellwig]

On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> So, can you *please* answer this question: what do you call (i.e., 
> what  everyday technical language term do use for) the thing
> that sits between a file descriptor and an i-node? 

An open file.

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 08:34 PM, Christoph Hellwig wrote:
> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
>> So, can you *please* answer this question: what do you call (i.e., 
>> what  everyday technical language term do use for) the thing
>> that sits between a file descriptor and an i-node? 
> 
> An open file.

Not an unambiguous, widely understood term.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 08:01 PM, Andy Lutomirski wrote:
> On 04/21/2014 09:45 AM, Jeff Layton wrote:
>> On Mon, 21 Apr 2014 12:10:04 -0400
>> Rich Felker <dalias@libc.org> wrote:
>>> I'm well aware of that. The problem is that the proposed API is using
>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>>> NEVER means file description (in existing usage) to mean file
>>> description. That's what's wrong.
>>>
>>
>> Fair enough. Assuming we kept "file-description locks" as a name, what
>> would you propose as new macro names?
> 
> F_OFD_...?  F_OPENFILE_...?
> 
> If you said "file description" to me, I'd assume you made a typo.  If,
> on the other hand, you said "open file" or "open file description" or,
> ugh, "struct file", I think I'd understand.

"Open file description locks" is a mouthful, but, personally, I could
live with it. "struct file" is not a term that belongs in user-space.
"open file" is too ambiguous, IMO.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> On 04/21/2014 06:10 PM, Rich Felker wrote:
> > I'm well aware of that. The problem is that the proposed API is using
> > the two-letter abbreviation FD, which ALWAYS means file descriptor and
> > NEVER means file description (in existing usage) to mean file
> > description. That's what's wrong.
> 
> So, can you *please* answer this question: what do you call (i.e., 
> what  everyday technical language term do use for) the thing
> that sits between a file descriptor and an i-node? 
> 
> (Please don't say 'struct file' -- that is not is an implementation 
> detail, and does not qualify as the kind of term that I could use 
> when documenting this feature in man pages.)

"Open file description".

> POSIX uses (or invented, I am not sure which) the term file description
> for a good reason: it is unambiguous, and therefore precise. I do agree
> that there's a risk of confusion between 'open file descriptor" and 
> 'and file description'--it's the same kind of risk as between English 
> terms such as 'arbitrator' and 'arbitration' (and any number of other
> examples), and as language speakers we deal with this every day.

There's not a problem when the full word is used. On the other hand,
if you use "arb" as an abbreviation for "arbitration" in a context
where it was already universally understood as meaning "arbitrator",
that would be a big problem.

Likewise the problem here isn't that "open file description" is a bad
term. It's that using "FD" to mean "[open] file description" is
utterly confusing, even moreso than just making up a new completely
random word.

> >> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >>    that are visually very close to the traditional POSIX lock names 
> >>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >>    when someone mistypes in code and/or misses such a misttyping
> >>    when reading code. That really must be fixed.
> > 
> > I agree, but I don't think making it worse is a solution.
> 
> I don't agree that it's making it worse. The real problem here is 
> that people use no good unambiguous term for the thing between a file
> descriptor and an inode. POSIX provides us with a solution that may
> not seem perfect, but it is unambiguous, and I think it might feel
> more comfortable if we used it often enough.

I would like to see it used more too, and in particular, I think it
belongs in the documentation for these new locking interfaces. But
that still doesn't answer the question of what to call them (the
macros) unless you want:

F_OPEN_FILE_DESCRIPTION_GETLK
F_OPEN_FILE_DESCRIPTION_SETLK
F_OPEN_FILE_DESCRIPTION_SETLKW

Perhaps "OP" (for open-private, i.e. private to the particular open)
would be a sensible choice; OTOH people are likely to misread it as
OPeration. The general principle I have in mind though is that it
might be nice to highlight the word "open" in "open file description"
since it (1) contrasts with file descriptor, despite file descriptors
also dealing with open files, and (2) contrasts well with legacy fcntl
locks, which are (this is the whole bug) associated with the
underlying file and not the open file description.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 02:32:38PM -0400, Jeff Layton wrote:
> > > Fair enough. Assuming we kept "file-description locks" as a name, what
> > > would you propose as new macro names?
> > 
> > I assume you meant, "assume we kept the term 'file-private locks'..."
> > In that case, at least make the constants something like
> > 
> > F_FP_SETLK
> > F_FP_SETLKW
> > F_FP_GETLK
> > 
> > so that they are not confused with the traditional constants.
> > 
> > Cheer,
> > 
> 
> Actually no, I was asking how Rich would name the constants if we use
> the name "file-description locks" (as per the patch I posted this
> morning), since his objection was the use if *_FD_* names.
> 
> I would assume that if we stick with "file-private locks" as the name,
> then we'll still change the constants to a form like *_FP_*.
> 
> Also, to be clear...Frank is correct that the name "file-private" came
> from allowing the locks to be "private" to a particular open file
> description. Though I agree that it's a crappy name at best...

As I mentioned in a reply to Michael just now, I think FP is bad
because the whole problem is that legacy fcntl locks are associated
with the underlying file rather than the open file description (open
instance). So open-private (OP) might be a better choice than
file-private.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Theodore Ts'o]

On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> So, can you *please* answer this question: what do you call (i.e., 
> what  everyday technical language term do use for) the thing
> that sits between a file descriptor and an i-node? 
> 
> (Please don't say 'struct file' -- that is not is an implementation 
> detail, and does not qualify as the kind of term that I could use 
> when documenting this feature in man pages.)

At least in a few places, if you are going to use "file description",
could you at least add a parenthetical comment:

	(commonly called a "struct file" by Linux kernel developers)

Yes, it's an implementation detail, but it's one that's been around
for over two decades, and IMHO highly unlikely to change in the
future.  So if you really want to use the POSIX terminology, it would
probably be a good idea to also use the term of art which is in common
use by the kernel developers, and I suspect has leaked out beyond
that.

I think it's actually a better and more functional name than what
POSIX uses, so maybe the best compromise is to use both, to make sure
there is no confusion.  In particular, if we have text in the man
pages where we have "file description" and "file descriptor" in the
close proximity it would probably be good to add "(i.e., a struct file
by Linux kernel developers)" just for clarity.

Cheers,

							- Ted

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 02:48:41PM -0400, Theodore Ts'o wrote:
> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> > So, can you *please* answer this question: what do you call (i.e., 
> > what  everyday technical language term do use for) the thing
> > that sits between a file descriptor and an i-node? 
> > 
> > (Please don't say 'struct file' -- that is not is an implementation 
> > detail, and does not qualify as the kind of term that I could use 
> > when documenting this feature in man pages.)
> 
> At least in a few places, if you are going to use "file description",
> could you at least add a parenthetical comment:
> 
> 	(commonly called a "struct file" by Linux kernel developers)
> 
> Yes, it's an implementation detail, but it's one that's been around
> for over two decades, and IMHO highly unlikely to change in the
> future.  So if you really want to use the POSIX terminology, it would
> probably be a good idea to also use the term of art which is in common
> use by the kernel developers, and I suspect has leaked out beyond
> that.

I don't think "struct file" has any meaning to any userspace
developers, and as such doesn't belong in documentation for userspace
programming. It's an implementation detail of the kernel that
userspace developers have no need to be aware of. There's already
enough leakage of broken kernel internals (e.g. tid vs pid vs tgid)
into userspace documentation that's immensely confusing for new
developers without adding more of it.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Theodore Ts'o]

On Mon, Apr 21, 2014 at 02:51:44PM -0400, Rich Felker wrote:
> I don't think "struct file" has any meaning to any userspace
> developers, and as such doesn't belong in documentation for userspace
> programming. It's an implementation detail of the kernel that
> userspace developers have no need to be aware of. There's already
> enough leakage of broken kernel internals (e.g. tid vs pid vs tgid)
> into userspace documentation that's immensely confusing for new
> developers without adding more of it.

Userspace programmers who are using dup(2) or dup(2) need to
understand this "implementation detail".  The fact that the file
descriptor is an integer index into an array which points to a "struct
file" object is a fundamental part of the Unix/POSIX interface.  So
the fact that it has leaked out there.

I think what you mean is that there is no need that we expose the name
"struct file".  My point is that "struct file" is actually a much
_better_ name than "file description".  Heck, "open file object" would
be better name than "file description".

Hmm, how about "open file object"?  And what I'd recommend is that we
try very hard to push that name across the documentation, into the
dup/dup2 man page, with an parenthetical explanation that if you read
reading POSIX specification, you may run across the term "file
description", which is the same thing.

Realistically, far more people are likely to be looking at the man
pages rather than the POSIX specification (if for no other reason than
you have to register your e-mail address to gain access to it
on-line).  So getting the man pages to be easily understandable is,
IMHO, more important than being in 100% alignment with POSIX.

						- Ted

Re: [PATCH] locks: rename file-private locks to file-description locks

[Christoph Hellwig]

On Mon, Apr 21, 2014 at 03:04:10PM -0400, Theodore Ts'o wrote:
> I think what you mean is that there is no need that we expose the name
> "struct file".  My point is that "struct file" is actually a much
> _better_ name than "file description".  Heck, "open file object" would
> be better name than "file description".

Open file description is what all current standards use.  I'm pretty
sure really old ones just used open file, but struct file has never
been used in an API description.  Introducing it now entirely out of
context is not helpful at all.

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

On Mon, 21 Apr 2014 14:48:29 -0400
Rich Felker <dalias@libc.org> wrote:

> On Mon, Apr 21, 2014 at 02:32:38PM -0400, Jeff Layton wrote:
> > > > Fair enough. Assuming we kept "file-description locks" as a name, what
> > > > would you propose as new macro names?
> > > 
> > > I assume you meant, "assume we kept the term 'file-private locks'..."
> > > In that case, at least make the constants something like
> > > 
> > > F_FP_SETLK
> > > F_FP_SETLKW
> > > F_FP_GETLK
> > > 
> > > so that they are not confused with the traditional constants.
> > > 
> > > Cheer,
> > > 
> > 
> > Actually no, I was asking how Rich would name the constants if we use
> > the name "file-description locks" (as per the patch I posted this
> > morning), since his objection was the use if *_FD_* names.
> > 
> > I would assume that if we stick with "file-private locks" as the name,
> > then we'll still change the constants to a form like *_FP_*.
> > 
> > Also, to be clear...Frank is correct that the name "file-private" came
> > from allowing the locks to be "private" to a particular open file
> > description. Though I agree that it's a crappy name at best...
> 
> As I mentioned in a reply to Michael just now, I think FP is bad
> because the whole problem is that legacy fcntl locks are associated
> with the underlying file rather than the open file description (open
> instance). So open-private (OP) might be a better choice than
> file-private.
> 
> Rich

Is "open-private" or "open-file-private" really any better than
"file-private" ? They're all names that only a mother could love and
I'm not sure any of them are really any clearer than the others. Also:

<pedantic>
Legacy fcntl locks are associated with the _process_ and not the
underlying file, per-se.
</pedantic>


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 08:46 PM, Rich Felker wrote:
> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 04/21/2014 06:10 PM, Rich Felker wrote:
>>> I'm well aware of that. The problem is that the proposed API is using
>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>>> NEVER means file description (in existing usage) to mean file
>>> description. That's what's wrong.
>>
>> So, can you *please* answer this question: what do you call (i.e., 
>> what  everyday technical language term do use for) the thing
>> that sits between a file descriptor and an i-node? 
>>
>> (Please don't say 'struct file' -- that is not is an implementation 
>> detail, and does not qualify as the kind of term that I could use 
>> when documenting this feature in man pages.)
> 
> "Open file description".

Oh! I didn't realize we agreed :-).

>> POSIX uses (or invented, I am not sure which) the term file description
>> for a good reason: it is unambiguous, and therefore precise. I do agree
>> that there's a risk of confusion between 'open file descriptor" and 
>> 'and file description'--it's the same kind of risk as between English 
>> terms such as 'arbitrator' and 'arbitration' (and any number of other
>> examples), and as language speakers we deal with this every day.
> 
> There's not a problem when the full word is used. On the other hand,
> if you use "arb" as an abbreviation for "arbitration" in a context
> where it was already universally understood as meaning "arbitrator",
> that would be a big problem.
> 
> Likewise the problem here isn't that "open file description" is a bad
> term. It's that using "FD" to mean "[open] file description" is
> utterly confusing, even moreso than just making up a new completely
> random word.

Ohh -- I had thought you a problem not just with "FD" but also
"(open) file description".

>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>>>    that are visually very close to the traditional POSIX lock names 
>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>>>    when someone mistypes in code and/or misses such a misttyping
>>>>    when reading code. That really must be fixed.
>>>
>>> I agree, but I don't think making it worse is a solution.
>>
>> I don't agree that it's making it worse. The real problem here is 
>> that people use no good unambiguous term for the thing between a file
>> descriptor and an inode. POSIX provides us with a solution that may
>> not seem perfect, but it is unambiguous, and I think it might feel
>> more comfortable if we used it often enough.
> 
> I would like to see it used more too, and in particular, I think it
> belongs in the documentation for these new locking interfaces. But
> that still doesn't answer the question of what to call them (the
> macros) unless you want:
> 
> F_OPEN_FILE_DESCRIPTION_GETLK
> F_OPEN_FILE_DESCRIPTION_SETLK
> F_OPEN_FILE_DESCRIPTION_SETLKW

Or just 'F_OFD_*'?

> Perhaps "OP" (for open-private, i.e. private to the particular open)
> would be a sensible choice; OTOH people are likely to misread it as
> OPeration. The general principle I have in mind though is that it
> might be nice to highlight the word "open" in "open file description"

(Fair enough.)

> since it (1) contrasts with file descriptor, despite file descriptors
> also dealing with open files, and (2) contrasts well with legacy fcntl
> locks, which are (this is the whole bug) associated with the
> underlying file and not the open file description.

Makes sense to me. (We are in more agreement that I realized.)

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

On Mon, 21 Apr 2014 21:39:12 +0200
"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:

> On 04/21/2014 08:46 PM, Rich Felker wrote:
> > On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> >> On 04/21/2014 06:10 PM, Rich Felker wrote:
> >>> I'm well aware of that. The problem is that the proposed API is using
> >>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> >>> NEVER means file description (in existing usage) to mean file
> >>> description. That's what's wrong.
> >>
> >> So, can you *please* answer this question: what do you call (i.e., 
> >> what  everyday technical language term do use for) the thing
> >> that sits between a file descriptor and an i-node? 
> >>
> >> (Please don't say 'struct file' -- that is not is an implementation 
> >> detail, and does not qualify as the kind of term that I could use 
> >> when documenting this feature in man pages.)
> > 
> > "Open file description".
> 
> Oh! I didn't realize we agreed :-).
> 
> >> POSIX uses (or invented, I am not sure which) the term file description
> >> for a good reason: it is unambiguous, and therefore precise. I do agree
> >> that there's a risk of confusion between 'open file descriptor" and 
> >> 'and file description'--it's the same kind of risk as between English 
> >> terms such as 'arbitrator' and 'arbitration' (and any number of other
> >> examples), and as language speakers we deal with this every day.
> > 
> > There's not a problem when the full word is used. On the other hand,
> > if you use "arb" as an abbreviation for "arbitration" in a context
> > where it was already universally understood as meaning "arbitrator",
> > that would be a big problem.
> > 
> > Likewise the problem here isn't that "open file description" is a bad
> > term. It's that using "FD" to mean "[open] file description" is
> > utterly confusing, even moreso than just making up a new completely
> > random word.
> 
> Ohh -- I had thought you a problem not just with "FD" but also
> "(open) file description".
> 
> >>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >>>>    that are visually very close to the traditional POSIX lock names 
> >>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >>>>    when someone mistypes in code and/or misses such a misttyping
> >>>>    when reading code. That really must be fixed.
> >>>
> >>> I agree, but I don't think making it worse is a solution.
> >>
> >> I don't agree that it's making it worse. The real problem here is 
> >> that people use no good unambiguous term for the thing between a file
> >> descriptor and an inode. POSIX provides us with a solution that may
> >> not seem perfect, but it is unambiguous, and I think it might feel
> >> more comfortable if we used it often enough.
> > 
> > I would like to see it used more too, and in particular, I think it
> > belongs in the documentation for these new locking interfaces. But
> > that still doesn't answer the question of what to call them (the
> > macros) unless you want:
> > 
> > F_OPEN_FILE_DESCRIPTION_GETLK
> > F_OPEN_FILE_DESCRIPTION_SETLK
> > F_OPEN_FILE_DESCRIPTION_SETLKW
> 
> Or just 'F_OFD_*'?
> 
> > Perhaps "OP" (for open-private, i.e. private to the particular open)
> > would be a sensible choice; OTOH people are likely to misread it as
> > OPeration. The general principle I have in mind though is that it
> > might be nice to highlight the word "open" in "open file description"
> 
> (Fair enough.)
> 
> > since it (1) contrasts with file descriptor, despite file descriptors
> > also dealing with open files, and (2) contrasts well with legacy fcntl
> > locks, which are (this is the whole bug) associated with the
> > underlying file and not the open file description.
> 
> Makes sense to me. (We are in more agreement that I realized.)
> 
> Cheers,
> 
> Michael
> 
> 
> 

So the motion is to call them "open file description locks" and change
the macros to read *_OFD_*. Does anyone object?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 09:06 PM, Christoph Hellwig wrote:
> On Mon, Apr 21, 2014 at 03:04:10PM -0400, Theodore Ts'o wrote:
>> I think what you mean is that there is no need that we expose the name
>> "struct file".  My point is that "struct file" is actually a much
>> _better_ name than "file description".  Heck, "open file object" would
>> be better name than "file description".
> 
> Open file description is what all current standards use.  I'm pretty
> sure really old ones just used open file, 

("open file description" was already in SUSv1 (1994))

> but struct file has never
> been used in an API description.  

Exactly.

> Introducing it now entirely out of
> context is not helpful at all.

In principle, I agree, though it might be helpful for some
people to mention this term in a side-note in, say, open(2).

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 03:04:10PM -0400, Theodore Ts'o wrote:
> On Mon, Apr 21, 2014 at 02:51:44PM -0400, Rich Felker wrote:
> > I don't think "struct file" has any meaning to any userspace
> > developers, and as such doesn't belong in documentation for userspace
> > programming. It's an implementation detail of the kernel that
> > userspace developers have no need to be aware of. There's already
> > enough leakage of broken kernel internals (e.g. tid vs pid vs tgid)
> > into userspace documentation that's immensely confusing for new
> > developers without adding more of it.
> 
> Userspace programmers who are using dup(2) or dup(2) need to
> understand this "implementation detail".  The fact that the file

No they don't. They need to understand the concept of an open file
description, not the kernel's "struct file" which is one (of many)
possible implementation of that. Irrelevant implementation details do
not belong in documentation.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Rich Felker]

On Mon, Apr 21, 2014 at 03:16:29PM -0400, Jeff Layton wrote:
> On Mon, 21 Apr 2014 14:48:29 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
> > On Mon, Apr 21, 2014 at 02:32:38PM -0400, Jeff Layton wrote:
> > > > > Fair enough. Assuming we kept "file-description locks" as a name, what
> > > > > would you propose as new macro names?
> > > > 
> > > > I assume you meant, "assume we kept the term 'file-private locks'..."
> > > > In that case, at least make the constants something like
> > > > 
> > > > F_FP_SETLK
> > > > F_FP_SETLKW
> > > > F_FP_GETLK
> > > > 
> > > > so that they are not confused with the traditional constants.
> > > > 
> > > > Cheer,
> > > > 
> > > 
> > > Actually no, I was asking how Rich would name the constants if we use
> > > the name "file-description locks" (as per the patch I posted this
> > > morning), since his objection was the use if *_FD_* names.
> > > 
> > > I would assume that if we stick with "file-private locks" as the name,
> > > then we'll still change the constants to a form like *_FP_*.
> > > 
> > > Also, to be clear...Frank is correct that the name "file-private" came
> > > from allowing the locks to be "private" to a particular open file
> > > description. Though I agree that it's a crappy name at best...
> > 
> > As I mentioned in a reply to Michael just now, I think FP is bad
> > because the whole problem is that legacy fcntl locks are associated
> > with the underlying file rather than the open file description (open
> > instance). So open-private (OP) might be a better choice than
> > file-private.
> 
> Is "open-private" or "open-file-private" really any better than
> "file-private" ? They're all names that only a mother could love and
> I'm not sure any of them are really any clearer than the others. Also:

Yes, much better. File-private expresses the current broken semantics
of fcntl locks: being associated with files. The whole point of the
new locks is NOT to be associated with files but with open file
descriptions.

Rich

Re: [PATCH] locks: rename file-private locks to file-description locks

[Stefan (metze) Metzmacher]

Am 21.04.2014 21:55, schrieb Jeff Layton:
> On Mon, 21 Apr 2014 21:39:12 +0200
> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
> 
>> On 04/21/2014 08:46 PM, Rich Felker wrote:
>>> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
>>>> On 04/21/2014 06:10 PM, Rich Felker wrote:
>>>>> I'm well aware of that. The problem is that the proposed API is using
>>>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>>>>> NEVER means file description (in existing usage) to mean file
>>>>> description. That's what's wrong.
>>>>
>>>> So, can you *please* answer this question: what do you call (i.e., 
>>>> what  everyday technical language term do use for) the thing
>>>> that sits between a file descriptor and an i-node? 
>>>>
>>>> (Please don't say 'struct file' -- that is not is an implementation 
>>>> detail, and does not qualify as the kind of term that I could use 
>>>> when documenting this feature in man pages.)
>>>
>>> "Open file description".
>>
>> Oh! I didn't realize we agreed :-).
>>
>>>> POSIX uses (or invented, I am not sure which) the term file description
>>>> for a good reason: it is unambiguous, and therefore precise. I do agree
>>>> that there's a risk of confusion between 'open file descriptor" and 
>>>> 'and file description'--it's the same kind of risk as between English 
>>>> terms such as 'arbitrator' and 'arbitration' (and any number of other
>>>> examples), and as language speakers we deal with this every day.
>>>
>>> There's not a problem when the full word is used. On the other hand,
>>> if you use "arb" as an abbreviation for "arbitration" in a context
>>> where it was already universally understood as meaning "arbitrator",
>>> that would be a big problem.
>>>
>>> Likewise the problem here isn't that "open file description" is a bad
>>> term. It's that using "FD" to mean "[open] file description" is
>>> utterly confusing, even moreso than just making up a new completely
>>> random word.
>>
>> Ohh -- I had thought you a problem not just with "FD" but also
>> "(open) file description".
>>
>>>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>>>>>    that are visually very close to the traditional POSIX lock names 
>>>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>>>>>    when someone mistypes in code and/or misses such a misttyping
>>>>>>    when reading code. That really must be fixed.
>>>>>
>>>>> I agree, but I don't think making it worse is a solution.
>>>>
>>>> I don't agree that it's making it worse. The real problem here is 
>>>> that people use no good unambiguous term for the thing between a file
>>>> descriptor and an inode. POSIX provides us with a solution that may
>>>> not seem perfect, but it is unambiguous, and I think it might feel
>>>> more comfortable if we used it often enough.
>>>
>>> I would like to see it used more too, and in particular, I think it
>>> belongs in the documentation for these new locking interfaces. But
>>> that still doesn't answer the question of what to call them (the
>>> macros) unless you want:
>>>
>>> F_OPEN_FILE_DESCRIPTION_GETLK
>>> F_OPEN_FILE_DESCRIPTION_SETLK
>>> F_OPEN_FILE_DESCRIPTION_SETLKW
>>
>> Or just 'F_OFD_*'?
>>
>>> Perhaps "OP" (for open-private, i.e. private to the particular open)
>>> would be a sensible choice; OTOH people are likely to misread it as
>>> OPeration. The general principle I have in mind though is that it
>>> might be nice to highlight the word "open" in "open file description"
>>
>> (Fair enough.)
>>
>>> since it (1) contrasts with file descriptor, despite file descriptors
>>> also dealing with open files, and (2) contrasts well with legacy fcntl
>>> locks, which are (this is the whole bug) associated with the
>>> underlying file and not the open file description.
>>
>> Makes sense to me. (We are in more agreement that I realized.)
>>
>> Cheers,
>>
>> Michael
>>
>>
>>
> 
> So the motion is to call them "open file description locks" and change
> the macros to read *_OFD_*. Does anyone object?

Works fine for me...

metze

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/21/2014 11:15 PM, Stefan (metze) Metzmacher wrote:
> Am 21.04.2014 21:55, schrieb Jeff Layton:
>> On Mon, 21 Apr 2014 21:39:12 +0200
>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
>>
>>> On 04/21/2014 08:46 PM, Rich Felker wrote:
>>>> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
>>>>> On 04/21/2014 06:10 PM, Rich Felker wrote:
>>>>>> I'm well aware of that. The problem is that the proposed API is using
>>>>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>>>>>> NEVER means file description (in existing usage) to mean file
>>>>>> description. That's what's wrong.
>>>>>
>>>>> So, can you *please* answer this question: what do you call (i.e., 
>>>>> what  everyday technical language term do use for) the thing
>>>>> that sits between a file descriptor and an i-node? 
>>>>>
>>>>> (Please don't say 'struct file' -- that is not is an implementation 
>>>>> detail, and does not qualify as the kind of term that I could use 
>>>>> when documenting this feature in man pages.)
>>>>
>>>> "Open file description".
>>>
>>> Oh! I didn't realize we agreed :-).
>>>
>>>>> POSIX uses (or invented, I am not sure which) the term file description
>>>>> for a good reason: it is unambiguous, and therefore precise. I do agree
>>>>> that there's a risk of confusion between 'open file descriptor" and 
>>>>> 'and file description'--it's the same kind of risk as between English 
>>>>> terms such as 'arbitrator' and 'arbitration' (and any number of other
>>>>> examples), and as language speakers we deal with this every day.
>>>>
>>>> There's not a problem when the full word is used. On the other hand,
>>>> if you use "arb" as an abbreviation for "arbitration" in a context
>>>> where it was already universally understood as meaning "arbitrator",
>>>> that would be a big problem.
>>>>
>>>> Likewise the problem here isn't that "open file description" is a bad
>>>> term. It's that using "FD" to mean "[open] file description" is
>>>> utterly confusing, even moreso than just making up a new completely
>>>> random word.
>>>
>>> Ohh -- I had thought you a problem not just with "FD" but also
>>> "(open) file description".
>>>
>>>>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>>>>>>    that are visually very close to the traditional POSIX lock names 
>>>>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>>>>>>    when someone mistypes in code and/or misses such a misttyping
>>>>>>>    when reading code. That really must be fixed.
>>>>>>
>>>>>> I agree, but I don't think making it worse is a solution.
>>>>>
>>>>> I don't agree that it's making it worse. The real problem here is 
>>>>> that people use no good unambiguous term for the thing between a file
>>>>> descriptor and an inode. POSIX provides us with a solution that may
>>>>> not seem perfect, but it is unambiguous, and I think it might feel
>>>>> more comfortable if we used it often enough.
>>>>
>>>> I would like to see it used more too, and in particular, I think it
>>>> belongs in the documentation for these new locking interfaces. But
>>>> that still doesn't answer the question of what to call them (the
>>>> macros) unless you want:
>>>>
>>>> F_OPEN_FILE_DESCRIPTION_GETLK
>>>> F_OPEN_FILE_DESCRIPTION_SETLK
>>>> F_OPEN_FILE_DESCRIPTION_SETLKW
>>>
>>> Or just 'F_OFD_*'?
>>>
>>>> Perhaps "OP" (for open-private, i.e. private to the particular open)
>>>> would be a sensible choice; OTOH people are likely to misread it as
>>>> OPeration. The general principle I have in mind though is that it
>>>> might be nice to highlight the word "open" in "open file description"
>>>
>>> (Fair enough.)
>>>
>>>> since it (1) contrasts with file descriptor, despite file descriptors
>>>> also dealing with open files, and (2) contrasts well with legacy fcntl
>>>> locks, which are (this is the whole bug) associated with the
>>>> underlying file and not the open file description.
>>>
>>> Makes sense to me. (We are in more agreement that I realized.)
>>>
>>> Cheers,
>>>
>>> Michael
>>>
>>>
>>>
>>
>> So the motion is to call them "open file description locks" and change
>> the macros to read *_OFD_*. Does anyone object?
> 
> Works fine for me...

And works for me.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] locks: rename file-private locks to file-description locks

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 5659 bytes --]

On Tue, 22 Apr 2014 06:54:36 +0200 "Michael Kerrisk (man-pages)"
<mtk.manpages@gmail.com> wrote:

> On 04/21/2014 11:15 PM, Stefan (metze) Metzmacher wrote:
> > Am 21.04.2014 21:55, schrieb Jeff Layton:
> >> On Mon, 21 Apr 2014 21:39:12 +0200
> >> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
> >>
> >>> On 04/21/2014 08:46 PM, Rich Felker wrote:
> >>>> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> >>>>> On 04/21/2014 06:10 PM, Rich Felker wrote:
> >>>>>> I'm well aware of that. The problem is that the proposed API is using
> >>>>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> >>>>>> NEVER means file description (in existing usage) to mean file
> >>>>>> description. That's what's wrong.
> >>>>>
> >>>>> So, can you *please* answer this question: what do you call (i.e., 
> >>>>> what  everyday technical language term do use for) the thing
> >>>>> that sits between a file descriptor and an i-node? 
> >>>>>
> >>>>> (Please don't say 'struct file' -- that is not is an implementation 
> >>>>> detail, and does not qualify as the kind of term that I could use 
> >>>>> when documenting this feature in man pages.)
> >>>>
> >>>> "Open file description".
> >>>
> >>> Oh! I didn't realize we agreed :-).
> >>>
> >>>>> POSIX uses (or invented, I am not sure which) the term file description
> >>>>> for a good reason: it is unambiguous, and therefore precise. I do agree
> >>>>> that there's a risk of confusion between 'open file descriptor" and 
> >>>>> 'and file description'--it's the same kind of risk as between English 
> >>>>> terms such as 'arbitrator' and 'arbitration' (and any number of other
> >>>>> examples), and as language speakers we deal with this every day.
> >>>>
> >>>> There's not a problem when the full word is used. On the other hand,
> >>>> if you use "arb" as an abbreviation for "arbitration" in a context
> >>>> where it was already universally understood as meaning "arbitrator",
> >>>> that would be a big problem.
> >>>>
> >>>> Likewise the problem here isn't that "open file description" is a bad
> >>>> term. It's that using "FD" to mean "[open] file description" is
> >>>> utterly confusing, even moreso than just making up a new completely
> >>>> random word.
> >>>
> >>> Ohh -- I had thought you a problem not just with "FD" but also
> >>> "(open) file description".
> >>>
> >>>>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> >>>>>>>    that are visually very close to the traditional POSIX lock names 
> >>>>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> >>>>>>>    when someone mistypes in code and/or misses such a misttyping
> >>>>>>>    when reading code. That really must be fixed.
> >>>>>>
> >>>>>> I agree, but I don't think making it worse is a solution.
> >>>>>
> >>>>> I don't agree that it's making it worse. The real problem here is 
> >>>>> that people use no good unambiguous term for the thing between a file
> >>>>> descriptor and an inode. POSIX provides us with a solution that may
> >>>>> not seem perfect, but it is unambiguous, and I think it might feel
> >>>>> more comfortable if we used it often enough.
> >>>>
> >>>> I would like to see it used more too, and in particular, I think it
> >>>> belongs in the documentation for these new locking interfaces. But
> >>>> that still doesn't answer the question of what to call them (the
> >>>> macros) unless you want:
> >>>>
> >>>> F_OPEN_FILE_DESCRIPTION_GETLK
> >>>> F_OPEN_FILE_DESCRIPTION_SETLK
> >>>> F_OPEN_FILE_DESCRIPTION_SETLKW
> >>>
> >>> Or just 'F_OFD_*'?
> >>>
> >>>> Perhaps "OP" (for open-private, i.e. private to the particular open)
> >>>> would be a sensible choice; OTOH people are likely to misread it as
> >>>> OPeration. The general principle I have in mind though is that it
> >>>> might be nice to highlight the word "open" in "open file description"
> >>>
> >>> (Fair enough.)
> >>>
> >>>> since it (1) contrasts with file descriptor, despite file descriptors
> >>>> also dealing with open files, and (2) contrasts well with legacy fcntl
> >>>> locks, which are (this is the whole bug) associated with the
> >>>> underlying file and not the open file description.
> >>>
> >>> Makes sense to me. (We are in more agreement that I realized.)
> >>>
> >>> Cheers,
> >>>
> >>> Michael
> >>>
> >>>
> >>>
> >>
> >> So the motion is to call them "open file description locks" and change
> >> the macros to read *_OFD_*. Does anyone object?
> > 
> > Works fine for me...
> 
> And works for me.
> 
> 

I think the word "open" is important here.
I find that "description" is not a word I would have every thought was
relevant here - it is obviously too long since I have read the man pages.

I would prefer
  per-open locks
which are contrasted with
  per-process locks

An alternative might be "flock-like" as locks created with "flock" have
exactly the property we are trying to describe.  Reading the man page for
"flock" then suggests "open file table entry locks" which is even more of a
mouthful.  "oftel" is pronounceable though.  Then we could talk about "oftel
locks" in the same sentence as "pin numbers" and "RAM memory".

But maybe I came too late to this party, and the boat has sailed?

Note to Michael: The text
   flock() does not lock files over NFS.
in flock(2) is no longer accurate.  The reality is ... complex.
See nfs(5), and search for "local_lock".


NeilBrown

(I don't care what the macros are - XX would work as well any anything else
for me).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] locks: rename file-private locks to file-description locks

[Michael Kerrisk (man-pages)]

On 04/27/2014 06:51 AM, NeilBrown wrote:
> On Tue, 22 Apr 2014 06:54:36 +0200 "Michael Kerrisk (man-pages)"
> <mtk.manpages@gmail.com> wrote:
> 
>> On 04/21/2014 11:15 PM, Stefan (metze) Metzmacher wrote:
>>> Am 21.04.2014 21:55, schrieb Jeff Layton:
>>>> On Mon, 21 Apr 2014 21:39:12 +0200
>>>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
>>>>
>>>>> On 04/21/2014 08:46 PM, Rich Felker wrote:
>>>>>> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
>>>>>>> On 04/21/2014 06:10 PM, Rich Felker wrote:
>>>>>>>> I'm well aware of that. The problem is that the proposed API is using
>>>>>>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
>>>>>>>> NEVER means file description (in existing usage) to mean file
>>>>>>>> description. That's what's wrong.
>>>>>>>
>>>>>>> So, can you *please* answer this question: what do you call (i.e., 
>>>>>>> what  everyday technical language term do use for) the thing
>>>>>>> that sits between a file descriptor and an i-node? 
>>>>>>>
>>>>>>> (Please don't say 'struct file' -- that is not is an implementation 
>>>>>>> detail, and does not qualify as the kind of term that I could use 
>>>>>>> when documenting this feature in man pages.)
>>>>>>
>>>>>> "Open file description".
>>>>>
>>>>> Oh! I didn't realize we agreed :-).
>>>>>
>>>>>>> POSIX uses (or invented, I am not sure which) the term file description
>>>>>>> for a good reason: it is unambiguous, and therefore precise. I do agree
>>>>>>> that there's a risk of confusion between 'open file descriptor" and 
>>>>>>> 'and file description'--it's the same kind of risk as between English 
>>>>>>> terms such as 'arbitrator' and 'arbitration' (and any number of other
>>>>>>> examples), and as language speakers we deal with this every day.
>>>>>>
>>>>>> There's not a problem when the full word is used. On the other hand,
>>>>>> if you use "arb" as an abbreviation for "arbitration" in a context
>>>>>> where it was already universally understood as meaning "arbitrator",
>>>>>> that would be a big problem.
>>>>>>
>>>>>> Likewise the problem here isn't that "open file description" is a bad
>>>>>> term. It's that using "FD" to mean "[open] file description" is
>>>>>> utterly confusing, even moreso than just making up a new completely
>>>>>> random word.
>>>>>
>>>>> Ohh -- I had thought you a problem not just with "FD" but also
>>>>> "(open) file description".
>>>>>
>>>>>>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
>>>>>>>>>    that are visually very close to the traditional POSIX lock names 
>>>>>>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
>>>>>>>>>    when someone mistypes in code and/or misses such a misttyping
>>>>>>>>>    when reading code. That really must be fixed.
>>>>>>>>
>>>>>>>> I agree, but I don't think making it worse is a solution.
>>>>>>>
>>>>>>> I don't agree that it's making it worse. The real problem here is 
>>>>>>> that people use no good unambiguous term for the thing between a file
>>>>>>> descriptor and an inode. POSIX provides us with a solution that may
>>>>>>> not seem perfect, but it is unambiguous, and I think it might feel
>>>>>>> more comfortable if we used it often enough.
>>>>>>
>>>>>> I would like to see it used more too, and in particular, I think it
>>>>>> belongs in the documentation for these new locking interfaces. But
>>>>>> that still doesn't answer the question of what to call them (the
>>>>>> macros) unless you want:
>>>>>>
>>>>>> F_OPEN_FILE_DESCRIPTION_GETLK
>>>>>> F_OPEN_FILE_DESCRIPTION_SETLK
>>>>>> F_OPEN_FILE_DESCRIPTION_SETLKW
>>>>>
>>>>> Or just 'F_OFD_*'?
>>>>>
>>>>>> Perhaps "OP" (for open-private, i.e. private to the particular open)
>>>>>> would be a sensible choice; OTOH people are likely to misread it as
>>>>>> OPeration. The general principle I have in mind though is that it
>>>>>> might be nice to highlight the word "open" in "open file description"
>>>>>
>>>>> (Fair enough.)
>>>>>
>>>>>> since it (1) contrasts with file descriptor, despite file descriptors
>>>>>> also dealing with open files, and (2) contrasts well with legacy fcntl
>>>>>> locks, which are (this is the whole bug) associated with the
>>>>>> underlying file and not the open file description.
>>>>>
>>>>> Makes sense to me. (We are in more agreement that I realized.)
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Michael
>>>>>
>>>>>
>>>>>
>>>>
>>>> So the motion is to call them "open file description locks" and change
>>>> the macros to read *_OFD_*. Does anyone object?
>>>
>>> Works fine for me...
>>
>> And works for me.
>>
>>
> 
> I think the word "open" is important here.
> I find that "description" is not a word I would have every thought was
> relevant here - it is obviously too long since I have read the man pages.
> 
> I would prefer
>   per-open locks

The problem I see with that is that the term doesn't really convey
any meaning. So, I think OFD locks is better (albeit a bit of a mouthful).

> which are contrasted with
>   per-process locks

And that term isn't really used, except in recent discussions.

> An alternative might be "flock-like" as locks created with "flock" have
> exactly the property we are trying to describe.  Reading the man page for
> "flock" then suggests "open file table entry locks" 

That was true until a few days ago:
http://git.kernel.org/cgit/docs/man-pages/man-pages.git/commit/?id=61a37c81ef11da9a12df91da181b943ef29cf2ba

> which is even more of a
> mouthful.  "oftel" is pronounceable though.  Then we could talk about "oftel
> locks" in the same sentence as "pin numbers" and "RAM memory".

I'll give full marks for inventiveness. But, the man-pages are moving 
(even) more consistently to the POSIX terminology, so I think that term
would have its own confusions.

> But maybe I came too late to this party, and the boat has sailed?

Jeff's decision in the end, but I suspect yes. 

> Note to Michael: The text
>    flock() does not lock files over NFS.
> in flock(2) is no longer accurate.  The reality is ... complex.
> See nfs(5), and search for "local_lock".

I'll take this to a separate mail...

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Michael Kerrisk (man-pages)]

[Trimming some folk from CC, and adding various NFS people]

On 04/27/2014 06:51 AM, NeilBrown wrote:

[...]

> Note to Michael: The text
>    flock() does not lock files over NFS.
> in flock(2) is no longer accurate.  The reality is ... complex.
> See nfs(5), and search for "local_lock".

Ahhh -- I see: 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2

Thanks for the heads up.

Just in general, it would be great if the flock(2) and fcntl(2) man pages
contained correct details for NFS, of course. So, for example, if there 
are any current gotchas for NFS and fcntl() byte-range locking, I'd like 
to add those to the fcntl(2) man page.

Anyway, returning to your point about flock(), how would this text
look for the flock(2) manual page:

    NOTES
       Since  kernel  2.0,  flock() is implemented as a system call in
       its own right rather than being emulated in the GNU  C  library
       as  a  call  to fcntl(2).  This yields classical BSD semantics:
       there is no interaction between the types  of  lock  placed  by
       flock()  and  fcntl(2),  and  flock() does not detect deadlock.
       (Note, however, that on some modern BSDs, flock() and  fcntl(2)
       locks do interact with one another.)

       In Linux kernels up to 2.6.11, flock() does not lock files over
       NFS (i.e., the scope of locks was limited to the local system).
       Instead,  one could use fcntl(2) byte-range locking, which does
       work over NFS, given a sufficiently recent version of Linux and
       a  server  which  supports  locking.   Since  Linux 2.6.12, NFS
       clients support flock() locks by emulating them  as  byte-range
       locks on the entire file.  This means that fcntl(2) and flock()
       locks do interact with  one  another  over  NFS.   Since  Linux
       2.6.37,  the  kernel  supports a compatibility mode that allows
       flock() locks (and also  fcntl(2)  byte  region  locks)  to  be
       treated  as  local; see the discussion of the local_lock option
       in nfs(5).
?

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 3182 bytes --]

On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> [Trimming some folk from CC, and adding various NFS people]
> 
> On 04/27/2014 06:51 AM, NeilBrown wrote:
> 
> [...]
> 
> > Note to Michael: The text
> >    flock() does not lock files over NFS.
> > in flock(2) is no longer accurate.  The reality is ... complex.
> > See nfs(5), and search for "local_lock".
> 
> Ahhh -- I see: 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
> 
> Thanks for the heads up.
> 
> Just in general, it would be great if the flock(2) and fcntl(2) man pages
> contained correct details for NFS, of course. So, for example, if there 
> are any current gotchas for NFS and fcntl() byte-range locking, I'd like 
> to add those to the fcntl(2) man page.

The only peculiarities I can think of are:
 - With NFS, locking or unlocking a region forces a flush of any cached data 
   for that file (or maybe for the region of the file).  I'm not sure if this
   is worth mentioning.

 - With NFSv4 the client can lose a lock if it is out of contact with the
   server for a period of time.  When this happens, any IO to the file by a
   process which "thinks" it holds a lock will fail until that process closes
   and re-opens the file.
   This behaviour is since 3.12.  Prior to that the client might lose and
   regain the lock without ever knowing thus potentially risking corruption
   (but only if client and server lost contact for an extended period).

> 
> Anyway, returning to your point about flock(), how would this text
> look for the flock(2) manual page:
> 
>     NOTES
>        Since  kernel  2.0,  flock() is implemented as a system call in
>        its own right rather than being emulated in the GNU  C  library
>        as  a  call  to fcntl(2).  This yields classical BSD semantics:
>        there is no interaction between the types  of  lock  placed  by
>        flock()  and  fcntl(2),  and  flock() does not detect deadlock.
>        (Note, however, that on some modern BSDs, flock() and  fcntl(2)
>        locks do interact with one another.)
> 
>        In Linux kernels up to 2.6.11, flock() does not lock files over
>        NFS (i.e., the scope of locks was limited to the local system).
>        Instead,  one could use fcntl(2) byte-range locking, which does
>        work over NFS, given a sufficiently recent version of Linux and
>        a  server  which  supports  locking.   Since  Linux 2.6.12, NFS
>        clients support flock() locks by emulating them  as  byte-range
>        locks on the entire file.  This means that fcntl(2) and flock()
>        locks do interact with  one  another  over  NFS.   Since  Linux
>        2.6.37,  the  kernel  supports a compatibility mode that allows
>        flock() locks (and also  fcntl(2)  byte  region  locks)  to  be
>        treated  as  local; see the discussion of the local_lock option
>        in nfs(5).
> ?

That seems to cover it quite well - thanks.

NeilBrown

> 
> Thanks,
> 
> Michael
> 
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Michael Kerrisk (man-pages)]

On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> wrote:
> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> [Trimming some folk from CC, and adding various NFS people]
>>
>> On 04/27/2014 06:51 AM, NeilBrown wrote:
>>
>> [...]
>>
>> > Note to Michael: The text
>> >    flock() does not lock files over NFS.
>> > in flock(2) is no longer accurate.  The reality is ... complex.
>> > See nfs(5), and search for "local_lock".
>>
>> Ahhh -- I see:
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
>>
>> Thanks for the heads up.
>>
>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
>> contained correct details for NFS, of course. So, for example, if there
>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
>> to add those to the fcntl(2) man page.
>
> The only peculiarities I can think of are:
>  - With NFS, locking or unlocking a region forces a flush of any cached data
>    for that file (or maybe for the region of the file).  I'm not sure if this
>    is worth mentioning.

I agree that it's probably not necessary to mention.

>  - With NFSv4 the client can lose a lock if it is out of contact with the
>    server for a period of time.  When this happens, any IO to the file by a
>    process which "thinks" it holds a lock will fail until that process closes
>    and re-opens the file.
>    This behaviour is since 3.12.  Prior to that the client might lose and
>    regain the lock without ever knowing thus potentially risking corruption
>    (but only if client and server lost contact for an extended period).

Do you have a pointer for that commit to 3.12?

>> Anyway, returning to your point about flock(), how would this text
>> look for the flock(2) manual page:
>>
>>     NOTES
>>        Since  kernel  2.0,  flock() is implemented as a system call in
>>        its own right rather than being emulated in the GNU  C  library
>>        as  a  call  to fcntl(2).  This yields classical BSD semantics:
>>        there is no interaction between the types  of  lock  placed  by
>>        flock()  and  fcntl(2),  and  flock() does not detect deadlock.
>>        (Note, however, that on some modern BSDs, flock() and  fcntl(2)
>>        locks do interact with one another.)
>>
>>        In Linux kernels up to 2.6.11, flock() does not lock files over
>>        NFS (i.e., the scope of locks was limited to the local system).
>>        Instead,  one could use fcntl(2) byte-range locking, which does
>>        work over NFS, given a sufficiently recent version of Linux and
>>        a  server  which  supports  locking.   Since  Linux 2.6.12, NFS
>>        clients support flock() locks by emulating them  as  byte-range
>>        locks on the entire file.  This means that fcntl(2) and flock()
>>        locks do interact with  one  another  over  NFS.   Since  Linux
>>        2.6.37,  the  kernel  supports a compatibility mode that allows
>>        flock() locks (and also  fcntl(2)  byte  region  locks)  to  be
>>        treated  as  local; see the discussion of the local_lock option
>>        in nfs(5).
>> ?
>
> That seems to cover it quite well - thanks.

Thanks for checking it.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> wrote:
> > On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
> > <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> [Trimming some folk from CC, and adding various NFS people]
> >>
> >> On 04/27/2014 06:51 AM, NeilBrown wrote:
> >>
> >> [...]
> >>
> >> > Note to Michael: The text
> >> >    flock() does not lock files over NFS.
> >> > in flock(2) is no longer accurate.  The reality is ... complex.
> >> > See nfs(5), and search for "local_lock".
> >>
> >> Ahhh -- I see:
> >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
> >>
> >> Thanks for the heads up.
> >>
> >> Just in general, it would be great if the flock(2) and fcntl(2) man pages
> >> contained correct details for NFS, of course. So, for example, if there
> >> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
> >> to add those to the fcntl(2) man page.
> >
> > The only peculiarities I can think of are:
> >  - With NFS, locking or unlocking a region forces a flush of any cached data
> >    for that file (or maybe for the region of the file).  I'm not sure if this
> >    is worth mentioning.
> 
> I agree that it's probably not necessary to mention.
> 
> >  - With NFSv4 the client can lose a lock if it is out of contact with the
> >    server for a period of time.  When this happens, any IO to the file by a
> >    process which "thinks" it holds a lock will fail until that process closes
> >    and re-opens the file.
> >    This behaviour is since 3.12.  Prior to that the client might lose and
> >    regain the lock without ever knowing thus potentially risking corruption
> >    (but only if client and server lost contact for an extended period).
> 
> Do you have a pointer for that commit to 3.12?
> 

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d

did most of the work while  the subsequent commit

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd

changed some details, added some documentation, and inverted the default
behaviour.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] locks: rename file-private locks to file-description locks

[Jeff Layton]

[-- Attachment #1: Type: text/plain, Size: 6372 bytes --]

On Sun, 27 Apr 2014 14:51:25 +1000
NeilBrown <neilb@suse.de> wrote:

> On Tue, 22 Apr 2014 06:54:36 +0200 "Michael Kerrisk (man-pages)"
> <mtk.manpages@gmail.com> wrote:
> 
> > On 04/21/2014 11:15 PM, Stefan (metze) Metzmacher wrote:
> > > Am 21.04.2014 21:55, schrieb Jeff Layton:
> > >> On Mon, 21 Apr 2014 21:39:12 +0200
> > >> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
> > >>
> > >>> On 04/21/2014 08:46 PM, Rich Felker wrote:
> > >>>> On Mon, Apr 21, 2014 at 08:32:44PM +0200, Michael Kerrisk (man-pages) wrote:
> > >>>>> On 04/21/2014 06:10 PM, Rich Felker wrote:
> > >>>>>> I'm well aware of that. The problem is that the proposed API is using
> > >>>>>> the two-letter abbreviation FD, which ALWAYS means file descriptor and
> > >>>>>> NEVER means file description (in existing usage) to mean file
> > >>>>>> description. That's what's wrong.
> > >>>>>
> > >>>>> So, can you *please* answer this question: what do you call (i.e., 
> > >>>>> what  everyday technical language term do use for) the thing
> > >>>>> that sits between a file descriptor and an i-node? 
> > >>>>>
> > >>>>> (Please don't say 'struct file' -- that is not is an implementation 
> > >>>>> detail, and does not qualify as the kind of term that I could use 
> > >>>>> when documenting this feature in man pages.)
> > >>>>
> > >>>> "Open file description".
> > >>>
> > >>> Oh! I didn't realize we agreed :-).
> > >>>
> > >>>>> POSIX uses (or invented, I am not sure which) the term file description
> > >>>>> for a good reason: it is unambiguous, and therefore precise. I do agree
> > >>>>> that there's a risk of confusion between 'open file descriptor" and 
> > >>>>> 'and file description'--it's the same kind of risk as between English 
> > >>>>> terms such as 'arbitrator' and 'arbitration' (and any number of other
> > >>>>> examples), and as language speakers we deal with this every day.
> > >>>>
> > >>>> There's not a problem when the full word is used. On the other hand,
> > >>>> if you use "arb" as an abbreviation for "arbitration" in a context
> > >>>> where it was already universally understood as meaning "arbitrator",
> > >>>> that would be a big problem.
> > >>>>
> > >>>> Likewise the problem here isn't that "open file description" is a bad
> > >>>> term. It's that using "FD" to mean "[open] file description" is
> > >>>> utterly confusing, even moreso than just making up a new completely
> > >>>> random word.
> > >>>
> > >>> Ohh -- I had thought you a problem not just with "FD" but also
> > >>> "(open) file description".
> > >>>
> > >>>>>>> 2) The new API constants (F_SETLKP, F_SETLKPW, F_GETLKP) have names
> > >>>>>>>    that are visually very close to the traditional POSIX lock names 
> > >>>>>>>    (F_SETLK, F_SETLKW, F_GETLK). That's an accident waiting to happen
> > >>>>>>>    when someone mistypes in code and/or misses such a misttyping
> > >>>>>>>    when reading code. That really must be fixed.
> > >>>>>>
> > >>>>>> I agree, but I don't think making it worse is a solution.
> > >>>>>
> > >>>>> I don't agree that it's making it worse. The real problem here is 
> > >>>>> that people use no good unambiguous term for the thing between a file
> > >>>>> descriptor and an inode. POSIX provides us with a solution that may
> > >>>>> not seem perfect, but it is unambiguous, and I think it might feel
> > >>>>> more comfortable if we used it often enough.
> > >>>>
> > >>>> I would like to see it used more too, and in particular, I think it
> > >>>> belongs in the documentation for these new locking interfaces. But
> > >>>> that still doesn't answer the question of what to call them (the
> > >>>> macros) unless you want:
> > >>>>
> > >>>> F_OPEN_FILE_DESCRIPTION_GETLK
> > >>>> F_OPEN_FILE_DESCRIPTION_SETLK
> > >>>> F_OPEN_FILE_DESCRIPTION_SETLKW
> > >>>
> > >>> Or just 'F_OFD_*'?
> > >>>
> > >>>> Perhaps "OP" (for open-private, i.e. private to the particular open)
> > >>>> would be a sensible choice; OTOH people are likely to misread it as
> > >>>> OPeration. The general principle I have in mind though is that it
> > >>>> might be nice to highlight the word "open" in "open file description"
> > >>>
> > >>> (Fair enough.)
> > >>>
> > >>>> since it (1) contrasts with file descriptor, despite file descriptors
> > >>>> also dealing with open files, and (2) contrasts well with legacy fcntl
> > >>>> locks, which are (this is the whole bug) associated with the
> > >>>> underlying file and not the open file description.
> > >>>
> > >>> Makes sense to me. (We are in more agreement that I realized.)
> > >>>
> > >>> Cheers,
> > >>>
> > >>> Michael
> > >>>
> > >>>
> > >>>
> > >>
> > >> So the motion is to call them "open file description locks" and change
> > >> the macros to read *_OFD_*. Does anyone object?
> > > 
> > > Works fine for me...
> > 
> > And works for me.
> > 
> > 
> 
> I think the word "open" is important here.
> I find that "description" is not a word I would have every thought was
> relevant here - it is obviously too long since I have read the man pages.
> 
> I would prefer
>   per-open locks
> which are contrasted with
>   per-process locks
> 
> An alternative might be "flock-like" as locks created with "flock" have
> exactly the property we are trying to describe.  Reading the man page for
> "flock" then suggests "open file table entry locks" which is even more of a
> mouthful.  "oftel" is pronounceable though.  Then we could talk about "oftel
> locks" in the same sentence as "pin numbers" and "RAM memory".
> 
> But maybe I came too late to this party, and the boat has sailed?
> 
> Note to Michael: The text
>    flock() does not lock files over NFS.
> in flock(2) is no longer accurate.  The reality is ... complex.
> See nfs(5), and search for "local_lock".
> 
> 
> NeilBrown
> 
> (I don't care what the macros are - XX would work as well any anything else
> for me).

I sent a pull request to Linus last week and he merged the patches over
the weekend. So yes, this ship has sailed unless someone feels strongly
enough about it to roll up the patches to change it.

I think we'll probably come to know these as "O.F.D. locks" over the long
run. Not a wonderful name, but I can live with it.

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] locks: rename file-private locks to file-description locks

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Mon, 28 Apr 2014 06:23:21 -0400 Jeff Layton <jlayton@redhat.com> wrote:


> I sent a pull request to Linus last week and he merged the patches over
> the weekend. So yes, this ship has sailed unless someone feels strongly
> enough about it to roll up the patches to change it.
> 
> I think we'll probably come to know these as "O.F.D. locks" over the long
> run. Not a wonderful name, but I can live with it.

I've managed to train my brain that "of_" means "device tree".  Coping with
"ofd" meaning "per-open" should be too hard.

:-)

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Michael Kerrisk (man-pages)]

On 04/27/2014 11:28 PM, NeilBrown wrote:
> On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
> <mtk.manpages@gmail.com> wrote:
> 
>> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb@suse.de> wrote:
>>> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
>>> <mtk.manpages@gmail.com> wrote:
>>>
>>>> [Trimming some folk from CC, and adding various NFS people]
>>>>
>>>> On 04/27/2014 06:51 AM, NeilBrown wrote:
>>>>
>>>> [...]
>>>>
>>>>> Note to Michael: The text
>>>>>    flock() does not lock files over NFS.
>>>>> in flock(2) is no longer accurate.  The reality is ... complex.
>>>>> See nfs(5), and search for "local_lock".
>>>>
>>>> Ahhh -- I see:
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
>>>>
>>>> Thanks for the heads up.
>>>>
>>>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
>>>> contained correct details for NFS, of course. So, for example, if there
>>>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
>>>> to add those to the fcntl(2) man page.
>>>
>>> The only peculiarities I can think of are:
>>>  - With NFS, locking or unlocking a region forces a flush of any cached data
>>>    for that file (or maybe for the region of the file).  I'm not sure if this
>>>    is worth mentioning.
>>
>> I agree that it's probably not necessary to mention.
>>
>>>  - With NFSv4 the client can lose a lock if it is out of contact with the
>>>    server for a period of time.  When this happens, any IO to the file by a
>>>    process which "thinks" it holds a lock will fail until that process closes
>>>    and re-opens the file.
>>>    This behaviour is since 3.12.  Prior to that the client might lose and
>>>    regain the lock without ever knowing thus potentially risking corruption
>>>    (but only if client and server lost contact for an extended period).
>>
>> Do you have a pointer for that commit to 3.12?
>>
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d
> 
> did most of the work while  the subsequent commit
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd
> 
> changed some details, added some documentation, and inverted the default
> behaviour.

Thanks for that detail. What do you think of the following text for the 
fcntl(2) man page:

       Before  Linux 3.12, if an NFS client is out of contact with the
       server for a period of time, it might lose and  regain  a  lock
       without  ever  being  aware  of the fact.  This scenario poten‐
       tially risks  data  corruption,  since  another  process  might
       acquire  a lock in the intervening period and perform file I/O.
       Since Linux 3.12, if the client loses contact with the  server,
       any I/O to the file by a process which "thinks" it holds a lock
       will fail until that process closes and reopens  the  file.   A
       kernel  parameter,  nfs.recover_lost_locks,  can be set to 1 to
       obtain the pre-3.12 behavior, whereby the client  will  attempt
       to  recover  lost  locks when contact is reestablished with the
       server.  Because of the attendant risk of data corruption, this
       parameter defaults to 0 (disabled).

?

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4571 bytes --]

On Tue, 29 Apr 2014 11:07:16 +0200 "Michael Kerrisk (man-pages)"
<mtk.manpages@gmail.com> wrote:

> On 04/27/2014 11:28 PM, NeilBrown wrote:
> > On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
> > <mtk.manpages@gmail.com> wrote:
> > 
> >> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb@suse.de> wrote:
> >>> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
> >>> <mtk.manpages@gmail.com> wrote:
> >>>
> >>>> [Trimming some folk from CC, and adding various NFS people]
> >>>>
> >>>> On 04/27/2014 06:51 AM, NeilBrown wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>> Note to Michael: The text
> >>>>>    flock() does not lock files over NFS.
> >>>>> in flock(2) is no longer accurate.  The reality is ... complex.
> >>>>> See nfs(5), and search for "local_lock".
> >>>>
> >>>> Ahhh -- I see:
> >>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
> >>>>
> >>>> Thanks for the heads up.
> >>>>
> >>>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
> >>>> contained correct details for NFS, of course. So, for example, if there
> >>>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
> >>>> to add those to the fcntl(2) man page.
> >>>
> >>> The only peculiarities I can think of are:
> >>>  - With NFS, locking or unlocking a region forces a flush of any cached data
> >>>    for that file (or maybe for the region of the file).  I'm not sure if this
> >>>    is worth mentioning.
> >>
> >> I agree that it's probably not necessary to mention.
> >>
> >>>  - With NFSv4 the client can lose a lock if it is out of contact with the
> >>>    server for a period of time.  When this happens, any IO to the file by a
> >>>    process which "thinks" it holds a lock will fail until that process closes
> >>>    and re-opens the file.
> >>>    This behaviour is since 3.12.  Prior to that the client might lose and
> >>>    regain the lock without ever knowing thus potentially risking corruption
> >>>    (but only if client and server lost contact for an extended period).
> >>
> >> Do you have a pointer for that commit to 3.12?
> >>
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d
> > 
> > did most of the work while  the subsequent commit
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd
> > 
> > changed some details, added some documentation, and inverted the default
> > behaviour.
> 
> Thanks for that detail. What do you think of the following text for the 
> fcntl(2) man page:
> 
>        Before  Linux 3.12, if an NFS client is out of contact with the
>        server for a period of time, it might lose and  regain  a  lock
>        without  ever  being  aware  of the fact.  This scenario poten‐
>        tially risks  data  corruption,  since  another  process  might
>        acquire  a lock in the intervening period and perform file I/O.
>        Since Linux 3.12, if the client loses contact with the  server,
>        any I/O to the file by a process which "thinks" it holds a lock
>        will fail until that process closes and reopens  the  file.   A
>        kernel  parameter,  nfs.recover_lost_locks,  can be set to 1 to
>        obtain the pre-3.12 behavior, whereby the client  will  attempt
>        to  recover  lost  locks when contact is reestablished with the
>        server.  Because of the attendant risk of data corruption, this
>        parameter defaults to 0 (disabled).
> 

Mostly good.

I'm just a little concerned about "if the client loses contact with the
server" in the middle there.  It is no longer qualified and it isn't clear
that the "for a period of time" qualification still applied.  And we should
probably quantify the period of time - which defaults to 90 seconds.
I don't remember just now the difference between
   /proc/fs/nfsd/nfsv4{lease,grace}time
but this 90 seconds is one of those.

Also this is NFSv4 specific.  With NFSv3 the failure mode is the reverse.  If
the server loses contact with a client then any lock stays in place
indefinitely ("why can't I read my mail"... I remember it well).

  Before Linux 3.12, if an NFSv4 client loses contact with the server
  (defined as more than 90 seconds with no communication), it might lose
  and regain ....

Just changing that bit should cover it I think.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Michael Kerrisk (man-pages)]

On 04/29/2014 11:24 AM, NeilBrown wrote:
> On Tue, 29 Apr 2014 11:07:16 +0200 "Michael Kerrisk (man-pages)"
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>> On 04/27/2014 11:28 PM, NeilBrown wrote:
>>> On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
>>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> wrote:
>>>>> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
>>>>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>
>>>>>> [Trimming some folk from CC, and adding various NFS people]
>>>>>>
>>>>>> On 04/27/2014 06:51 AM, NeilBrown wrote:
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>> Note to Michael: The text
>>>>>>>    flock() does not lock files over NFS.
>>>>>>> in flock(2) is no longer accurate.  The reality is ... complex.
>>>>>>> See nfs(5), and search for "local_lock".
>>>>>>
>>>>>> Ahhh -- I see:
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
>>>>>>
>>>>>> Thanks for the heads up.
>>>>>>
>>>>>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
>>>>>> contained correct details for NFS, of course. So, for example, if there
>>>>>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
>>>>>> to add those to the fcntl(2) man page.
>>>>>
>>>>> The only peculiarities I can think of are:
>>>>>  - With NFS, locking or unlocking a region forces a flush of any cached data
>>>>>    for that file (or maybe for the region of the file).  I'm not sure if this
>>>>>    is worth mentioning.
>>>>
>>>> I agree that it's probably not necessary to mention.
>>>>
>>>>>  - With NFSv4 the client can lose a lock if it is out of contact with the
>>>>>    server for a period of time.  When this happens, any IO to the file by a
>>>>>    process which "thinks" it holds a lock will fail until that process closes
>>>>>    and re-opens the file.
>>>>>    This behaviour is since 3.12.  Prior to that the client might lose and
>>>>>    regain the lock without ever knowing thus potentially risking corruption
>>>>>    (but only if client and server lost contact for an extended period).
>>>>
>>>> Do you have a pointer for that commit to 3.12?
>>>>
>>>
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d
>>>
>>> did most of the work while  the subsequent commit
>>>
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd
>>>
>>> changed some details, added some documentation, and inverted the default
>>> behaviour.
>>
>> Thanks for that detail. What do you think of the following text for the 
>> fcntl(2) man page:
>>
>>        Before  Linux 3.12, if an NFS client is out of contact with the
>>        server for a period of time, it might lose and  regain  a  lock
>>        without  ever  being  aware  of the fact.  This scenario poten‐
>>        tially risks  data  corruption,  since  another  process  might
>>        acquire  a lock in the intervening period and perform file I/O.
>>        Since Linux 3.12, if the client loses contact with the  server,
>>        any I/O to the file by a process which "thinks" it holds a lock
>>        will fail until that process closes and reopens  the  file.   A
>>        kernel  parameter,  nfs.recover_lost_locks,  can be set to 1 to
>>        obtain the pre-3.12 behavior, whereby the client  will  attempt
>>        to  recover  lost  locks when contact is reestablished with the
>>        server.  Because of the attendant risk of data corruption, this
>>        parameter defaults to 0 (disabled).
>>
> 
> Mostly good.
> 
> I'm just a little concerned about "if the client loses contact with the
> server" in the middle there.  It is no longer qualified and it isn't clear
> that the "for a period of time" qualification still applied.  And we should
> probably quantify the period of time - which defaults to 90 seconds.
> I don't remember just now the difference between
>    /proc/fs/nfsd/nfsv4{lease,grace}time
> but this 90 seconds is one of those.
> 
> Also this is NFSv4 specific.  With NFSv3 the failure mode is the reverse.  If
> the server loses contact with a client then any lock stays in place
> indefinitely ("why can't I read my mail"... I remember it well).
> 
>   Before Linux 3.12, if an NFSv4 client loses contact with the server
>   (defined as more than 90 seconds with no communication), it might lose
>   and regain ....

Thanks, Neil. Changed as you suggest. I'd quite like to mention
which of /proc/fs/nfsd/nfsv4{lease,grace}time is relevant here. I had a 
quick scan, but could not determine it with complete confidence. My suspicion, 
looking at fs/lockd/svcproc.c and fs/lockd/grace.c::locks_in_grace()
is that it is /proc/fs/nfsd/nfsv4gracetime that is relevant here. Can anyone
confirm?

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Jeff Layton]

On Tue, 29 Apr 2014 11:53:40 +0200
"Michael Kerrisk (man-pages)" <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On 04/29/2014 11:24 AM, NeilBrown wrote:
> > On Tue, 29 Apr 2014 11:07:16 +0200 "Michael Kerrisk (man-pages)"
> > <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> >> On 04/27/2014 11:28 PM, NeilBrown wrote:
> >>> On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
> >>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>
> >>>> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> wrote:
> >>>>> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
> >>>>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>>
> >>>>>> [Trimming some folk from CC, and adding various NFS people]
> >>>>>>
> >>>>>> On 04/27/2014 06:51 AM, NeilBrown wrote:
> >>>>>>
> >>>>>> [...]
> >>>>>>
> >>>>>>> Note to Michael: The text
> >>>>>>>    flock() does not lock files over NFS.
> >>>>>>> in flock(2) is no longer accurate.  The reality is ... complex.
> >>>>>>> See nfs(5), and search for "local_lock".
> >>>>>>
> >>>>>> Ahhh -- I see:
> >>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
> >>>>>>
> >>>>>> Thanks for the heads up.
> >>>>>>
> >>>>>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
> >>>>>> contained correct details for NFS, of course. So, for example, if there
> >>>>>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
> >>>>>> to add those to the fcntl(2) man page.
> >>>>>
> >>>>> The only peculiarities I can think of are:
> >>>>>  - With NFS, locking or unlocking a region forces a flush of any cached data
> >>>>>    for that file (or maybe for the region of the file).  I'm not sure if this
> >>>>>    is worth mentioning.
> >>>>
> >>>> I agree that it's probably not necessary to mention.
> >>>>
> >>>>>  - With NFSv4 the client can lose a lock if it is out of contact with the
> >>>>>    server for a period of time.  When this happens, any IO to the file by a
> >>>>>    process which "thinks" it holds a lock will fail until that process closes
> >>>>>    and re-opens the file.
> >>>>>    This behaviour is since 3.12.  Prior to that the client might lose and
> >>>>>    regain the lock without ever knowing thus potentially risking corruption
> >>>>>    (but only if client and server lost contact for an extended period).
> >>>>
> >>>> Do you have a pointer for that commit to 3.12?
> >>>>
> >>>
> >>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d
> >>>
> >>> did most of the work while  the subsequent commit
> >>>
> >>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd
> >>>
> >>> changed some details, added some documentation, and inverted the default
> >>> behaviour.
> >>
> >> Thanks for that detail. What do you think of the following text for the 
> >> fcntl(2) man page:
> >>
> >>        Before  Linux 3.12, if an NFS client is out of contact with the
> >>        server for a period of time, it might lose and  regain  a  lock
> >>        without  ever  being  aware  of the fact.  This scenario poten‐
> >>        tially risks  data  corruption,  since  another  process  might
> >>        acquire  a lock in the intervening period and perform file I/O.
> >>        Since Linux 3.12, if the client loses contact with the  server,
> >>        any I/O to the file by a process which "thinks" it holds a lock
> >>        will fail until that process closes and reopens  the  file.   A
> >>        kernel  parameter,  nfs.recover_lost_locks,  can be set to 1 to
> >>        obtain the pre-3.12 behavior, whereby the client  will  attempt
> >>        to  recover  lost  locks when contact is reestablished with the
> >>        server.  Because of the attendant risk of data corruption, this
> >>        parameter defaults to 0 (disabled).
> >>
> > 
> > Mostly good.
> > 
> > I'm just a little concerned about "if the client loses contact with the
> > server" in the middle there.  It is no longer qualified and it isn't clear
> > that the "for a period of time" qualification still applied.  And we should
> > probably quantify the period of time - which defaults to 90 seconds.
> > I don't remember just now the difference between
> >    /proc/fs/nfsd/nfsv4{lease,grace}time
> > but this 90 seconds is one of those.
> > 
> > Also this is NFSv4 specific.  With NFSv3 the failure mode is the reverse.  If
> > the server loses contact with a client then any lock stays in place
> > indefinitely ("why can't I read my mail"... I remember it well).
> > 
> >   Before Linux 3.12, if an NFSv4 client loses contact with the server
> >   (defined as more than 90 seconds with no communication), it might lose
> >   and regain ....
> 
> Thanks, Neil. Changed as you suggest. I'd quite like to mention
> which of /proc/fs/nfsd/nfsv4{lease,grace}time is relevant here. I had a 
> quick scan, but could not determine it with complete confidence. My suspicion, 
> looking at fs/lockd/svcproc.c and fs/lockd/grace.c::locks_in_grace()
> is that it is /proc/fs/nfsd/nfsv4gracetime that is relevant here. Can anyone
> confirm?
> 

The difference here is subtle. The gracetime is how long after a reboot
should knfsd allow clients to reclaim state (and deny the creation of
new locks and opens). The leasetime is how long the NFSv4 lease period
is. There is a relationship between the two that's illustrated in the
comments above write_gracetime:

/**
 * write_gracetime - Set or report current NFSv4 grace period time
 *
 * As above, but sets the time of the NFSv4 grace period.
 *
 * Note this should never be set to less than the *previous*
 * lease-period time, but we don't try to enforce this.  (In the common
 * case (a new boot), we don't know what the previous lease time was
 * anyway.)
 */

The value you're interested in here is the nfsv4leasetime. If the
client doesn't renew its lease within that period, then it's subject to
the server giving up on it and dropping any state that it holds on that
clients' behalf.

Note that this is not a firm timeout. The server runs a job
periodically to clean out expired stateful objects, and it's likely
that there is some time (maybe even up to another whole lease period)
between when the timeout expires and the job actually runs. If the
client gets a RENEW in there within that window, its lease will be
renewed and its state preserved.

Also note that all of the above just applies to the Linux knfsd. There
are many other servers in the field and they have different rules for
dropping state held by clients that have gone AWOL.

-- 
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: flock() and NFS [Was: Re: [PATCH] locks: rename file-private locks to file-description locks]

[Michael Kerrisk (man-pages)]

On 04/29/2014 01:34 PM, Jeff Layton wrote:
> On Tue, 29 Apr 2014 11:53:40 +0200
> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:
> 
>> On 04/29/2014 11:24 AM, NeilBrown wrote:
>>> On Tue, 29 Apr 2014 11:07:16 +0200 "Michael Kerrisk (man-pages)"
>>> <mtk.manpages@gmail.com> wrote:
>>>
>>>> On 04/27/2014 11:28 PM, NeilBrown wrote:
>>>>> On Sun, 27 Apr 2014 13:11:33 +0200 "Michael Kerrisk (man-pages)"
>>>>> <mtk.manpages@gmail.com> wrote:
>>>>>
>>>>>> On Sun, Apr 27, 2014 at 12:04 PM, NeilBrown <neilb@suse.de> wrote:
>>>>>>> On Sun, 27 Apr 2014 11:16:02 +0200 "Michael Kerrisk (man-pages)"
>>>>>>> <mtk.manpages@gmail.com> wrote:
>>>>>>>
>>>>>>>> [Trimming some folk from CC, and adding various NFS people]
>>>>>>>>
>>>>>>>> On 04/27/2014 06:51 AM, NeilBrown wrote:
>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>> Note to Michael: The text
>>>>>>>>>    flock() does not lock files over NFS.
>>>>>>>>> in flock(2) is no longer accurate.  The reality is ... complex.
>>>>>>>>> See nfs(5), and search for "local_lock".
>>>>>>>>
>>>>>>>> Ahhh -- I see:
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eebde23223aeb0ad2d9e3be6590ff8bbfab0fc2
>>>>>>>>
>>>>>>>> Thanks for the heads up.
>>>>>>>>
>>>>>>>> Just in general, it would be great if the flock(2) and fcntl(2) man pages
>>>>>>>> contained correct details for NFS, of course. So, for example, if there
>>>>>>>> are any current gotchas for NFS and fcntl() byte-range locking, I'd like
>>>>>>>> to add those to the fcntl(2) man page.
>>>>>>>
>>>>>>> The only peculiarities I can think of are:
>>>>>>>  - With NFS, locking or unlocking a region forces a flush of any cached data
>>>>>>>    for that file (or maybe for the region of the file).  I'm not sure if this
>>>>>>>    is worth mentioning.
>>>>>>
>>>>>> I agree that it's probably not necessary to mention.
>>>>>>
>>>>>>>  - With NFSv4 the client can lose a lock if it is out of contact with the
>>>>>>>    server for a period of time.  When this happens, any IO to the file by a
>>>>>>>    process which "thinks" it holds a lock will fail until that process closes
>>>>>>>    and re-opens the file.
>>>>>>>    This behaviour is since 3.12.  Prior to that the client might lose and
>>>>>>>    regain the lock without ever knowing thus potentially risking corruption
>>>>>>>    (but only if client and server lost contact for an extended period).
>>>>>>
>>>>>> Do you have a pointer for that commit to 3.12?
>>>>>>
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ef1820f9be27b6ad158f433ab38002ab8131db4d
>>>>>
>>>>> did most of the work while  the subsequent commit
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f6de7a39c181dfb8a2c534661a53c73afb3081cd
>>>>>
>>>>> changed some details, added some documentation, and inverted the default
>>>>> behaviour.
>>>>
>>>> Thanks for that detail. What do you think of the following text for the 
>>>> fcntl(2) man page:
>>>>
>>>>        Before  Linux 3.12, if an NFS client is out of contact with the
>>>>        server for a period of time, it might lose and  regain  a  lock
>>>>        without  ever  being  aware  of the fact.  This scenario poten‐
>>>>        tially risks  data  corruption,  since  another  process  might
>>>>        acquire  a lock in the intervening period and perform file I/O.
>>>>        Since Linux 3.12, if the client loses contact with the  server,
>>>>        any I/O to the file by a process which "thinks" it holds a lock
>>>>        will fail until that process closes and reopens  the  file.   A
>>>>        kernel  parameter,  nfs.recover_lost_locks,  can be set to 1 to
>>>>        obtain the pre-3.12 behavior, whereby the client  will  attempt
>>>>        to  recover  lost  locks when contact is reestablished with the
>>>>        server.  Because of the attendant risk of data corruption, this
>>>>        parameter defaults to 0 (disabled).
>>>>
>>>
>>> Mostly good.
>>>
>>> I'm just a little concerned about "if the client loses contact with the
>>> server" in the middle there.  It is no longer qualified and it isn't clear
>>> that the "for a period of time" qualification still applied.  And we should
>>> probably quantify the period of time - which defaults to 90 seconds.
>>> I don't remember just now the difference between
>>>    /proc/fs/nfsd/nfsv4{lease,grace}time
>>> but this 90 seconds is one of those.
>>>
>>> Also this is NFSv4 specific.  With NFSv3 the failure mode is the reverse.  If
>>> the server loses contact with a client then any lock stays in place
>>> indefinitely ("why can't I read my mail"... I remember it well).
>>>
>>>   Before Linux 3.12, if an NFSv4 client loses contact with the server
>>>   (defined as more than 90 seconds with no communication), it might lose
>>>   and regain ....
>>
>> Thanks, Neil. Changed as you suggest. I'd quite like to mention
>> which of /proc/fs/nfsd/nfsv4{lease,grace}time is relevant here. I had a 
>> quick scan, but could not determine it with complete confidence. My suspicion, 
>> looking at fs/lockd/svcproc.c and fs/lockd/grace.c::locks_in_grace()
>> is that it is /proc/fs/nfsd/nfsv4gracetime that is relevant here. Can anyone
>> confirm?
>>
> 
> The difference here is subtle. The gracetime is how long after a reboot
> should knfsd allow clients to reclaim state (and deny the creation of
> new locks and opens). The leasetime is how long the NFSv4 lease period
> is. There is a relationship between the two that's illustrated in the
> comments above write_gracetime:
> 
> /**
>  * write_gracetime - Set or report current NFSv4 grace period time
>  *
>  * As above, but sets the time of the NFSv4 grace period.
>  *
>  * Note this should never be set to less than the *previous*
>  * lease-period time, but we don't try to enforce this.  (In the common
>  * case (a new boot), we don't know what the previous lease time was
>  * anyway.)
>  */
> 
> The value you're interested in here is the nfsv4leasetime. If the
> client doesn't renew its lease within that period, then it's subject to
> the server giving up on it and dropping any state that it holds on that
> clients' behalf.
> 
> Note that this is not a firm timeout. The server runs a job
> periodically to clean out expired stateful objects, and it's likely
> that there is some time (maybe even up to another whole lease period)
> between when the timeout expires and the job actually runs. If the
> client gets a RENEW in there within that window, its lease will be
> renewed and its state preserved.
> 
> Also note that all of the above just applies to the Linux knfsd. There
> are many other servers in the field and they have different rules for
> dropping state held by clients that have gone AWOL.

Thanks for the detailed explanation, Jeff. I've updated the draft text to
mention nfsv4gracetime. I won't add the subtleties you mention above
(but they'll go into the commit message).

The text is now:

   Record locking and NFS
       Before  Linux  3.12,  if an NFSv4 client loses contact with the
       server for a period of time (defined as more  than  90  seconds
       with no communication), it might lose and regain a lock without
       ever being aware of the fact.  (The period of time after  which
       contact is assumed lost is defined by /proc/fs/nfsd/nfsv4lease‐
       time, which expresses the period in seconds.  The default value
       for  this  file  is  90.)  This scenario potentially risks data
       corruption, since another process might acquire a lock  in  the
       intervening period and perform file I/O.

       Since  Linux  3.12,  if  an NFSv4 client loses contact with the
       server, any I/O to the file by  a  process  which  "thinks"  it
       holds  a  lock  will fail until that process closes and reopens
       the file.  A kernel parameter, nfs.recover_lost_locks,  can  be
       set  to  1  to obtain the pre-3.12 behavior, whereby the client
       will attempt to recover lost locks  when  contact  is  reestab‐
       lished  with the server.  Because of the attendant risk of data
       corruption, this parameter defaults to 0 (disabled).

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/