[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
Huh... wasnt saying that when I looked at it and now it does :puzzled:
Owen
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++;
> >
>
>
|