| Message ID | 1464786697-20639-5-git-send-email-dsafonov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 01, 2016 at 04:11:35PM +0300, Dmitry Safonov wrote: > As we have here core registers, use them to determine application's > mode and sizes of register set and elf_prstatus instead of TIF_IA32 > flag. > > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > Cc: linux-fsdevel@vger.kernel.org > Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> This can be used unrelated to vdso re-mapping idea, right? -- 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
On 06/03/2016 12:51 PM, Cyrill Gorcunov wrote: > On Wed, Jun 01, 2016 at 04:11:35PM +0300, Dmitry Safonov wrote: >> As we have here core registers, use them to determine application's >> mode and sizes of register set and elf_prstatus instead of TIF_IA32 >> flag. >> >> Cc: Andy Lutomirski <luto@kernel.org> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: Oleg Nesterov <oleg@redhat.com> >> Cc: Alexander Viro <viro@zeniv.linux.org.uk> >> Cc: linux-fsdevel@vger.kernel.org >> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> > > This can be used unrelated to vdso re-mapping idea, right? > Right -- 4,5,6 are pathes for dropping TIF_IA32 flag. I put them in this patches set because of their goal, to make tasks independent of presence of TIF_IA32. -- 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
On Fri, Jun 03, 2016 at 12:56:30PM +0300, Dmitry Safonov wrote: > > > > This can be used unrelated to vdso re-mapping idea, right? > > > > Right -- 4,5,6 are pathes for dropping TIF_IA32 flag. > I put them in this patches set because of their goal, > to make tasks independent of presence of TIF_IA32. Ah, great. So this is continues Andy's idea of ripping off this flag. Thanks! -- 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
On 06/01, Dmitry Safonov wrote: > > static int fill_thread_core_info(struct elf_thread_core_info *t, > const struct user_regset_view *view, > - long signr, size_t *total) > + long signr, size_t *total, > + struct pt_regs *regs __maybe_unused) > { > unsigned int i; > > @@ -1652,11 +1653,11 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, > */ > fill_prstatus(&t->prstatus, t->task, signr); > (void) view->regsets[0].get(t->task, &view->regsets[0], > - 0, PR_REG_SIZE(t->prstatus.pr_reg), > + 0, PR_REG_SIZE(t->prstatus.pr_reg, regs), Hmm. I don't understand this... Note that this "regs" argument has nothing to do with t->task if the process is multithreaded, > @@ -1772,7 +1773,8 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, > * Now fill in each thread's information. > */ > for (t = info->thread; t != NULL; t = t->next) > - if (!fill_thread_core_info(t, view, siginfo->si_signo, &info->size)) > + if (!fill_thread_core_info(t, view, siginfo->si_signo, > + &info->size, regs)) fill_note_info(..., args) is called with args = task_pt_regs(dumper_thread). Oleg. -- 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
On 06/06, Oleg Nesterov wrote: > > On 06/01, Dmitry Safonov wrote: > > > > static int fill_thread_core_info(struct elf_thread_core_info *t, > > const struct user_regset_view *view, > > - long signr, size_t *total) > > + long signr, size_t *total, > > + struct pt_regs *regs __maybe_unused) > > { > > unsigned int i; > > > > @@ -1652,11 +1653,11 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, > > */ > > fill_prstatus(&t->prstatus, t->task, signr); > > (void) view->regsets[0].get(t->task, &view->regsets[0], > > - 0, PR_REG_SIZE(t->prstatus.pr_reg), > > + 0, PR_REG_SIZE(t->prstatus.pr_reg, regs), > > Hmm. I don't understand this... Note that this "regs" argument has nothing > to do with t->task if the process is multithreaded, > > > @@ -1772,7 +1773,8 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, > > * Now fill in each thread's information. > > */ > > for (t = info->thread; t != NULL; t = t->next) > > - if (!fill_thread_core_info(t, view, siginfo->si_signo, &info->size)) > > + if (!fill_thread_core_info(t, view, siginfo->si_signo, > > + &info->size, regs)) > > fill_note_info(..., args) is called with args = task_pt_regs(dumper_thread). forgot to mention... yes, this matches the fact we use a single "view" for all threads, and we get it via task_user_regset_view(dump_task). But this change (imo) adds even more confusion, and without the next patch the logic looks "obviously wrong", becauase PR_REG_SIZE/etc look at dumper_thread->cs while task_user_regset_view() checks thread flags. Anyway I fail to understand these macros... Say, PR_REG_SIZE(S). Can't we kill it and use regsets[0].n * regsets[0].size instead ? These numbers should match whatever we do, if we call ->get(). Oleg. -- 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
On 06/07/2016 01:43 AM, Oleg Nesterov wrote: > On 06/06, Oleg Nesterov wrote: >> >> On 06/01, Dmitry Safonov wrote: >>> >>> static int fill_thread_core_info(struct elf_thread_core_info *t, >>> const struct user_regset_view *view, >>> - long signr, size_t *total) >>> + long signr, size_t *total, >>> + struct pt_regs *regs __maybe_unused) >>> { >>> unsigned int i; >>> >>> @@ -1652,11 +1653,11 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, >>> */ >>> fill_prstatus(&t->prstatus, t->task, signr); >>> (void) view->regsets[0].get(t->task, &view->regsets[0], >>> - 0, PR_REG_SIZE(t->prstatus.pr_reg), >>> + 0, PR_REG_SIZE(t->prstatus.pr_reg, regs), >> >> Hmm. I don't understand this... Note that this "regs" argument has nothing >> to do with t->task if the process is multithreaded, >> >>> @@ -1772,7 +1773,8 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, >>> * Now fill in each thread's information. >>> */ >>> for (t = info->thread; t != NULL; t = t->next) >>> - if (!fill_thread_core_info(t, view, siginfo->si_signo, &info->size)) >>> + if (!fill_thread_core_info(t, view, siginfo->si_signo, >>> + &info->size, regs)) >> >> fill_note_info(..., args) is called with args = task_pt_regs(dumper_thread). > > forgot to mention... yes, this matches the fact we use a single "view" > for all threads, and we get it via task_user_regset_view(dump_task). > > But this change (imo) adds even more confusion, and without the next patch > the logic looks "obviously wrong", becauase PR_REG_SIZE/etc look at > dumper_thread->cs while task_user_regset_view() checks thread flags. > > Anyway I fail to understand these macros... Say, PR_REG_SIZE(S). Can't we > kill it and use regsets[0].n * regsets[0].size instead ? These numbers > should match whatever we do, if we call ->get(). > Thanks, the idea of dropping PR_REG_SIZE looks better than my patch! I'll try to drop those macros for the next revision.
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h index 5a3b2c119ed0..d0b517fc77ff 100644 --- a/arch/x86/include/asm/compat.h +++ b/arch/x86/include/asm/compat.h @@ -264,10 +264,10 @@ struct compat_shmid64_ds { #ifdef CONFIG_X86_X32_ABI typedef struct user_regs_struct compat_elf_gregset_t; -#define PR_REG_SIZE(S) (test_thread_flag(TIF_IA32) ? 68 : 216) -#define PRSTATUS_SIZE(S) (test_thread_flag(TIF_IA32) ? 144 : 296) -#define SET_PR_FPVALID(S,V) \ - do { *(int *) (((void *) &((S)->pr_reg)) + PR_REG_SIZE(0)) = (V); } \ +#define PR_REG_SIZE(S, R) (!user_64bit_mode(R) ? 68 : 216) +#define PRSTATUS_SIZE(S, R) (!user_64bit_mode(R) ? 144 : 296) +#define SET_PR_FPVALID(S, V, R) \ + do { *(int *) (((void *) &((S)->pr_reg)) + PR_REG_SIZE(0, R)) = (V); } \ while (0) #define COMPAT_USE_64BIT_TIME \ diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index e158b22ef32f..3876382edc72 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1623,11 +1623,11 @@ static void do_thread_regset_writeback(struct task_struct *task, } #ifndef PR_REG_SIZE -#define PR_REG_SIZE(S) sizeof(S) +#define PR_REG_SIZE(S, R) sizeof(S) #endif #ifndef PRSTATUS_SIZE -#define PRSTATUS_SIZE(S) sizeof(S) +#define PRSTATUS_SIZE(S, R) sizeof(S) #endif #ifndef PR_REG_PTR @@ -1635,12 +1635,13 @@ static void do_thread_regset_writeback(struct task_struct *task, #endif #ifndef SET_PR_FPVALID -#define SET_PR_FPVALID(S, V) ((S)->pr_fpvalid = (V)) +#define SET_PR_FPVALID(S, V, R) ((S)->pr_fpvalid = (V)) #endif static int fill_thread_core_info(struct elf_thread_core_info *t, const struct user_regset_view *view, - long signr, size_t *total) + long signr, size_t *total, + struct pt_regs *regs __maybe_unused) { unsigned int i; @@ -1652,11 +1653,11 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, */ fill_prstatus(&t->prstatus, t->task, signr); (void) view->regsets[0].get(t->task, &view->regsets[0], - 0, PR_REG_SIZE(t->prstatus.pr_reg), + 0, PR_REG_SIZE(t->prstatus.pr_reg, regs), PR_REG_PTR(&t->prstatus), NULL); fill_note(&t->notes[0], "CORE", NT_PRSTATUS, - PRSTATUS_SIZE(t->prstatus), &t->prstatus); + PRSTATUS_SIZE(t->prstatus, regs), &t->prstatus); *total += notesize(&t->notes[0]); do_thread_regset_writeback(t->task, &view->regsets[0]); @@ -1686,7 +1687,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, regset->core_note_type, size, data); else { - SET_PR_FPVALID(&t->prstatus, 1); + SET_PR_FPVALID(&t->prstatus, 1, regs); fill_note(&t->notes[i], "CORE", NT_PRFPREG, size, data); } @@ -1772,7 +1773,8 @@ static int fill_note_info(struct elfhdr *elf, int phdrs, * Now fill in each thread's information. */ for (t = info->thread; t != NULL; t = t->next) - if (!fill_thread_core_info(t, view, siginfo->si_signo, &info->size)) + if (!fill_thread_core_info(t, view, siginfo->si_signo, + &info->size, regs)) return 0; /*
As we have here core registers, use them to determine application's mode and sizes of register set and elf_prstatus instead of TIF_IA32 flag. Cc: Andy Lutomirski <luto@kernel.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> --- arch/x86/include/asm/compat.h | 8 ++++---- fs/binfmt_elf.c | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-)