Skip to content

Commit fc327e2

Browse files
arch/tile: fix up some issues in calling do_work_pending()
First, we were at risk of handling thread-info flags, in particular do_signal(), when returning from kernel space. This could happen after a failed kernel_execve(), or when forking a kernel thread. The fix is to test in do_work_pending() for user_mode() and return immediately if so; we already had this test for one of the flags, so I just hoisted it to the top of the function. Second, if a ptraced process updated the callee-saved registers in the ptregs struct and then processed another thread-info flag, we would overwrite the modifications with the original callee-saved registers. To fix this, we add a register to note if we've already saved the registers once, and skip doing it on additional passes through the loop. To avoid a performance hit from the couple of extra instructions involved, I modified the GET_THREAD_INFO() macro to be guaranteed to be one instruction, then bundled it with adjacent instructions, yielding an overall net savings. Reported-By: Al Viro <[email protected]> Signed-off-by: Chris Metcalf <[email protected]>
1 parent 36be505 commit fc327e2

File tree

4 files changed

+68
-27
lines changed

4 files changed

+68
-27
lines changed

arch/tile/include/asm/thread_info.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,14 @@ extern void cpu_idle_on_new_stack(struct thread_info *old_ti,
100100

101101
#else /* __ASSEMBLY__ */
102102

103-
/* how to get the thread information struct from ASM */
103+
/*
104+
* How to get the thread information struct from assembly.
105+
* Note that we use different macros since different architectures
106+
* have different semantics in their "mm" instruction and we would
107+
* like to guarantee that the macro expands to exactly one instruction.
108+
*/
104109
#ifdef __tilegx__
105-
#define GET_THREAD_INFO(reg) move reg, sp; mm reg, zero, LOG2_THREAD_SIZE, 63
110+
#define EXTRACT_THREAD_INFO(reg) mm reg, zero, LOG2_THREAD_SIZE, 63
106111
#else
107112
#define GET_THREAD_INFO(reg) mm reg, sp, zero, LOG2_THREAD_SIZE, 31
108113
#endif

arch/tile/kernel/intvec_32.S

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,18 @@ STD_ENTRY(interrupt_return)
838838
.Lresume_userspace:
839839
FEEDBACK_REENTER(interrupt_return)
840840

841+
/*
842+
* Use r33 to hold whether we have already loaded the callee-saves
843+
* into ptregs. We don't want to do it twice in this loop, since
844+
* then we'd clobber whatever changes are made by ptrace, etc.
845+
* Get base of stack in r32.
846+
*/
847+
{
848+
GET_THREAD_INFO(r32)
849+
movei r33, 0
850+
}
851+
852+
.Lretry_work_pending:
841853
/*
842854
* Disable interrupts so as to make sure we don't
843855
* miss an interrupt that sets any of the thread flags (like
@@ -848,9 +860,6 @@ STD_ENTRY(interrupt_return)
848860
IRQ_DISABLE(r20, r21)
849861
TRACE_IRQS_OFF /* Note: clobbers registers r0-r29 */
850862

851-
/* Get base of stack in r32; note r30/31 are used as arguments here. */
852-
GET_THREAD_INFO(r32)
853-
854863

855864
/* Check to see if there is any work to do before returning to user. */
856865
{
@@ -866,16 +875,18 @@ STD_ENTRY(interrupt_return)
866875

867876
/*
868877
* Make sure we have all the registers saved for signal
869-
* handling or single-step. Call out to C code to figure out
870-
* exactly what we need to do for each flag bit, then if
871-
* necessary, reload the flags and recheck.
878+
* handling, notify-resume, or single-step. Call out to C
879+
* code to figure out exactly what we need to do for each flag bit,
880+
* then if necessary, reload the flags and recheck.
872881
*/
873-
push_extra_callee_saves r0
874882
{
875883
PTREGS_PTR(r0, PTREGS_OFFSET_BASE)
876-
jal do_work_pending
884+
bnz r33, 1f
877885
}
878-
bnz r0, .Lresume_userspace
886+
push_extra_callee_saves r0
887+
movei r33, 1
888+
1: jal do_work_pending
889+
bnz r0, .Lretry_work_pending
879890

880891
/*
881892
* In the NMI case we
@@ -1180,10 +1191,12 @@ handle_syscall:
11801191
add r20, r20, tp
11811192
lw r21, r20
11821193
addi r21, r21, 1
1183-
sw r20, r21
1194+
{
1195+
sw r20, r21
1196+
GET_THREAD_INFO(r31)
1197+
}
11841198

11851199
/* Trace syscalls, if requested. */
1186-
GET_THREAD_INFO(r31)
11871200
addi r31, r31, THREAD_INFO_FLAGS_OFFSET
11881201
lw r30, r31
11891202
andi r30, r30, _TIF_SYSCALL_TRACE
@@ -1362,15 +1375,17 @@ handle_ill:
13621375
3:
13631376
/* set PC and continue */
13641377
lw r26, r24
1365-
sw r28, r26
1378+
{
1379+
sw r28, r26
1380+
GET_THREAD_INFO(r0)
1381+
}
13661382

13671383
/*
13681384
* Clear TIF_SINGLESTEP to prevent recursion if we execute an ill.
13691385
* The normal non-arch flow redundantly clears TIF_SINGLESTEP, but we
13701386
* need to clear it here and can't really impose on all other arches.
13711387
* So what's another write between friends?
13721388
*/
1373-
GET_THREAD_INFO(r0)
13741389

13751390
addi r1, r0, THREAD_INFO_FLAGS_OFFSET
13761391
{

arch/tile/kernel/intvec_64.S

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,20 @@ STD_ENTRY(interrupt_return)
646646
.Lresume_userspace:
647647
FEEDBACK_REENTER(interrupt_return)
648648

649+
/*
650+
* Use r33 to hold whether we have already loaded the callee-saves
651+
* into ptregs. We don't want to do it twice in this loop, since
652+
* then we'd clobber whatever changes are made by ptrace, etc.
653+
*/
654+
{
655+
movei r33, 0
656+
move r32, sp
657+
}
658+
659+
/* Get base of stack in r32. */
660+
EXTRACT_THREAD_INFO(r32)
661+
662+
.Lretry_work_pending:
649663
/*
650664
* Disable interrupts so as to make sure we don't
651665
* miss an interrupt that sets any of the thread flags (like
@@ -656,9 +670,6 @@ STD_ENTRY(interrupt_return)
656670
IRQ_DISABLE(r20, r21)
657671
TRACE_IRQS_OFF /* Note: clobbers registers r0-r29 */
658672

659-
/* Get base of stack in r32; note r30/31 are used as arguments here. */
660-
GET_THREAD_INFO(r32)
661-
662673

663674
/* Check to see if there is any work to do before returning to user. */
664675
{
@@ -674,16 +685,18 @@ STD_ENTRY(interrupt_return)
674685

675686
/*
676687
* Make sure we have all the registers saved for signal
677-
* handling or single-step. Call out to C code to figure out
688+
* handling or notify-resume. Call out to C code to figure out
678689
* exactly what we need to do for each flag bit, then if
679690
* necessary, reload the flags and recheck.
680691
*/
681-
push_extra_callee_saves r0
682692
{
683693
PTREGS_PTR(r0, PTREGS_OFFSET_BASE)
684-
jal do_work_pending
694+
bnez r33, 1f
685695
}
686-
bnez r0, .Lresume_userspace
696+
push_extra_callee_saves r0
697+
movei r33, 1
698+
1: jal do_work_pending
699+
bnez r0, .Lretry_work_pending
687700

688701
/*
689702
* In the NMI case we
@@ -968,11 +981,16 @@ handle_syscall:
968981
shl16insli r20, r20, hw0(irq_stat + IRQ_CPUSTAT_SYSCALL_COUNT_OFFSET)
969982
add r20, r20, tp
970983
ld4s r21, r20
971-
addi r21, r21, 1
972-
st4 r20, r21
984+
{
985+
addi r21, r21, 1
986+
move r31, sp
987+
}
988+
{
989+
st4 r20, r21
990+
EXTRACT_THREAD_INFO(r31)
991+
}
973992

974993
/* Trace syscalls, if requested. */
975-
GET_THREAD_INFO(r31)
976994
addi r31, r31, THREAD_INFO_FLAGS_OFFSET
977995
ld r30, r31
978996
andi r30, r30, _TIF_SYSCALL_TRACE

arch/tile/kernel/process.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,10 @@ struct task_struct *__sched _switch_to(struct task_struct *prev,
567567
*/
568568
int do_work_pending(struct pt_regs *regs, u32 thread_info_flags)
569569
{
570+
/* If we enter in kernel mode, do nothing and exit the caller loop. */
571+
if (!user_mode(regs))
572+
return 0;
573+
570574
if (thread_info_flags & _TIF_NEED_RESCHED) {
571575
schedule();
572576
return 1;
@@ -589,8 +593,7 @@ int do_work_pending(struct pt_regs *regs, u32 thread_info_flags)
589593
return 1;
590594
}
591595
if (thread_info_flags & _TIF_SINGLESTEP) {
592-
if ((regs->ex1 & SPR_EX_CONTEXT_1_1__PL_MASK) == 0)
593-
single_step_once(regs);
596+
single_step_once(regs);
594597
return 0;
595598
}
596599
panic("work_pending: bad flags %#x\n", thread_info_flags);

0 commit comments

Comments
 (0)