Skip to contentSkip to conversation

"sh: convert to ->regset_get()" breaks linux-sh build

regression fix Al Viro 3 participants 4 messages Aug 9, 2020

Subsystems (2)
lore
Share & tools
lore

Al Viro

author
#0 lore raw
Quoted 9 lines · Al Viro

What the... oh, I see.

Commit in the regset followup series has fixed that, with bisect hazard unnoticed. And since only the followups have not gone in, bisect hazard has turned into a mainline breakage ;/

Sorry about that. FWIW, the commit in question is this; all per-architecture parts in it are mutually independent, but I'll probably just send this one to Linus - no point splitting it up.

FWIW, there are several ways to handle that. One is to pull vfs.git#fixes - that's the first commit of #regset.followups. Another is to cherry-pick the same. And the minimal fix would be the subset of that commit as below. Linus, what would you prefer?

sh: kill unused dump_fpu() instance

dead code now that fdpic has switched to regset coredumps.

Fixes 3399d90ce63e "sh: convert to ->regset_get()"

---

arch/sh/include/asm/fpu.h +0 -5
index 43cfaf929aa7..04584be8986c 100644
@@ -37,11 +37,6 @@ struct user_regset;
 extern int do_fpu_inst(unsigned short, struct pt_regs *);
 extern int init_fpu(struct task_struct *);
 
-extern int fpregs_get(struct task_struct *target,
-		      const struct user_regset *regset,
-		      unsigned int pos, unsigned int count,
-		      void *kbuf, void __user *ubuf);
-
 static inline void __unlazy_fpu(struct task_struct *tsk, struct pt_regs *regs)
 {
 	if (task_thread_info(tsk)->status & TS_USEDFPU) {
arch/sh/kernel/process_32.c +0 -18
index 6ab397bc47ed..7a59a63560c5 100644
@@ -93,24 +93,6 @@ void release_thread(struct task_struct *dead_task)
 	/* do nothing */
 }
 
-/* Fill in the fpu structure for a core dump.. */
-int dump_fpu(struct pt_regs *regs, elf_fpregset_t *fpu)
-{
-	int fpvalid = 0;
-
-#if defined(CONFIG_SH_FPU)
-	struct task_struct *tsk = current;
-
-	fpvalid = !!tsk_used_math(tsk);
-	if (fpvalid)
-		fpvalid = !fpregs_get(tsk, NULL,
-				      (struct membuf){fpu, sizeof(*fpu)});
-#endif
-
-	return fpvalid;
-}
-EXPORT_SYMBOL(dump_fpu);
-
 asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 
arch/sh/kernel/ptrace_32.c +1 -1
index 5c93bdb6c41a..609b7c917e6e 100644
@@ -165,7 +165,7 @@ static int genregs_set(struct task_struct *target,
 }
 
 #ifdef CONFIG_SH_FPU
-int fpregs_get(struct task_struct *target,
+static int fpregs_get(struct task_struct *target,
 	       const struct user_regset *regset,
 	       struct membuf to)
 {
#1 lore raw
Quoted 5 lines · Al Viro

FWIW, there are several ways to handle that. One is to pull vfs.git#fixes - that's the first commit of #regset.followups. Another is to cherry-pick the same. And the minimal fix would be the subset of that commit as below. Linus, what would you prefer?

Mind just sending me a proper pull request for the fixes branch with that one commit?

Linus

author
#2 lore raw
In reply to Al Viro
Quoted 9 lines · Linus Torvalds
Quoted 5 lines · Al Viro

FWIW, there are several ways to handle that. One is to pull vfs.git#fixes - that's the first commit of #regset.followups. Another is to cherry-pick the same. And the minimal fix would be the subset of that commit as below. Linus, what would you prefer?

Mind just sending me a proper pull request for the fixes branch with that one commit?

Sure.

Fixing regression from unnoticed bisect hazard in regset series. A bunch of old (aout, originally) primitives used by coredumps became dead code after fdpic conversion to regsets. Removal of that dead code had been the first commit in the followups to regset series; unfortunately, it happened to hide the bisect hazard on sh (extern for fpregs_get() had not been updated in the main series when it should have been; followup simply made fpregs_get() static). And without that followup commit this bisect hazard became breakage in the mainline.

Please, pull that followup commit in - all it does is dead code removal, so it should be safe enough. And it deals with regression. One trivial conflict in nios2 - addition of nios2_clone() in mainline vs. removal of dump_fpu() in this commit, both in the very end of arch/nios2/kernel/process.c.

The following changes since commit ce327e1c54119179066d6f3573a28001febc9265:

regset: kill user_regset_copyout{,_zero}() (2020-07-27 14:31:13 -0400)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git fixes

for you to fetch changes up to bb1a773d5b6bf018bf84fdb3fbba01d3ef54e2c9:

kill unused dump_fpu() instances (2020-07-27 14:33:10 -0400)

----------------------------------------------------------------
Al Viro (2):
Merge branch 'work.fdpic' into regset.followup kill unused dump_fpu() instances

 arch/arc/kernel/process.c      |  5 ----
 arch/arm/kernel/process.c      | 15 ----------
 arch/hexagon/kernel/process.c  |  9 ------
 arch/ia64/kernel/process.c     | 34 ----------------------
 arch/nios2/kernel/process.c    |  8 -----
 arch/openrisc/kernel/process.c |  7 -----
 arch/parisc/include/asm/elf.h  |  3 --
 arch/parisc/kernel/process.c   | 19 ------------
 arch/s390/kernel/process.c     | 18 ------------
 arch/sh/include/asm/fpu.h      |  5 ----
 arch/sh/kernel/process_32.c    | 18 ------------
 arch/sh/kernel/ptrace_32.c     |  2 +-
 arch/sparc/kernel/process_32.c | 49 -------------------------------
 arch/sparc/kernel/process_64.c | 66 ------------------------------------------
 14 files changed, 1 insertion(+), 257 deletions(-)
maintainer Tested-by
#3 lore raw
In reply to Al Viro

Hi Al!

Quoted 75 lines · Al Viro
Quoted 9 lines · Al Viro

What the... oh, I see.

Commit in the regset followup series has fixed that, with bisect hazard unnoticed. And since only the followups have not gone in, bisect hazard has turned into a mainline breakage ;/

Sorry about that. FWIW, the commit in question is this; all per-architecture parts in it are mutually independent, but I'll probably just send this one to Linus - no point splitting it up.

FWIW, there are several ways to handle that. One is to pull vfs.git#fixes - that's the first commit of #regset.followups. Another is to cherry-pick the same. And the minimal fix would be the subset of that commit as below. Linus, what would you prefer?

sh: kill unused dump_fpu() instance

dead code now that fdpic has switched to regset coredumps.

Fixes 3399d90ce63e "sh: convert to ->regset_get()"

---

arch/sh/include/asm/fpu.h +0 -5
index 43cfaf929aa7..04584be8986c 100644
@@ -37,11 +37,6 @@ struct user_regset;
 extern int do_fpu_inst(unsigned short, struct pt_regs *);
 extern int init_fpu(struct task_struct *);
 
-extern int fpregs_get(struct task_struct *target,
-		      const struct user_regset *regset,
-		      unsigned int pos, unsigned int count,
-		      void *kbuf, void __user *ubuf);
-
 static inline void __unlazy_fpu(struct task_struct *tsk, struct pt_regs *regs)
 {
 	if (task_thread_info(tsk)->status & TS_USEDFPU) {
arch/sh/kernel/process_32.c +0 -18
index 6ab397bc47ed..7a59a63560c5 100644
@@ -93,24 +93,6 @@ void release_thread(struct task_struct *dead_task)
 	/* do nothing */
 }
 
-/* Fill in the fpu structure for a core dump.. */
-int dump_fpu(struct pt_regs *regs, elf_fpregset_t *fpu)
-{
-	int fpvalid = 0;
-
-#if defined(CONFIG_SH_FPU)
-	struct task_struct *tsk = current;
-
-	fpvalid = !!tsk_used_math(tsk);
-	if (fpvalid)
-		fpvalid = !fpregs_get(tsk, NULL,
-				      (struct membuf){fpu, sizeof(*fpu)});
-#endif
-
-	return fpvalid;
-}
-EXPORT_SYMBOL(dump_fpu);
-
 asmlinkage void ret_from_fork(void);
 asmlinkage void ret_from_kernel_thread(void);
 
arch/sh/kernel/ptrace_32.c +0 -0
index 5c93bdb6c41a..609b7c917e6e 100644

arch/sh/kernel/ptrace_32.c static int genregs_set(struct task_struct *target, · line 171

Quoted 8 lines
arch/sh/kernel/ptrace_32.c +1 -1
@@ -165,7 +165,7 @@ static int genregs_set(struct task_struct *target,
 }
 
 #ifdef CONFIG_SH_FPU
-int fpregs_get(struct task_struct *target,
+static int fpregs_get(struct task_struct *target,
 	       const struct user_regset *regset,
 	       struct membuf to)
 {

This fixes the linux-next build on SH for me and also produces a bootable kernel image which boots without any issues on my SH7785LCR.

Thanks,
Adrian

Signature · 4 lines
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [email protected]
`. `'   Freie Universitaet Berlin - [email protected]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913