Fw: [Bugme-new] [Bug 2885] New: realtime process can't preempt low

Fw: [Bugme-new] [Bug 2885] New: realtime process can't preempt low

[Andrew Morton]

This would appear to indicate that CONFIG_PREEMPT isn't working
correctly on ia64.


Begin forwarded message:

Date: Mon, 14 Jun 2004 00:25:22 -0700
From: bugme-daemon@osdl.org
To: bugme-new@lists.osdl.org
Subject: [Bugme-new] [Bug 2885] New: realtime process can't preempt low priority process in kernel


http://bugme.osdl.org/show_bug.cgi?id(85

           Summary: realtime process can't preempt low priority process in
                    kernel
    Kernel Version: 2.6.5
            Status: NEW
          Severity: normal
             Owner: rml@tech9.net
         Submitter: hong.liu@intel.com
                CC: yi.zhu@intel.com


Distribution:
Hardware Environment: 4 Itanium2 1.4G processors-tiger4
Software Environment: gcc-3.3.3
Problem Description:

After turning on the CONFIG_PREEMPT option and recompile the kernel source 
code, the realtime process still can't preempt the low priority process which 
is doing busy loop in kernel. 
The test case does pass both on a pentium 4 PC and a Xeon 2.4G smp machine.

Steps to reproduce:
in the directory of the test case, 
#make
#insmod ./modules/hog.ko
#mknod /dev/hog c 126 0
#./do_hog

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

Re: Fw: [Bugme-new] [Bug 2885] New: realtime process can't preempt low priority process in kernel

[Peter Chubb]

Here's a patch to fix OSDL BugMe 2885: IA64 preemption support.

There is one small change in semantics: when returning from a
preemption, the preemption flag will *not* be rechecked.  Otherwise, I
found that it was easy to get into a livelock situation where no
forward progress was made.

Signed-off-by: Peter Chubb <peterc@gelato.unsw.edu.au>

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/06/22 09:08:06+10:00 peterc@gelato.unsw.edu.au 
#   [IA64] [Bug 2885]New: realtime process can't preempt low priority process in kernel 
#   
#   Rearranged code to make it work. There were two problems:
#   
#   	1.  The preempt flag was being tested only if code was 
#               leaving for user space
#   	   (the logic should be: test for RESCHEDULE if we're 
#   	    switching to a kernel thread, test everything
#               if switching to a user thread)
#       
#           2.  The check of the user space flags was being repeated 
#               even if the work had been done.
#   
#    
# 
# arch/ia64/kernel/entry.S
#   2004/06/22 09:07:57+10:00 peterc@gelato.unsw.edu.au +20 -7
#   [IA64] [Bug 2885]New: realtime process can't preempt low priority process in kernel 
#   
#   Rearranged code to make it work. There were two problems:
#   
#   	1.  The preempt flag was being tested only if code was 
#               leaving for user space
#   	   (the logic should be: test for RESCHEDULE if we're 
#   	    switching to a kernel thread, test everything
#               if switching to a user thread)
#       
#           2.  The check of the user space flags was being repeated 
#               even if the work had been done.
#   
#    
# 
diff -Nru a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S
--- a/arch/ia64/kernel/entry.S	Tue Jun 22 09:22:03 2004
+++ b/arch/ia64/kernel/entry.S	Tue Jun 22 09:22:03 2004
@@ -662,6 +662,11 @@
 	/*
 	 * work.need_resched etc. mustn't get changed by this CPU before it returns to
 	 * user- or fsys-mode, hence we disable interrupts early on:
+	 *
+	 * p6 means to check the threadinfo_flags for work to do, including
+	 * preemption if we're returning to kernel mode.
+	 * This code is reentered at .work_processed_syscall
+	 * with p6 = 0 so we don't recheck
 	 */
 #ifdef CONFIG_PREEMPT
 	rsm psr.i				// disable interrupts
@@ -669,8 +674,6 @@
 (pUStk)	rsm psr.i
 #endif
 	cmp.eq pLvSys,p0=r0,r0			// pLvSys=1: leave from syscall
-(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
-.work_processed_syscall:
 #ifdef CONFIG_PREEMPT
 (pKStk) adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
 	;;
@@ -678,8 +681,11 @@
 (pKStk) ld4 r21=[r20]			// r21 <- preempt_count
 (pUStk)	mov r21=0			// r21 <- 0
 	;;
-(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
+	cmp.eq.unc p6,p0=r21,r0		// p6 <- pUStk || (preempt_count = 0)
+#else
+(pUStk) cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
 #endif /* CONFIG_PREEMPT */
+.work_processed_syscall:
 	adds r16=PT(LOADRS)+16,r12
 	adds r17=PT(AR_BSPSTORE)+16,r12
 	adds r18=TI_FLAGS+IA64_TASK_SIZE,r13
@@ -775,6 +781,11 @@
 	/*
 	 * work.need_resched etc. mustn't get changed by this CPU before it returns to
 	 * user- or fsys-mode, hence we disable interrupts early on:
+	 *
+	 * p6 means to check the threadinfo_flags for work to do, including
+	 * preemption if we're returning to kernel mode.
+	 * This code is reentered at .work_processed_kernel
+	 * with p6 = 0 so we don't recheck
 	 */
 #ifdef CONFIG_PREEMPT
 	rsm psr.i				// disable interrupts
@@ -782,18 +793,20 @@
 (pUStk)	rsm psr.i
 #endif
 	cmp.eq p0,pLvSys=r0,r0			// pLvSys=0: leave from kernel
-(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
 	;;
-.work_processed_kernel:
 #ifdef CONFIG_PREEMPT
 	adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
 	;;
 	.pred.rel.mutex pUStk,pKStk
 (pKStk)	ld4 r21=[r20]			// r21 <- preempt_count
-(pUStk)	mov r21=0			// r21 <- 0
+(pUStk)	mov r21=0
 	;;
-(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
+	cmp.eq p6,p0=r21,r0		// p6 <- pUStk || (preempt_count = 0)
+#else
+(pUStk)	cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
 #endif /* CONFIG_PREEMPT */
+
+.work_processed_kernel:
 	adds r17=TI_FLAGS+IA64_TASK_SIZE,r13
 	;;
 (p6)	ld4 r31=[r17]				// load current_thread_info()->flags


-- 
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
The technical we do immediately,  the political takes *forever*

RE: Fw: [Bugme-new] [Bug 2885] New: realtime process can't preempt low priority process in kernel

[Zhu, Yi]

Peter Chubb wrote:
> Here's a patch to fix OSDL BugMe 2885: IA64 preemption support.
> 
> There is one small change in semantics: when returning from a
> preemption, the preemption flag will *not* be rechecked.
> Otherwise, I found that it was easy to get into a livelock
> situation where no forward progress was made.

Thank you Peter, the patch works perfectly.

-yi

> 
> Signed-off-by: Peter Chubb <peterc@gelato.unsw.edu.au>
> 
> # This is a BitKeeper generated diff -Nru style patch. #
> # ChangeSet
> #   2004/06/22 09:08:06+10:00 peterc@gelato.unsw.edu.au
> #   [IA64] [Bug 2885]New: realtime process can't preempt low priority
> process in kernel #
> #   Rearranged code to make it work. There were two problems: #
> #   	1.  The preempt flag was being tested only if code was
> #               leaving for user space
> #   	   (the logic should be: test for RESCHEDULE if we're
> #   	    switching to a kernel thread, test everything
> #               if switching to a user thread)
> #
> #           2.  The check of the user space flags was being repeated
> #               even if the work had been done.
> #
> #
> #
> # arch/ia64/kernel/entry.S
> #   2004/06/22 09:07:57+10:00 peterc@gelato.unsw.edu.au +20 -7
> #   [IA64] [Bug 2885]New: realtime process can't preempt low priority
> process in kernel #
> #   Rearranged code to make it work. There were two problems: #
> #   	1.  The preempt flag was being tested only if code was
> #               leaving for user space
> #   	   (the logic should be: test for RESCHEDULE if we're
> #   	    switching to a kernel thread, test everything
> #               if switching to a user thread)
> #
> #           2.  The check of the user space flags was being repeated
> #               even if the work had been done.
> #
> #
> #
> diff -Nru a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S
> --- a/arch/ia64/kernel/entry.S	Tue Jun 22 09:22:03 2004
> +++ b/arch/ia64/kernel/entry.S	Tue Jun 22 09:22:03 2004 @@
-662,6
>  	+662,11 @@ /*
>  	 * work.need_resched etc. mustn't get changed by this
> CPU before it returns to
>  	 * user- or fsys-mode, hence we disable interrupts early on: +
*
> +	 * p6 means to check the threadinfo_flags for work to
> do, including
> +	 * preemption if we're returning to kernel mode.
> +	 * This code is reentered at .work_processed_syscall
> +	 * with p6 = 0 so we don't recheck
>  	 */
>  #ifdef CONFIG_PREEMPT
>  	rsm psr.i				// disable interrupts
> @@ -669,8 +674,6 @@
>  (pUStk)	rsm psr.i
>  #endif
>  	cmp.eq pLvSys,p0=r0,r0			// pLvSys=1:
> leave from syscall
> -(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
> -.work_processed_syscall:
>  #ifdef CONFIG_PREEMPT
>  (pKStk) adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
>  	;;
> @@ -678,8 +681,11 @@
>  (pKStk) ld4 r21=[r20]			// r21 <- preempt_count
>  (pUStk)	mov r21=0			// r21 <- 0
>  	;;
> -(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
> +	cmp.eq.unc p6,p0=r21,r0		// p6 <- pUStk ||
> (preempt_count = 0)
> +#else
> +(pUStk) cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
>  #endif /* CONFIG_PREEMPT */
> +.work_processed_syscall:
>  	adds r16=PT(LOADRS)+16,r12
>  	adds r17=PT(AR_BSPSTORE)+16,r12
>  	adds r18=TI_FLAGS+IA64_TASK_SIZE,r13
> @@ -775,6 +781,11 @@
>  	/*
>  	 * work.need_resched etc. mustn't get changed by this
> CPU before it returns to
>  	 * user- or fsys-mode, hence we disable interrupts early on: +
*
> +	 * p6 means to check the threadinfo_flags for work to
> do, including
> +	 * preemption if we're returning to kernel mode.
> +	 * This code is reentered at .work_processed_kernel
> +	 * with p6 = 0 so we don't recheck
>  	 */
>  #ifdef CONFIG_PREEMPT
>  	rsm psr.i				// disable interrupts
> @@ -782,18 +793,20 @@
>  (pUStk)	rsm psr.i
>  #endif
>  	cmp.eq p0,pLvSys=r0,r0			// pLvSys=0:
> leave from kernel
> -(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
>  	;;
> -.work_processed_kernel:
>  #ifdef CONFIG_PREEMPT
>  	adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
>  	;;
>  	.pred.rel.mutex pUStk,pKStk
>  (pKStk)	ld4 r21=[r20]			// r21 <- preempt_count
> -(pUStk)	mov r21=0			// r21 <- 0
> +(pUStk)	mov r21=0
>  	;;
> -(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
> +	cmp.eq p6,p0=r21,r0		// p6 <- pUStk ||
> (preempt_count = 0)
> +#else
> +(pUStk)	cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
>  #endif /* CONFIG_PREEMPT */
> +
> +.work_processed_kernel:
>  	adds r17=TI_FLAGS+IA64_TASK_SIZE,r13
>  	;;
>  (p6)	ld4 r31=[r17]				// load
> current_thread_info()->flags

Re: Fw: [Bugme-new] [Bug 2885] New: realtime process can't preempt low priority process in kernel

[David Mosberger]

Peter,

Can you confirm that this patch also works for you?  It's your patch with
the following changes:

 o Consolidate code into a single #ifdef CONFIG_PREEMPT.
 o Replace unconditional "cmp.eq.unc" with "cmp.eq".
 o Cleanup/correct comment.
 o Sync up code in ia64_leave_kernel with ia64_leave_syscall.

Rest should be the same.

Two questions though: first, the code will call notify_resume_user
when returning to kernel-level execution if preempt_count is 0 and
current_thread_info()->need_resched() is 0.  Is this really what you
had in mind?

Second, isn't preempt_count necessarily 0 when returning to
user-level?  If so, it should be possible to simplify the
CONFIG_PREEMPT code a bit more.

Thanks,

	--david

>>>>> On Tue, 22 Jun 2004 09:57:43 +1000, Peter Chubb <peterc@gelato.unsw.edu.au> said:

  Peter> Here's a patch to fix OSDL BugMe 2885: IA64 preemption
  Peter> support.

  Peter> There is one small change in semantics: when returning from a
  Peter> preemption, the preemption flag will *not* be rechecked.
  Peter> Otherwise, I found that it was easy to get into a livelock
  Peter> situation where no forward progress was made.

---------------------------------------------------------------------------
ia64: Fix OSDL BugMe report 2885: realtime process can't preempt low priority process in kernel

Rearranged code to make it work. There were two problems:

 1. The preempt flag was being tested only if code was leaving for
    user space (the logic should be: test for RESCHEDULE if we're
    switching to a kernel thread, test everything if switching to a
    user thread)

 2. The check of the user space flags was being repeated even if the
    work had been done.

There is one small change in semantics: when returning from a
preemption, the preemption flag will *not* be rechecked.  Otherwise, I
found that it was easy to get into a livelock situation where no
forward progress was made.

=== arch/ia64/kernel/entry.S 1.60 vs edited ==--- 1.60/arch/ia64/kernel/entry.S	Wed Jun 16 21:12:18 2004
+++ edited/arch/ia64/kernel/entry.S	Tue Jun 29 11:17:02 2004
@@ -663,25 +663,31 @@
 	PT_REGS_UNWIND_INFO(0)
 	/*
 	 * work.need_resched etc. mustn't get changed by this CPU before it returns to
-	 * user- or fsys-mode, hence we disable interrupts early on:
+	 * user- or fsys-mode, hence we disable interrupts early on.
+	 *
+	 * p6 controls whether current_thread_info()->flags needs to be check for
+	 * extra work.  We always check for extra work when returning to user-level.
+	 * With CONFIG_PREEMPT, we also check for extra work when the preempt_count
+	 * is 0.  After extra work processing has been completed, execution
+	 * resumes at .work_processed_syscall with p6 set to 1 if the extra-work-check
+	 * needs to be redone.
 	 */
 #ifdef CONFIG_PREEMPT
 	rsm psr.i				// disable interrupts
-#else
-(pUStk)	rsm psr.i
-#endif
 	cmp.eq pLvSys,p0=r0,r0			// pLvSys=1: leave from syscall
-(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
-.work_processed_syscall:
-#ifdef CONFIG_PREEMPT
 (pKStk) adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
 	;;
 	.pred.rel.mutex pUStk,pKStk
 (pKStk) ld4 r21=[r20]			// r21 <- preempt_count
 (pUStk)	mov r21=0			// r21 <- 0
 	;;
-(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
-#endif /* CONFIG_PREEMPT */
+	cmp.eq p6,p0=r21,r0		// p6 <- pUStk || (preempt_count = 0)
+#else /* !CONFIG_PREEMPT */
+(pUStk)	rsm psr.i
+	cmp.eq pLvSys,p0=r0,r0		// pLvSys=1: leave from syscall
+(pUStk)	cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
+#endif
+.work_processed_syscall:
 	adds r16=PT(LOADRS)+16,r12
 	adds r17=PT(AR_BSPSTORE)+16,r12
 	adds r18=TI_FLAGS+IA64_TASK_SIZE,r13
@@ -776,26 +782,31 @@
 	PT_REGS_UNWIND_INFO(0)
 	/*
 	 * work.need_resched etc. mustn't get changed by this CPU before it returns to
-	 * user- or fsys-mode, hence we disable interrupts early on:
+	 * user- or fsys-mode, hence we disable interrupts early on.
+	 *
+	 * p6 controls whether current_thread_info()->flags needs to be check for
+	 * extra work.  We always check for extra work when returning to user-level.
+	 * With CONFIG_PREEMPT, we also check for extra work when the preempt_count
+	 * is 0.  After extra work processing has been completed, execution
+	 * resumes at .work_processed_syscall with p6 set to 1 if the extra-work-check
+	 * needs to be redone.
 	 */
 #ifdef CONFIG_PREEMPT
 	rsm psr.i				// disable interrupts
-#else
-(pUStk)	rsm psr.i
-#endif
 	cmp.eq p0,pLvSys=r0,r0			// pLvSys=0: leave from kernel
-(pUStk)	cmp.eq.unc p6,p0=r0,r0			// p6 <- pUStk
-	;;
-.work_processed_kernel:
-#ifdef CONFIG_PREEMPT
-	adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
+(pKStk)	adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
 	;;
 	.pred.rel.mutex pUStk,pKStk
 (pKStk)	ld4 r21=[r20]			// r21 <- preempt_count
 (pUStk)	mov r21=0			// r21 <- 0
 	;;
-(p6)	cmp.eq.unc p6,p0=r21,r0		// p6 <- p6 && (r21 = 0)
-#endif /* CONFIG_PREEMPT */
+	cmp.eq p6,p0=r21,r0		// p6 <- pUStk || (preempt_count = 0)
+#else
+(pUStk)	rsm psr.i
+	cmp.eq p0,pLvSys=r0,r0		// pLvSys=0: leave from kernel
+(pUStk)	cmp.eq.unc p6,p0=r0,r0		// p6 <- pUStk
+#endif
+.work_processed_kernel:
 	adds r17=TI_FLAGS+IA64_TASK_SIZE,r13
 	;;
 (p6)	ld4 r31=[r17]				// load current_thread_info()->flags