Re: F_SETOWN_EX and F_GETLK64 conflict

Re: F_SETOWN_EX and F_GETLK64 conflict

[Peter Zijlstra]

On Sun, 2009-10-04 at 12:35 +0000, Joseph S. Myers wrote:
> In asm-generic/fcntl.h, F_SETOWN_EX and F_GETLK64 both have value 12, and 
> F_GETOWN_EX and F_SETLK64 both have value 13.  I don't see how this is 
> going to work correctly.  See 
> <http://sourceware.org/ml/libc-ports/2009-10/msg00013.html>.


Ugh,.. yeah, non obvious collision that.

How about something like:

---
 arch/alpha/include/asm/fcntl.h |    4 ++--
 include/asm-generic/fcntl.h    |   19 +++++++++++--------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h
index e42823e..73126e4 100644
--- a/arch/alpha/include/asm/fcntl.h
+++ b/arch/alpha/include/asm/fcntl.h
@@ -26,8 +26,8 @@
 #define F_GETOWN	6	/*  for sockets. */
 #define F_SETSIG	10	/*  for sockets. */
 #define F_GETSIG	11	/*  for sockets. */
-#define F_SETOWN_EX	12
-#define F_GETOWN_EX	13
+#define F_SETOWN_EX	15
+#define F_GETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		1
diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h
index 0c3dd86..cd2d789 100644
--- a/include/asm-generic/fcntl.h
+++ b/include/asm-generic/fcntl.h
@@ -73,9 +73,18 @@
 #define F_SETSIG	10	/* for sockets. */
 #define F_GETSIG	11	/* for sockets. */
 #endif
+
+#ifndef CONFIG_64BIT
+#ifndef F_GETLK64
+#define F_GETLK64	12	/*  using 'struct flock64' */
+#define F_SETLK64	13
+#define F_SETLKW64	14
+#endif
+#endif
+
 #ifndef F_SETOWN_EX
-#define F_SETOWN_EX	12
-#define F_GETOWN_EX	13
+#define F_SETOWN_EX	15
+#define F_GETOWN_EX	16
 #endif
 
 #define F_OWNER_TID	0
@@ -139,12 +148,6 @@ struct flock {
 
 #ifndef CONFIG_64BIT
 
-#ifndef F_GETLK64
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-#endif
-
 #ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD

Re: F_SETOWN_EX and F_GETLK64 conflict

[Andreas Schwab]

Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org> writes:

> On Sun, 2009-10-04 at 12:35 +0000, Joseph S. Myers wrote:
>> In asm-generic/fcntl.h, F_SETOWN_EX and F_GETLK64 both have value 12, and 
>> F_GETOWN_EX and F_SETLK64 both have value 13.  I don't see how this is 
>> going to work correctly.  See 
>> <http://sourceware.org/ml/libc-ports/2009-10/msg00013.html>.
>
>
> Ugh,.. yeah, non obvious collision that.
>
> How about something like:

That really needs to be fixed before 2.6.32.

Andreas.

-- 
Andreas Schwab, schwab-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: F_SETOWN_EX and F_GETLK64 conflict

[Ulrich Drepper]

On Sun, Oct 4, 2009 at 14:30, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>  arch/alpha/include/asm/fcntl.h |    4 ++--
>  include/asm-generic/fcntl.h    |   19 +++++++++++--------
>  2 files changed, 13 insertions(+), 10 deletions(-)

This patch looks fine:

Acked-by: Ulrich Drepper <drepper@redhat.com>


Linus, can you please apply it in whatever form so that we know the
value of the constants?

Re: F_SETOWN_EX and F_GETLK64 conflict

[Stephen Rothwell]

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

Hi Peter,

On Sun, 04 Oct 2009 23:30:22 +0200 Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org> wrote:
>
> On Sun, 2009-10-04 at 12:35 +0000, Joseph S. Myers wrote:
> > In asm-generic/fcntl.h, F_SETOWN_EX and F_GETLK64 both have value 12, and 
> > F_GETOWN_EX and F_SETLK64 both have value 13.  I don't see how this is 
> > going to work correctly.  See 
> > <http://sourceware.org/ml/libc-ports/2009-10/msg00013.html>.
> 
> 
> Ugh,.. yeah, non obvious collision that.

This is why we have F_LINUX_SPECIFIC_BASE and use it in
include/linux/fcntl.h ... it should be used as a base for any new fcntl
types since they don't require backward compatibility.

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: F_SETOWN_EX and F_GETLK64 conflict

[Stephen Rothwell]

On Fri, 30 Oct 2009 16:13:20 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> This is why we have F_LINUX_SPECIFIC_BASE and use it in
> include/linux/fcntl.h ... it should be used as a base for any new fcntl
> types since they don't require backward compatibility.

Something like this (not even built, but pretty obvious):

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 30 Oct 2009 16:25:51 +1100
Subject: [PATCH] fcntl: use consistent valuse for F_SETOWN_EX/F_GETOWN_EX

This is what F_LINUX_SPECIFIC_BASE was meant for.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/alpha/include/asm/fcntl.h  |    2 --
 arch/parisc/include/asm/fcntl.h |    2 --
 include/asm-generic/fcntl.h     |   14 --------------
 include/linux/fcntl.h           |   12 ++++++++++++
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h
index e42823e..25da001 100644
--- a/arch/alpha/include/asm/fcntl.h
+++ b/arch/alpha/include/asm/fcntl.h
@@ -26,8 +26,6 @@
 #define F_GETOWN	6	/*  for sockets. */
 #define F_SETSIG	10	/*  for sockets. */
 #define F_GETSIG	11	/*  for sockets. */
-#define F_SETOWN_EX	12
-#define F_GETOWN_EX	13
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		1
diff --git a/arch/parisc/include/asm/fcntl.h b/arch/parisc/include/asm/fcntl.h
index 5f39d55..1e1c824 100644
--- a/arch/parisc/include/asm/fcntl.h
+++ b/arch/parisc/include/asm/fcntl.h
@@ -28,8 +28,6 @@
 #define F_SETOWN	12	/*  for sockets. */
 #define F_SETSIG	13	/*  for sockets. */
 #define F_GETSIG	14	/*  for sockets. */
-#define F_GETOWN_EX	15
-#define F_SETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		01
diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h
index 0c3dd86..980d6b5 100644
--- a/include/asm-generic/fcntl.h
+++ b/include/asm-generic/fcntl.h
@@ -73,20 +73,6 @@
 #define F_SETSIG	10	/* for sockets. */
 #define F_GETSIG	11	/* for sockets. */
 #endif
-#ifndef F_SETOWN_EX
-#define F_SETOWN_EX	12
-#define F_GETOWN_EX	13
-#endif
-
-#define F_OWNER_TID	0
-#define F_OWNER_PID	1
-#define F_OWNER_GID	2
-
-struct f_owner_ex {
-	int	type;
-	pid_t	pid;
-};
-
 /* for F_[GET|SET]FL */
 #define FD_CLOEXEC	1	/* actually anything with low bit set goes */
 
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 8603740..b34c41d 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -40,6 +40,18 @@
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
 
+#define F_SETOWN_EX	(F_LINUX_SPECIFIC_BASE + 7)
+#define F_GETOWN_EX	(F_LINUX_SPECIFIC_BASE + 8)
+
+#define F_OWNER_TID	0
+#define F_OWNER_PID	1
+#define F_OWNER_GID	2
+
+struct f_owner_ex {
+	int	type;
+	pid_t	pid;
+};
+
 #ifdef __KERNEL__
 
 #ifndef force_o_largefile
-- 
1.6.5.2


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: F_SETOWN_EX and F_GETLK64 conflict

[Stephen Rothwell]

Hi Andrew,

On Fri, 30 Oct 2009 16:29:06 +1100 Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org> wrote:
>
> On Fri, 30 Oct 2009 16:13:20 +1100 Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org> wrote:
> >
> > This is why we have F_LINUX_SPECIFIC_BASE and use it in
> > include/linux/fcntl.h ... it should be used as a base for any new fcntl
> > types since they don't require backward compatibility.
> 
> Something like this (not even built, but pretty obvious):

So I provide an alternative patch (which IMHO is a better one), I get no
comments, and 2 weeks later the original is now in Linus' tree ...

The new fcntl numbers that are being fixed here were only introduced
after v2.6.31, so there is no ABI breakage by changing them.

> From: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> Date: Fri, 30 Oct 2009 16:25:51 +1100
> Subject: [PATCH] fcntl: use consistent valuse for F_SETOWN_EX/F_GETOWN_EX
> 
> This is what F_LINUX_SPECIFIC_BASE was meant for.
> 
> Signed-off-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> ---
>  arch/alpha/include/asm/fcntl.h  |    2 --
>  arch/parisc/include/asm/fcntl.h |    2 --
>  include/asm-generic/fcntl.h     |   14 --------------
>  include/linux/fcntl.h           |   12 ++++++++++++
>  4 files changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h
> index e42823e..25da001 100644
> --- a/arch/alpha/include/asm/fcntl.h
> +++ b/arch/alpha/include/asm/fcntl.h
> @@ -26,8 +26,6 @@
>  #define F_GETOWN	6	/*  for sockets. */
>  #define F_SETSIG	10	/*  for sockets. */
>  #define F_GETSIG	11	/*  for sockets. */
> -#define F_SETOWN_EX	12
> -#define F_GETOWN_EX	13
>  
>  /* for posix fcntl() and lockf() */
>  #define F_RDLCK		1
> diff --git a/arch/parisc/include/asm/fcntl.h b/arch/parisc/include/asm/fcntl.h
> index 5f39d55..1e1c824 100644
> --- a/arch/parisc/include/asm/fcntl.h
> +++ b/arch/parisc/include/asm/fcntl.h
> @@ -28,8 +28,6 @@
>  #define F_SETOWN	12	/*  for sockets. */
>  #define F_SETSIG	13	/*  for sockets. */
>  #define F_GETSIG	14	/*  for sockets. */
> -#define F_GETOWN_EX	15
> -#define F_SETOWN_EX	16
>  
>  /* for posix fcntl() and lockf() */
>  #define F_RDLCK		01
> diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h
> index 0c3dd86..980d6b5 100644
> --- a/include/asm-generic/fcntl.h
> +++ b/include/asm-generic/fcntl.h
> @@ -73,20 +73,6 @@
>  #define F_SETSIG	10	/* for sockets. */
>  #define F_GETSIG	11	/* for sockets. */
>  #endif
> -#ifndef F_SETOWN_EX
> -#define F_SETOWN_EX	12
> -#define F_GETOWN_EX	13
> -#endif
> -
> -#define F_OWNER_TID	0
> -#define F_OWNER_PID	1
> -#define F_OWNER_GID	2
> -
> -struct f_owner_ex {
> -	int	type;
> -	pid_t	pid;
> -};
> -
>  /* for F_[GET|SET]FL */
>  #define FD_CLOEXEC	1	/* actually anything with low bit set goes */
>  
> diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> index 8603740..b34c41d 100644
> --- a/include/linux/fcntl.h
> +++ b/include/linux/fcntl.h
> @@ -40,6 +40,18 @@
>                                             unlinking file.  */
>  #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
>  
> +#define F_SETOWN_EX	(F_LINUX_SPECIFIC_BASE + 7)
> +#define F_GETOWN_EX	(F_LINUX_SPECIFIC_BASE + 8)
> +
> +#define F_OWNER_TID	0
> +#define F_OWNER_PID	1
> +#define F_OWNER_GID	2
> +
> +struct f_owner_ex {
> +	int	type;
> +	pid_t	pid;
> +};
> +
>  #ifdef __KERNEL__
>  
>  #ifndef force_o_largefile

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/

[PATCH] fcntl: use architecture independent values for new fcntl operations

[Stephen Rothwell]

This is what F_LINUX_SPECIFIC_BASE was meant for.

These values were only introduced during this release cycle, so it is
still early enough to get them right.

Signed-off-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
---
 arch/alpha/include/asm/fcntl.h  |    2 --
 arch/parisc/include/asm/fcntl.h |    2 --
 include/asm-generic/fcntl.h     |   14 --------------
 include/linux/fcntl.h           |   12 ++++++++++++
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h
index 73126e4..25da001 100644
--- a/arch/alpha/include/asm/fcntl.h
+++ b/arch/alpha/include/asm/fcntl.h
@@ -26,8 +26,6 @@
 #define F_GETOWN	6	/*  for sockets. */
 #define F_SETSIG	10	/*  for sockets. */
 #define F_GETSIG	11	/*  for sockets. */
-#define F_SETOWN_EX	15
-#define F_GETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		1
diff --git a/arch/parisc/include/asm/fcntl.h b/arch/parisc/include/asm/fcntl.h
index 5f39d55..1e1c824 100644
--- a/arch/parisc/include/asm/fcntl.h
+++ b/arch/parisc/include/asm/fcntl.h
@@ -28,8 +28,6 @@
 #define F_SETOWN	12	/*  for sockets. */
 #define F_SETSIG	13	/*  for sockets. */
 #define F_GETSIG	14	/*  for sockets. */
-#define F_GETOWN_EX	15
-#define F_SETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		01
diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h
index cd2d789..8ec0716 100644
--- a/include/asm-generic/fcntl.h
+++ b/include/asm-generic/fcntl.h
@@ -82,20 +82,6 @@
 #endif
 #endif
 
-#ifndef F_SETOWN_EX
-#define F_SETOWN_EX	15
-#define F_GETOWN_EX	16
-#endif
-
-#define F_OWNER_TID	0
-#define F_OWNER_PID	1
-#define F_OWNER_GID	2
-
-struct f_owner_ex {
-	int	type;
-	pid_t	pid;
-};
-
 /* for F_[GET|SET]FL */
 #define FD_CLOEXEC	1	/* actually anything with low bit set goes */
 
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 8603740..b34c41d 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -40,6 +40,18 @@
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
 
+#define F_SETOWN_EX	(F_LINUX_SPECIFIC_BASE + 7)
+#define F_GETOWN_EX	(F_LINUX_SPECIFIC_BASE + 8)
+
+#define F_OWNER_TID	0
+#define F_OWNER_PID	1
+#define F_OWNER_GID	2
+
+struct f_owner_ex {
+	int	type;
+	pid_t	pid;
+};
+
 #ifdef __KERNEL__
 
 #ifndef force_o_largefile
-- 
1.6.5.2

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/

[PATCH v2] fcntl: Use consistent values for F_[GS]ETOWN_EX

[Stephen Rothwell]

These values were only introduced during this release cycle, so it is
still early enough to get them right.

alpha uses the same values that are in asm-generic/fcntl.h, so just
remove them.

parisc uses the values interchanged for no apparent reason, so remove them
to give us consistency across all architectures.

Signed-off-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
---
 arch/alpha/include/asm/fcntl.h  |    2 --
 arch/parisc/include/asm/fcntl.h |    2 --
 include/asm-generic/fcntl.h     |    2 --
 3 files changed, 0 insertions(+), 6 deletions(-)

Ulrich pointed out that using F_LINUX_SPECIFIC_BASE for these make code
less efficient when these values are used in switch statements.

diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h
index 73126e4..25da001 100644
--- a/arch/alpha/include/asm/fcntl.h
+++ b/arch/alpha/include/asm/fcntl.h
@@ -26,8 +26,6 @@
 #define F_GETOWN	6	/*  for sockets. */
 #define F_SETSIG	10	/*  for sockets. */
 #define F_GETSIG	11	/*  for sockets. */
-#define F_SETOWN_EX	15
-#define F_GETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		1
diff --git a/arch/parisc/include/asm/fcntl.h b/arch/parisc/include/asm/fcntl.h
index 5f39d55..1e1c824 100644
--- a/arch/parisc/include/asm/fcntl.h
+++ b/arch/parisc/include/asm/fcntl.h
@@ -28,8 +28,6 @@
 #define F_SETOWN	12	/*  for sockets. */
 #define F_SETSIG	13	/*  for sockets. */
 #define F_GETSIG	14	/*  for sockets. */
-#define F_GETOWN_EX	15
-#define F_SETOWN_EX	16
 
 /* for posix fcntl() and lockf() */
 #define F_RDLCK		01
diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h
index cd2d789..c6f3926 100644
--- a/include/asm-generic/fcntl.h
+++ b/include/asm-generic/fcntl.h
@@ -82,10 +82,8 @@
 #endif
 #endif
 
-#ifndef F_SETOWN_EX
 #define F_SETOWN_EX	15
 #define F_GETOWN_EX	16
-#endif
 
 #define F_OWNER_TID	0
 #define F_OWNER_PID	1
-- 
1.6.5.2

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/