2009-10-04 21:31:10

by Peter Zijlstra

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

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


2009-10-29 17:04:37

by Andreas Schwab

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

Peter Zijlstra <[email protected]> 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, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."

2009-10-30 03:28:11

by Ulrich Drepper

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

On Sun, Oct 4, 2009 at 14:30, Peter Zijlstra <[email protected]> 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 <[email protected]>


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

2009-10-30 05:13:29

by Stephen Rothwell

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

Hi Peter,

On Sun, 04 Oct 2009 23:30:22 +0200 Peter Zijlstra <[email protected]> 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 [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (747.00 B)
(No filename) (198.00 B)
Download all attachments

2009-10-30 05:29:10

by Stephen Rothwell

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

On Fri, 30 Oct 2009 16:13:20 +1100 Stephen Rothwell <[email protected]> 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 <[email protected]>
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 <[email protected]>
---
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 [email protected]
http://www.canb.auug.org.au/~sfr/

2009-11-12 23:33:03

by Stephen Rothwell

[permalink] [raw]
Subject: Re: F_SETOWN_EX and F_GETLK64 conflict

Hi Andrew,

On Fri, 30 Oct 2009 16:29:06 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Fri, 30 Oct 2009 16:13:20 +1100 Stephen Rothwell <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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 [email protected]
http://www.canb.auug.org.au/~sfr/

2009-11-14 07:01:31

by Stephen Rothwell

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

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 <[email protected]>
---
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 [email protected]
http://www.canb.auug.org.au/~sfr/

2009-11-14 09:45:06

by Stephen Rothwell

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

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 <[email protected]>
---
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 [email protected]
http://www.canb.auug.org.au/~sfr/