s390x sys32 (remove duplicated code)

RFC PATCH In review Martin Schwidefsky 2 participants 3 messages Aug 27, 2002 – Sep 4, 2002

lore

Share & tools

lore

Martin Schwidefsky

author

2002-08-27 07:30 UTC #0 lore raw

Hi Tim,

Quoted 10 lines

I am working on a patch that removes the old limit of NGROUPS.
In doing so, I am checking everything that references NGROUPS. One such place is in arch/s390x/kernel/linux32.c.
I notice these things:
* s390x defines __kernel_old_gid_t to __kernel_gid_t (unsigned int)
* s390x has a sys32_getgroups16() which looks JUST LIKE uid16.c sys_getgroups16
* s390x does not define CONFIG_UID16, but seems to need it

The early versions of 64 bit s390x did not have the 31 bit emulation layer. As long as only 64 bit code is executed you don't need the 16 bit uids/gids. Thats why is isn't defined.

Quoted 4 lines

It seems to me that if we do:
* s390x defines CONFIG_UID16
* typedef __kernel_old_gid_t to u16
* get rid of all the sys32_*16 stuff and just call the uid16.c function

I checked the code and didn't find any reason why this shouldn't work. In fact with the 31 bit emulation layer the 64 bit kernel does need the 16 bit uid/gid system calls although only for the emulation. To make it really perfect you could define CONFIG_UID16 dependent on CONFIG_S390_SUPPORT. This saves a few bytes in the image if the emulation support is not enabled.

Quoted 2 lines

We could save a bit of duplicated code. Patches that remove code are good, right?

Definitly.

blue skies,
Martin

Linux/390 Design & Development, IBM Deutschland Entwicklung GmbH Schönaicherstr. 220, D-71032 Böblingen, Telefon: 49 - (0)7031 - 16-2247
E-Mail: [email protected]

Tim Hockin

2002-08-29 00:37 UTC #1 lore raw

RFC PATCH 1/1 s390x sys32...)

Quoted 11 lines · Martin Schwidefsky

Quoted 4 lines
It seems to me that if we do:
* s390x defines CONFIG_UID16
* typedef __kernel_old_gid_t to u16
* get rid of all the sys32_*16 stuff and just call the uid16.c function
I checked the code and didn't find any reason why this shouldn't work. In fact with the 31 bit emulation layer the 64 bit kernel does need the 16 bit uid/gid system calls although only for the emulation. To make it really perfect you could define CONFIG_UID16 dependent on CONFIG_S390_SUPPORT. This saves a few bytes in the image if the emulation support is not enabled.

Ok, done. I don't know if you guys prefer BK or plain-old patch, so I'll do this:

Attached are the two patches (one as sent before, the other is just the above change). You can apply the patches, or if you prefer, ping me back and I'll put a bk repository up publicly for you. The patches are probably easiest.

This change will make my life easier for the rest of the ngroups crap I am fixing. I will commit this locally on teh assumption it will turn up sooner or later on your end.

Now, for 2.5.x - do you want me to do another patchset, or are these close enough?

Thanks.

Signature · 4 lines

Tim Hockin
Systems Software Engineer
Sun Microsystems, Linux Kernel Engineering
[email protected]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:

#	           ChangeSet	1.587   -> 1.588  
#	arch/s390x/defconfig	1.8     -> 1.9    
#	arch/s390x/config.in	1.9     -> 1.10

# arch/s390x/kernel/linux32.c 1.10 -> 1.11
# arch/s390x/kernel/entry.S 1.13 -> 1.14
# arch/s390x/kernel/wrapper32.S 1.4 -> 1.5
# include/asm-s390x/posix_types.h 1.1 -> 1.3
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/08/26 [email protected] 1.588

# enable UID16 for s390x
# change s390x duplicated code to call UID16 functions
# --------------------------------------------
#
diff -Nru a/arch/s390x/config.in b/arch/s390x/config.in
--- a/arch/s390x/config.in	Wed Aug 28 17:22:31 2002
+++ b/arch/s390x/config.in	Wed Aug 28 17:22:31 2002
@@ -13,6 +13,7 @@
 mainmenu_name "Linux Kernel Configuration"
 define_bool CONFIG_ARCH_S390 y
 define_bool CONFIG_ARCH_S390X y
+define_bool CONFIG_UID16 y

mainmenu_option next_comment
comment 'Code maturity level options'

arch/s390x/defconfig +1 -0

diff -Nru a/arch/s390x/defconfig b/arch/s390x/defconfig
@@ -9,6 +9,7 @@
 # CONFIG_GENERIC_BUST_SPINLOCK is not set
 CONFIG_ARCH_S390=y
 CONFIG_ARCH_S390X=y
+CONFIG_UID16=y
 
 #
 # Code maturity level options

arch/s390x/kernel/entry.S +5 -5

diff -Nru a/arch/s390x/kernel/entry.S b/arch/s390x/kernel/entry.S
@@ -391,7 +391,7 @@
         .long  SYSCALL(sys_mount,sys32_mount_wrapper)
         .long  SYSCALL(sys_oldumount,sys32_oldumount_wrapper)
         .long  SYSCALL(sys_ni_syscall,sys32_setuid16_wrapper)   /* old setuid16 syscall*/
-        .long  SYSCALL(sys_ni_syscall,sys32_getuid16)   /* old getuid16 syscall*/
+        .long  SYSCALL(sys_ni_syscall,sys_getuid16)   /* old getuid16 syscall*/
         .long  SYSCALL(sys_ni_syscall,sys_ni_syscall) /* 25 old stime syscall */
         .long  SYSCALL(sys_ptrace,sys32_ptrace_wrapper)
         .long  SYSCALL(sys_alarm,sys32_alarm_wrapper)

@@ -413,11 +413,11 @@
         .long  SYSCALL(sys_times,sys32_times_wrapper)
         .long  SYSCALL(sys_ni_syscall,sys_ni_syscall) /* old prof syscall */
         .long  SYSCALL(sys_brk,sys32_brk_wrapper)               /* 45 */
-        .long  SYSCALL(sys_ni_syscall,sys32_setgid16)   /* old setgid16 syscall*/
-        .long  SYSCALL(sys_ni_syscall,sys32_getgid16)   /* old getgid16 syscall*/
+        .long  SYSCALL(sys_ni_syscall,sys_setgid16)   /* old setgid16 syscall*/
+        .long  SYSCALL(sys_ni_syscall,sys_getgid16)   /* old getgid16 syscall*/
         .long  SYSCALL(sys_signal,sys32_signal_wrapper)
-        .long  SYSCALL(sys_ni_syscall,sys32_geteuid16)  /* old geteuid16 syscall */
-        .long  SYSCALL(sys_ni_syscall,sys32_getegid16)  /* old getegid16 syscall */
+        .long  SYSCALL(sys_ni_syscall,sys_geteuid16)  /* old geteuid16 syscall */
+        .long  SYSCALL(sys_ni_syscall,sys_getegid16)  /* old getegid16 syscall */
         .long  SYSCALL(sys_acct,sys32_acct_wrapper)
         .long  SYSCALL(sys_umount,sys32_umount_wrapper)
         .long  SYSCALL(sys_ni_syscall,sys_ni_syscall) /* old lock syscall */

arch/s390x/kernel/linux32.c +0 ~~-175~~

181 lines not on this page · raw message

arch/s390x/kernel/wrapper32.S +15 ~~-15~~

diff -Nru a/arch/s390x/kernel/wrapper32.S b/arch/s390x/kernel/wrapper32.S
@@ -83,7 +83,7 @@
 	llgtr	%r2,%r2			# const char *
 	llgfr	%r3,%r3			# __kernel_old_uid_emu31_t 
 	llgfr	%r4,%r4			# __kernel_old_uid_emu31_t 
-	jg	sys32_lchown16		# branch to system call
+	jg	sys_lchown16		# branch to system call
 
 	.globl  sys32_lseek_wrapper 
 sys32_lseek_wrapper:

@@ -111,7 +111,7 @@
 	.globl  sys32_setuid16_wrapper 
 sys32_setuid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_uid_emu31_t 
-	jg	sys32_setuid16		# branch to system call
+	jg	sys_setuid16		# branch to system call
 
 #sys32_getuid16_wrapper			# void

@@ -195,7 +195,7 @@
 	.globl  sys32_setgid16_wrapper 
 sys32_setgid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_gid_emu31_t 
-	jg	sys32_setgid16		# branch to system call
+	jg	sys_setgid16		# branch to system call
 
 #sys32_getgid16_wrapper			# void

@@ -278,13 +278,13 @@
 sys32_setreuid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_uid_emu31_t 
 	llgfr	%r3,%r3			# __kernel_old_uid_emu31_t 
-	jg	sys32_setreuid16	# branch to system call
+	jg	sys_setreuid16		# branch to system call
 
 	.globl  sys32_setregid16_wrapper 
 sys32_setregid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_gid_emu31_t 
 	llgfr	%r3,%r3			# __kernel_old_gid_emu31_t 
-	jg	sys32_setregid16	# branch to system call
+	jg	sys_setregid16		# branch to system call
 
 #sys32_sigsuspend_wrapper		# done in sigsuspend_glue

@@ -338,13 +338,13 @@
 sys32_getgroups16_wrapper:
 	lgfr	%r2,%r2			# int
 	llgtr	%r3,%r3			# __kernel_old_gid_emu31_t *
-	jg	sys32_getgroups16	# branch to system call
+	jg	sys_getgroups16		# branch to system call
 
 	.globl  sys32_setgroups16_wrapper 
 sys32_setgroups16_wrapper:
 	lgfr	%r2,%r2			# int
 	llgtr	%r3,%r3			# __kernel_old_gid_emu31_t *
-	jg	sys32_setgroups16	# branch to system call
+	jg	sys_setgroups16		# branch to system call
 
 	.globl  sys32_symlink_wrapper 
 sys32_symlink_wrapper:

@@ -419,7 +419,7 @@
 	llgfr	%r2,%r2			# unsigned int
 	llgtr	%r3,%r3			# __kernel_old_uid_emu31_t *
 	llgtr	%r4,%r4			# __kernel_old_gid_emu31_t *
-	jg	sys32_fchown16		# branch to system call
+	jg	sys_fchown16		# branch to system call
 
 	.globl  sys32_getpriority_wrapper 
 sys32_getpriority_wrapper:

@@ -619,12 +619,12 @@
 	.globl  sys32_setfsuid16_wrapper 
 sys32_setfsuid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_uid_emu31_t 
-	jg	sys32_setfsuid16	# branch to system call
+	jg	sys_setfsuid16		# branch to system call
 
 	.globl  sys32_setfsgid16_wrapper 
 sys32_setfsgid16_wrapper:
 	llgfr	%r2,%r2			# __kernel_old_gid_emu31_t 
-	jg	sys32_setfsgid16	# branch to system call
+	jg	sys_setfsgid16		# branch to system call
 
 	.globl  sys32_llseek_wrapper 
 sys32_llseek_wrapper:

@@ -771,14 +771,14 @@
 	llgfr	%r2,%r2			# __kernel_old_uid_emu31_t 
 	llgfr	%r3,%r3			# __kernel_old_uid_emu31_t 
 	llgfr	%r4,%r4			# __kernel_old_uid_emu31_t 
-	jg	sys32_setresuid16	# branch to system call
+	jg	sys_setresuid16		# branch to system call
 
 	.globl  sys32_getresuid16_wrapper 
 sys32_getresuid16_wrapper:
 	llgtr	%r2,%r2			# __kernel_old_uid_emu31_t *
 	llgtr	%r3,%r3			# __kernel_old_uid_emu31_t *
 	llgtr	%r4,%r4			# __kernel_old_uid_emu31_t *
-	jg	sys32_getresuid16	# branch to system call
+	jg	sys_getresuid16		# branch to system call
 
 	.globl  sys32_query_module_wrapper 
 sys32_query_module_wrapper:

@@ -808,14 +808,14 @@
 	llgfr	%r2,%r2			# __kernel_old_gid_emu31_t 
 	llgfr	%r3,%r3			# __kernel_old_gid_emu31_t 
 	llgfr	%r4,%r4			# __kernel_old_gid_emu31_t 
-	jg	sys32_setresgid16	# branch to system call
+	jg	sys_setresgid16		# branch to system call
 
 	.globl  sys32_getresgid16_wrapper 
 sys32_getresgid16_wrapper:
 	llgtr	%r2,%r2			# __kernel_old_gid_emu31_t *
 	llgtr	%r3,%r3			# __kernel_old_gid_emu31_t *
 	llgtr	%r4,%r4			# __kernel_old_gid_emu31_t *
-	jg	sys32_getresgid16	# branch to system call
+	jg	sys_getresgid16		# branch to system call
 
 	.globl  sys32_prctl_wrapper 
 sys32_prctl_wrapper:

@@ -889,7 +889,7 @@
 	llgtr	%r2,%r2			# const char *
 	llgfr	%r3,%r3			# __kernel_old_uid_emu31_t 
 	llgfr	%r4,%r4			# __kernel_old_gid_emu31_t 
-	jg	sys32_chown16		# branch to system call
+	jg	sys_chown16		# branch to system call
 
 	.globl  sys32_getcwd_wrapper 
 sys32_getcwd_wrapper:

include/asm-s390x/posix_types.h +2 -2

diff -Nru a/include/asm-s390x/posix_types.h b/include/asm-s390x/posix_types.h
@@ -37,8 +37,8 @@
 typedef unsigned short	__kernel_uid16_t;
 typedef unsigned short	__kernel_gid16_t;
 
-typedef __kernel_uid_t __kernel_old_uid_t;
-typedef __kernel_gid_t __kernel_old_gid_t;
+typedef __kernel_uid16_t __kernel_old_uid_t;
+typedef __kernel_gid16_t __kernel_old_gid_t;
 typedef __kernel_uid_t __kernel_uid32_t;
 typedef __kernel_gid_t __kernel_gid32_t;

#	           ChangeSet	1.588   -> 1.589  
#	arch/s390x/defconfig	1.9     -> 1.10   
#	arch/s390x/config.in	1.10    -> 1.11

#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/08/28 [email protected] 1.589
# S390x: make CONFIG_UID16 depend on legacy CONFIG_S390_SUPPORT # --------------------------------------------
#

arch/s390x/config.in +1 -1

diff -Nru a/arch/s390x/config.in b/arch/s390x/config.in
@@ -13,7 +13,6 @@
 mainmenu_name "Linux Kernel Configuration"
 define_bool CONFIG_ARCH_S390 y
 define_bool CONFIG_ARCH_S390X y
-define_bool CONFIG_UID16 y
 
 mainmenu_option next_comment
 comment 'Code maturity level options'

@@ -25,6 +24,7 @@
 bool 'Symmetric multi-processing support' CONFIG_SMP
 bool 'Kernel support for 31 bit emulation' CONFIG_S390_SUPPORT
 if [ "$CONFIG_S390_SUPPORT" = "y" ]; then
+  define_bool CONFIG_UID16 y
   tristate 'Kernel support for 31 bit ELF binaries' CONFIG_BINFMT_ELF32 
 fi
 endmenu

arch/s390x/defconfig +1 -1

diff -Nru a/arch/s390x/defconfig b/arch/s390x/defconfig
@@ -9,7 +9,6 @@
 # CONFIG_GENERIC_BUST_SPINLOCK is not set
 CONFIG_ARCH_S390=y
 CONFIG_ARCH_S390X=y
-CONFIG_UID16=y
 
 #
 # Code maturity level options

@@ -21,6 +20,7 @@
 #
 CONFIG_SMP=y
 CONFIG_S390_SUPPORT=y
+CONFIG_UID16=y
 CONFIG_BINFMT_ELF32=y
 
 #

Tim Hockin

2002-09-04 18:15 UTC #2 lore raw

Re: [PATCH} s390x sys32 duplicated code cleanup (was [PATCH RFC] s390x sys32...)

Quoted 8 lines · Tim Hockin

Quoted 6 lines · Martin Schwidefsky
Quoted 4 lines
It seems to me that if we do:
* s390x defines CONFIG_UID16
* typedef __kernel_old_gid_t to u16
* get rid of all the sys32_*16 stuff and just call the uid16.c function

Quoted 6 lines

Quoted 6 lines
I checked the code and didn't find any reason why this shouldn't work. In fact with the 31 bit emulation layer the 64 bit kernel does need the 16 bit uid/gid system calls although only for the emulation. To make it really perfect you could define CONFIG_UID16 dependent on CONFIG_S390_SUPPORT. This saves a few bytes in the image if the emulation support is not enabled.

Uggh, DaveM pointed out a very good issue with this fix (similar for Sparc64) and core files. Core files will now have truncated uid/gid values because fs/binfmt_elf calls NEW_TO_OLD_UID(). May be other places, too.

I guess you should not apply this patch until I've had a better think about it.

Tim

Signature · 4 lines

Tim Hockin
Systems Software Engineer
Sun Microsystems, Linux Kernel Engineering
[email protected]