on 14/07/2011 00:07 Hans Petter Selasky said the following:
> Author: hselasky
> Date: Wed Jul 13 21:07:50 2011
> New Revision: 223989
> URL: http://svn.freebsd.org/changeset/base/223989
> 
> Log:
>   Fix for dump after shutdown with USB keyboard plugged in. It appears that 
> the
>   system timer is stopped during shutdown and that the pause() statement in 
> ukbd
>   causes infinite hang in this regard. The fix is to use mi_switch() instead 
> of
>   pause() to do the required task switch to ensure that the required USB 
> processes
>   get executed.

I have a question partially about this particular change, partially about 
USB/ukbd
polling mode in general.  See below:

>   Reported by:        mike_kar...@mcafee.com
>   MFC after:  1 week
> 
> Modified:
>   head/sys/dev/usb/input/ukbd.c
> 
> Modified: head/sys/dev/usb/input/ukbd.c
> ==============================================================================
> --- head/sys/dev/usb/input/ukbd.c     Wed Jul 13 21:07:41 2011        
> (r223988)
> +++ head/sys/dev/usb/input/ukbd.c     Wed Jul 13 21:07:50 2011        
> (r223989)
> @@ -59,6 +59,8 @@ __FBSDID("$FreeBSD$");
>  #include <sys/callout.h>
>  #include <sys/malloc.h>
>  #include <sys/priv.h>
> +#include <sys/proc.h>
> +#include <sys/sched.h>
>  #include <sys/kdb.h>
>  
>  #include <dev/usb/usb.h>
> @@ -386,6 +388,33 @@ ukbd_put_key(struct ukbd_softc *sc, uint
>  }
>  
>  static void
> +ukbd_yield(void)
> +{
> +     struct thread *td = curthread;
> +     uint32_t old_prio;
> +
> +     DROP_GIANT();
> +
> +     thread_lock(td);
> +
> +     /* get current priority */
> +     old_prio = td->td_base_pri;
> +
> +     /* set new priority */
> +     sched_prio(td, td->td_user_pri);
> +
> +     /* cause a task switch */
> +     mi_switch(SW_INVOL | SWT_RELINQUISH, NULL);
> +
> +     /* restore priority */
> +     sched_prio(td, old_prio);
> +
> +     thread_unlock(td);
> +
> +     PICKUP_GIANT();
> +}
> +
> +static void
>  ukbd_do_poll(struct ukbd_softc *sc, uint8_t wait)
>  {
>       DPRINTFN(2, "polling\n");
> @@ -396,8 +425,9 @@ ukbd_do_poll(struct ukbd_softc *sc, uint
>  
>       if (kdb_active == 0) {

The question is: why this special subcase is needed at all.
If I understand correctly, the polling mode is used only in some special
situations/contexts.
So why not always use the generic code (after this if-block) that does "proper"
polling?  What do we win when using this special case that seems to depend on 
the
scheduler?

Unfortunately I couldn't fully understand commit log of r203896.

One of the reasons I am asking about this is that soon-ish we may have changes
that disable scheduler in a context where panicstr != NULL.

>               while (sc->sc_inputs == 0) {
> -                     /* make sure the USB code gets a chance to run */
> -                     pause("UKBD", 1);
> +
> +                     /* give USB threads a chance to run */
> +                     ukbd_yield();
>  
>                       /* check if we should wait */
>                       if (!wait)


-- 
Andriy Gapon
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to