On 03/20/2017 05:48 PM, Nadav Har'El wrote:
On Mon, Mar 20, 2017 at 4:12 PM, Sergiy Kibrik <[email protected]
<mailto:[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".
will correct that
+ 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...).
Barrier must be set before call to notify_remote_via_evtchn(), which triggers
code execution on the other side of the channel, so that other side is
guaranteed to have correct content in out_prod when executed.
If we insert barrier before writing to out_prod — in case of reorder it might
actually be changed after evtchn signaled, thus other end gets outdated content
in out_prod.
+
+ 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.
I'll verify with large writes then and post v3.
Thank you for taking a look.
--
regards,
Sergiy
--
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.
