Re: Native preemption is the culprit [was Re: today's CURRENT lockups]

On Thursday 08 July 2004 09:21 am, Taku YAMAMOTO wrote:
> greetings,
>
>
> A quick glance showed me that there are some interesting code paths in
> sched_ule.c that can be problematic in SMP case.
>
>   1. sched_choose() => kseq_idled() => sched_add()
>   2. sched_choose() => kseq_assign() => sched_add()
>   3. sched_runnable() => kseq_assign() => sched_add()
>
> Here is the patch that re-enables preemption except for the above three
> cases.

This looks correct.  I'll test it locally first.  Has it worked for you all 
day?

> --- sched_ule.c.orig	Tue Jul  6 14:57:29 2004
> +++ sched_ule.c	Thu Jul  8 06:37:30 2004
> _at__at_ -286,6 +286,7 _at__at_
>  static void sched_balance_groups(void);
>  static void sched_balance_group(struct kseq_group *ksg);
>  static void sched_balance_pair(struct kseq *high, struct kseq *low);
> +static void sched_add_internal(struct thread *td, int preemptive);
>  static void kseq_move(struct kseq *from, int cpu);
>  static int kseq_idled(struct kseq *kseq);
>  static void kseq_notify(struct kse *ke, int cpu);
> _at__at_ -616,7 +617,7 _at__at_
>  			kseq_runq_rem(steal, ke);
>  			kseq_load_rem(steal, ke);
>  			ke->ke_cpu = PCPU_GET(cpuid);
> -			sched_add(ke->ke_thread);
> +			sched_add_internal(ke->ke_thread, 0);
>  			return (0);
>  		}
>  	}
> _at__at_ -644,7 +645,7 _at__at_
>  	for (; ke != NULL; ke = nke) {
>  		nke = ke->ke_assign;
>  		ke->ke_flags &= ~KEF_ASSIGNED;
> -		sched_add(ke->ke_thread);
> +		sched_add_internal(ke->ke_thread, 0);
>  	}
>  }
>
> _at__at_ -1542,6 +1543,14 _at__at_
>  void
>  sched_add(struct thread *td)
>  {
> +#ifdef SMP
> +	sched_add_internal(td, 1);
> +}
> +
> +static void
> +sched_add_internal(struct thread *td, int preemptive)
> +{
> +#endif /* SMP */
>  	struct kseq *kseq;
>  	struct ksegrp *kg;
>  	struct kse *ke;
> _at__at_ -1623,17 +1632,21 _at__at_
>          if (td->td_priority < curthread->td_priority)
>                  curthread->td_flags |= TDF_NEEDRESCHED;
>
> -#if 0
>  #ifdef SMP
>  	/*
>  	 * Only try to preempt if the thread is unpinned or pinned to the
>  	 * current CPU.
> +	 * XXX - avoid preemption if called from sched_ule.c internally.
> +	 * there're a few code pathes that may be problematic:
> +	 *     sched_choose() => kseq_idled() => sched_add
> +	 *     sched_choose() => kseq_assign() => sched_add
> +	 *     sched_runnable() => kseq_assign() => sched_add
>  	 */
> -	if (KSE_CAN_MIGRATE(ke, class) || ke->ke_cpu == PCPU_GET(cpuid))
> +	if (preemptive &&
> +	    (KSE_CAN_MIGRATE(ke, class) || ke->ke_cpu == PCPU_GET(cpuid)))
>  #endif
>  	if (maybe_preempt(td))
>  		return;
> -#endif
>  	ke->ke_ksegrp->kg_runq_kses++;
>  	ke->ke_state = KES_ONRUNQ;
>
>
> This patch is tested on P4_at_2.8GHz HTT-enabled machine.
>
> It has been running for several hours without a hang, although I have to
> admit that this machine is too idle, far from being stressed.
>
>
> On Tue, 6 Jul 2004 00:14:21 -0400 (EDT)
>
> Robert Watson <rwatson_at_freebsd.org> wrote:
> > (This time to more people)
> >
> > The patch below appears to (brute force) eliminate the crash/hang I'm
> > experiencing with SCHED_ULE in the post-preemption universe.  However, I
> > was experiencing it only in the SMP case, not UP, so it could be I'm just
> > not triggering it timing-wise.  This would be a temporary fix until jhb
> > is online again post-USENIX to take a look, assuming this works around
> > the problem for people other than me.
> >
> > Note that this is probably damaging to interrupt processing latency.
> >
> > Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
> > robert_at_fledge.watson.org      Principal Research Scientist, McAfee
> > Research
>
> (snip)
>
> > _______________________________________________
> > freebsd-current_at_freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-current
> > To unsubscribe, send any mail to
> > "freebsd-current-unsubscribe_at_freebsd.org"

-- 
John Baldwin <jhb_at_FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org