Luigi Rizzo wrote:
> [Cc peter because he introduced this code]
> 
> Hi,
> i was trying the following code in -current (basically copied from
> vm_zeropage.c), to implement device polling in the idle loop, and
> noticed that the process would take all of the CPU time. Being
> suspicious that something was wrong with priorities, I added a
> couple of printfs, and to my surprise, the priority of the process
> (pri_level) after the first tsleep() becomes 104 instead of staying
> at 255 as it was originally initialized.
> 
> A quick scan of the files in sys/sys shows that 104 is PPAUSE,
> and this is the source of the problem.

Excellent catch!  This particular problem was one of the main reasons
why this is still defaulting to 'off'.  I have a couple of other changes
to it pending commit to fix some of Bruce's complaints, but I hadn't
noticed the cause of this.

Part of the problem is that tsleep temporarily elevates the priority for
wakeup, and it is normally returned back to "normal" level when the process
returns to userland (see the *_userpri stuff).

Changing the tlseep PPAUSE value isn't necessarily the right thing either
because the priority propagation mechanism will elevate us if we hold a lock
that something else *needs*.  (This is so that when it yields, we will run
and do what we need to do and presumably release the lock.)

> I guess this is a bug in vm_zeropage.c, the second parameter
> in tsleep should be td->td_ksegrp->kg_pri.pri_level instead
> of PPAUSE, or at least the priority should be reset to the
> original value after returning from the tsleep ?

Possibly both of these.  Let me do some checking.

>       cheers
>       luigi
> 
> static void
> poll_idle(void)
> {
>         struct thread *td = curthread;
>         struct rtprio rtp;
>  
>         rtp.prio = RTP_PRIO_MAX;        /* lowest priority */
>         rtp.type = RTP_PRIO_IDLE;
>         mtx_lock_spin(&sched_lock);
>         rtp_to_pri(&rtp, &td->td_ksegrp->kg_pri);
>         mtx_unlock_spin(&sched_lock);
>    
>         printf("idlepoll start and sleep, pri is %d native %d user %d\n",
>                 td->td_ksegrp->kg_pri.pri_level,
>                 td->td_ksegrp->kg_pri.pri_native,
>                 td->td_ksegrp->kg_pri.pri_user);
>         for (;;) {
>                 if (poll_in_idle && poll_handlers > 0) {
>                         idlepoll_sleeping = 0;
>                         mtx_lock(&Giant);
>                         ether_poll(poll_each_burst);
>                         mtx_unlock(&Giant);
>                         mtx_assert(&Giant, MA_NOTOWNED);
>                         mtx_lock_spin(&sched_lock);
>                         setrunqueue(td);
>                         td->td_proc->p_stats->p_ru.ru_nvcsw++;
>                         mi_switch();
>                         mtx_unlock_spin(&sched_lock);
>                 } else {
>                       printf("idlepoll goes to sleep, "
>                               "pri is %d native %d user %d\n",
>                               td->td_ksegrp->kg_pri.pri_level,
>                               td->td_ksegrp->kg_pri.pri_native,
>                               td->td_ksegrp->kg_pri.pri_user);
>                         idlepoll_sleeping = 1;
>                         tsleep(&idlepoll_sleeping, PPAUSE, "pollid", hz * 3);
>                 }
>         }
> }
> 
> static struct proc *idlepoll;
> static struct kproc_desc idlepoll_kp = {
>          "idlepoll",
>          poll_idle,
>          &idlepoll
> };
> SYSINIT(idlepoll, SI_SUB_KTHREAD_VM, SI_ORDER_ANY, kproc_start, &idlepoll_kp)
> 
> 
> 

Cheers,
-Peter
--
Peter Wemm - [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
"All of this is for nothing if we don't go to the stars" - JMS/B5


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to