[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add upper bound to receiver ring poll to reduce DPC latency
On 17/02/2023 10:47, Owen Smith wrote:
It doesnt look like threaded DPCs are used
https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvif.git;a=blob;f=src/xenvif/receiver.c;h=21451331d330168b4792428f56057e16577c3ca7;hb=HEAD#l2529
<https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvif.git;a=blob;f=src/xenvif/receiver.c;h=21451331d330168b4792428f56057e16577c3ca7;hb=HEAD#l2529>
But that line is calling KeInitialize*Threaded*Dpc(). Am I missing
something?
Paul
I'm not sure of the best way to share results, as most testing was done
on XenServer patched variants (I dont think our patches should have
affected the general performance). Headline results seem to suggest that
file copy speeds (over network, between 2 VMs on the same host)
increased from approx 350-400MB/s to approx 420-460MB/s. This would be
the case where netback is supplying sufficient data to keep the rings
active, looping inside the DPC routines, processing the
incoming fragments. Rachel may be able to collate the results into an
easier to view format (results are currently presented on an internal
wiki page)
Owen
On Tue, Feb 14, 2023 at 4:39 PM Paul Durrant <xadimgnik@xxxxxxxxx
<mailto:xadimgnik@xxxxxxxxx>> wrote:
[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open
attachments unless you have verified the sender and know the content
is safe.
On 07/02/2023 14:05, Rachel.Yan@xxxxxxxxxx
<mailto:Rachel.Yan@xxxxxxxxxx> wrote:
> From: Rachel Yan <Rachel.Yan@xxxxxxxxxx
<mailto:Rachel.Yan@xxxxxxxxxx>>
>
> Adds an upper bound to the ring poll iteration with optimal value
found through experimentation to avoid going round the ring an
infinite (or very large) number of times. This has been tested to
show improvements in DPC latencies and file transfer speeds.
>
Do you have numbers you can share? The reason for moving to threaded
DPCs was to avoid having to play games like this.
Paul
> Signed-off-by: Rachel Yan <rachel.yan@xxxxxxxxxx
<mailto:rachel.yan@xxxxxxxxxx>>
>
> ---
> src/xenvif/receiver.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
> index 2145133..d469de4 100644
> --- a/src/xenvif/receiver.c
> +++ b/src/xenvif/receiver.c
> @@ -2013,11 +2013,15 @@ ReceiverRingPoll(
> PXENVIF_RECEIVER Receiver;
> PXENVIF_FRONTEND Frontend;
> ULONG Count;
> + ULONG MaxCount;
> + BOOLEAN NeedQueueDpc;
>
> Receiver = Ring->Receiver;
> Frontend = Receiver->Frontend;
>
> Count = 0;
> + MaxCount = 10 * XENVIF_RECEIVER_RING_SIZE;
> + NeedQueueDpc = FALSE;
>
> if (!Ring->Enabled || Ring->Paused)
> goto done;
> @@ -2068,6 +2072,15 @@ ReceiverRingPoll(
> PXENVIF_RECEIVER_FRAGMENT Fragment;
> PMDL Mdl;
>
> + // avoid going through the ring an infinite (or
very large) amount of times
> + // if the netback producer happens to fill in just
enough packets to cause us
> + // to go around the ring multiple times. This should
reduce Dpc latencies.
> +
> + if (Count >= MaxCount) {
> + NeedQueueDpc = TRUE;
> + break;
> + }
> +
> rsp = RING_GET_RESPONSE(&Ring->Front, rsp_cons);
>
> // netback is required to complete requests in
order and place
> @@ -2247,7 +2260,7 @@ ReceiverRingPoll(
> if (!__ReceiverRingIsStopped(Ring))
> ReceiverRingFill(Ring);
>
> - if (Ring->PacketQueue != NULL &&
> + if ((NeedQueueDpc || Ring->PacketQueue != NULL) &&
> KeInsertQueueDpc(&Ring->QueueDpc, NULL, NULL))
> Ring->QueueDpcs++;
>
|
