Date: Mon, 12 Dec 2011 10:03:54 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: mdf@FreeBSD.org Cc: usb@FreeBSD.org, hackers@FreeBSD.org Subject: Re: kern_yield vs ukbd_yield Message-ID: <4EE5B56A.4000106@FreeBSD.org> In-Reply-To: <CAMBSHm-Gmyfm83wDnhKTWAM%2BM90SEdub9uNOexG7QvuGWvN3iQ@mail.gmail.com> References: <4EE51CB5.1060505@FreeBSD.org> <CAMBSHm-Gmyfm83wDnhKTWAM%2BM90SEdub9uNOexG7QvuGWvN3iQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
on 11/12/2011 23:48 mdf@FreeBSD.org said the following:
> On Sun, Dec 11, 2011 at 1:12 PM, Andriy Gapon <avg@freebsd.org> wrote:
>>
>> Does the following change do what I think that it does?
>> Thank you!
>>
>> Author: Andriy Gapon <avg@icyb.net.ua>
>> Date: Thu Sep 1 16:50:13 2011 +0300
>>
>> ukbd: drop local duplicate of kern_yield and use that instead
>>
>> diff --git a/sys/dev/usb/input/ukbd.c b/sys/dev/usb/input/ukbd.c
>> index 086c178..8078cbb 100644
>> --- a/sys/dev/usb/input/ukbd.c
>> +++ b/sys/dev/usb/input/ukbd.c
>> @@ -399,33 +399,6 @@ ukbd_put_key(struct ukbd_softc *sc, uint32_t key)
>> }
>>
>> 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)
>> {
>>
>> @@ -439,7 +412,7 @@ ukbd_do_poll(struct ukbd_softc *sc, uint8_t wait)
>> while (sc->sc_inputs == 0) {
>>
>> /* give USB threads a chance to run */
>> - ukbd_yield();
>> + kern_yield(-1);
>
> Not quite.
>
> 1) -1 should be spelled PRI_UNCHANGED, except ukbd_yield() uses
> td_user_pri, but then puts it back again, so I think UNCHANGED is what
> is meant.
> 2) kern_yield() calls it a SW_VOL rather than SW_INVOL, which seems
> the desired behaviour here anyways, since this is an explicit (i.e.
> voluntary) yield.
Thank you for the explanation. So would you say that the patch is OK?
--
Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4EE5B56A.4000106>