svn commit: r297190 - head/sys/kern

[Bruce Evans]

On Thu, 24 Mar 2016, Edward Tomasz [utf-8] NapieraÅ~Ba wrote:

> On 0324T1015, Warner Losh wrote:
>> On Thu, Mar 24, 2016 at 9:46 AM, Ian Lepore <ian at freebsd.org> wrote:
>>
>>> On Thu, 2016-03-24 at 16:01 +0100, Edward Tomasz Napierała wrote:
>>>> On 0324T1609, Alexander Motin wrote:
>>>>> On 24.03.16 15:42, Edward Tomasz Napierała wrote:
>>>>>> On 0324T1032, Jean-Sébastien Pédron wrote:
>>>>>>> On 23/03/2016 18:45, Edward Tomasz Napierala wrote:
>>>>>>>>> So maybe callouts are disabled in this situation. If there
>>>>>>>>> is a way to
>>>>>>>>> detect that, then vt(4) can go back to a "synchronous mode"
>>>>>>>>> where it
>>>>>>>>> refreshes the screen after each typed character, like it
>>>>>>>>> does when ddb
>>>>>>>>> is active.
>>>>>>>>
>>>>>>>> Looks like that's the case: for some reason the callouts
>>>>>>>> don't work.
>>>>>>>> This trivial hack is a (mostly) working workaround:
>>>>>>>>
>>>>>>>> Index: svn/head/sys/kern/kern_cons.c
>>>>>>>> =============================================================
>>>>>>>> ======
>>>>>>>> --- svn/head/sys/kern/kern_cons.c     (revision 297210)
>>>>>>>> +++ svn/head/sys/kern/kern_cons.c     (working copy)
>>>>>>>> @@ -430,6 +430,7 @@ cngets(char *cp, size_t size, int
>>>>>>>> visible)
>>>>>>>>       lp = cp;
>>>>>>>>       end = cp + size - 1;
>>>>>>>>       for (;;) {
>>>>>>>> +             pause("meh", 1);
>>>>>>>
>>>>>>> Could you please explain how this works to me? Does calling
>>>>>>> pause() here
>>>>>>> give a chance to interrupt handlers or other threads of
>>>>>>> running?
>>>>>>
>>>>>> It looks like it allows the callout to run.  I've did an
>>>>>> experiment
>>>>>> and added a simple callout that printed something each second;
>>>>>> during
>>>>>> the root mount prompt it doesn't get run unless you type '.',
>>>>>> which
>>>>>> calls pause(9).
>>>>>
>>>>> Kernel threads run with absolute priorities, so if somehow this
>>>>> threads
>>>>> happen to have higher or equal priority then callout thread, or the
>>>>> kernel is built without PREEMPTION, then the last may never be
>>>>> executed
>>>>> until this thread get to sleep or at least call sched_yield().
>>>>
>>>> The callout's td_priority seems to be 40; the thread running the
>>>> prompt
>>>> is 84, so it's lower.
>>>>
>>>> I've just noticed another curious thing, though: when you press
>>>> ScrLk,
>>>> the screen gets immediately refreshed; also, pressing arrows works
>>>> just
>>>> the way it should.  In other words, the refresh is broken except for
>>>> the ScrlLk mode, where it works as it should.
>>>
>>> Since cngets() is used only by the mountroot prompt and the geli pw
>>> entry, pausing/yielding within the input loop seems like a good idea.
>>>  It would allow for things like plugging in a usb device and having it
>>> actually appear without having to enter a '.' several times.
>>>
>>> It would be nice if the pause were done with pause_sbt() and a shorter
>>> timeout, maybe a millisecond or even 100uS.  Otherwise things like
>>> pasting text at that prompt in a serial console is likely to drop
>>> chars.
>>>
>>> Hmmm... speaking of the geli pw prompt... what's the locking situation
>>> there?  Will there be any problems calling pause() from that context?
>>
>> PVM isn't an ideal priority to wait at. PWAIT would be better. However,
>> if the only reason to call pause is run the scheduler after each character,
>> perhaps a better solution would be to call kern_yield() instead? We could
>> do that instead of cpu_waitspin() inside of cngetc, but that would break
>> the debugger's use of it....

Console drivers can't block or depend on timeouts.  Perhaps cngets() can,
because it is only called in simple contexts, but the simple contexts
make it especially easy for it to work using the same synchronous i/o that
is needed in more complicated contexts.

> I think we should first try to figure out why this doesn't work in the first
> place.
>
> Basically, even though the interrupts are running, scheduler seems to be ok,
> and the thread that's calling this has a lower priority than the callouts
> thread, it can't be preempted.  This doesn't seem to be caused by vt(4);
> with syscons the callouts don't get called either (it just doesn't break
> the echo in this case).  To demonstrate the problem you can add add
> a callout that calls printf each second and then does an infinite loop.

cngets() is even correctly wrapped by cngrab()/cnungrab().  This is supposed
to put the console driver in a special synchronous mode for the duration of
the grabbing.  (Console drivers still need to be reentrant, so that they
can do i/o when the grabbed section is reentered for ddb or another
non-maskable trap.)  syscons and vt have similar too-simple grab handlers
which start with a screen switch in the !cold case.  This should refresh
the screen and/or switch it to special low-level console screen(s).

Screen switching in ddb is noticeably more broken for vt than for syscons.
Indeed, it just doesn't work in vt (except for the initial switch to vty0).
This might be related.

Bruce