On Mon, Mar 20, 2017 at 4:12 PM, Sergiy Kibrik <
[email protected]> wrote:
> On 03/20/2017 03:55 PM, Sergiy Kibrik wrote:
>
>> void XEN_Console::write(const char *str, size_t len) {
>> - HYPERVISOR_console_write(str, len);
>> + assert(len > 0);
>> + if (!_interface) {
>> + HYPERVISOR_console_write(str, len);
>> + return;
>> + }
>> +
>> + /* str might be larger then ring, so write it by chunks in this case
>> */
>>
>
Instead of "larger then ring", you should write "larger than the available
space in the ring".
> + XENCONS_RING_IDX prod = _interface->out_prod;
>> + constexpr auto ringsize = sizeof(_interface->out) - 1;
>> + while (true) {
>> + XENCONS_RING_IDX cons = _interface->out_cons;
>>
> + auto delta = prod - cons;
>> +
>> + if (unlikely(delta > ringsize)) {
>> + prod = cons; /* ring is corrupted, reset is the best we can
>> do */
>> + delta = 0;
>> + }
>> +
>> + size_t c = 0;
>> + for (; c < std::min(ringsize - delta, len); c++)
>> + _interface->out[MASK_XENCONS_IDX(prod++, _interface->out)]
>> = *str++;
>> +
>> + _interface->out_prod = prod;
>> + wmb();
>>
>
I'm not convinced this wmb() is in the right place (shouldn't it be between
writing to the array and setting the index). Can you please give this
another thought? (unfortunately I'm rusty on this topic...).
> +
>> + if (likely(c == len))
>> + break; /* flush() will do evtchn notification */
>> +
>> + len -= ringsize - delta;
>> + notify_remote_via_evtchn(_evtchn);
>> +
>> + while (_interface->out_cons != _interface->out_prod)
>> + cpu_relax(); /* can't sleep here */
>> + }
>> }
>>
>>
> hi Nadav,
> I've implemented flow control like this,
The code looks correct to me. Thanks.
> but I can't properly verify and test it, as LineDiscipline and
> debug_early() write to console by single character at a time, while flow
> control presumes writing by quite large blocks of 2KB size.
> In this v2 variant console works fine right now, but can you suggest how
> flow control can be verified?
>
You're right that today, line-discipline.cc writes characters one at a
time, and there is some waste in it but we never really worried about
console performance (KVM's serial console is so slow anyway, that this
seemed negligible...).
Your code doesn't assume large writes, and is not needed *only* for large
writes - even if you try to write 1 byte, you can see that you have 0 free
bytes in the ring (because the host didn't keep up). Moreover, if you do a
large high-level write in the test application, this will translate to many
1-byte writes to your driver followed by just one flush() (which will alert
the hypervisor) - so such a large write will likely fill the entire ring
with many single bytes - and only then your new code will do a notification
and wait to continue. Before your new code sends the notification - I think
Xen won't even notice it has new characters to display and won't consume
the ring.
So I think a large high-level write *will* trigger your new code, even if
it causes single-byte writes internally.
But instead, or in addition, you can just to test this in an ad-hoc
fashion, you can stick a call to your write somewhere in the code somewhere
with a large buffer, to see it works.
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
