[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
Ahh... is a non-threaded DPC for the Poll, is a threaded DPC for the queue.
Should both be threaded? or does the poll need to be a non-threaded DPC, and therefore this patch should help.
Owen
Do you perhaps have a local patch that undoes the change to threaded DPC?
Paul
On 20/02/2023 16:24, Owen Smith wrote:
> Huh... wasnt saying that when I looked at it and now it does :puzzled:
>
> Owen
>
> On Mon, Feb 20, 2023 at 2:13 PM Paul Durrant <xadimgnik@xxxxxxxxx
> <mailto:xadimgnik@xxxxxxxxx>> wrote:
>
> 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> <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>
> > <mailto: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>
> > <mailto:Rachel.Yan@xxxxxxxxxx <mailto:Rachel.Yan@xxxxxxxxxx>>
> wrote:
> > > From: Rachel Yan <Rachel.Yan@xxxxxxxxxx
> <mailto:Rachel.Yan@xxxxxxxxxx>
> > <mailto: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>
> > <mailto: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++;
> > >
> >
> >
>
|
