From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] xen/blkback: Don't let in-flight requests defer pending ones. Date: Tue, 31 May 2011 12:08:07 -0400 Message-ID: <20110531160807.GA31639@dumpdata.com> References: <1306613567.20284.83.camel@ramone> <1306614070-13137-1-git-send-email-daniel.stodden@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1306614070-13137-1-git-send-email-daniel.stodden@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Daniel Stodden Cc: pradeepv@amazon.com, Xen List-Id: xen-devel@lists.xenproject.org On Sat, May 28, 2011 at 01:21:10PM -0700, Daniel Stodden wrote: > Running RING_FINAL_CHECK_FOR_REQUESTS from make_response is a bad > idea. It means that in-flight I/O is essentially blocking continued > batches. This essentially kills throughput on frontends which unplug > (or even just notify) early and rightfully assume addtional requests You have any perf numbers? > will be picked up on time, not synchronously. > > Signed-off-by: Daniel Stodden > --- > drivers/block/xen-blkback/blkback.c | 36 ++++++++++++++++++---------------- > 1 files changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c > index 9dee545..48ad7fa 100644 > --- a/drivers/block/xen-blkback/blkback.c > +++ b/drivers/block/xen-blkback/blkback.c > @@ -451,7 +451,8 @@ static void end_block_io_op(struct bio *bio, int error) > * (which has the sectors we want, number of them, grant references, etc), > * and transmute it to the block API to hand it over to the proper block disk. > */ > -static int do_block_io_op(struct xen_blkif *blkif) > +static int > +__do_block_io_op(struct xen_blkif *blkif) > { > union blkif_back_rings *blk_rings = &blkif->blk_rings; > struct blkif_request req; > @@ -508,6 +509,23 @@ static int do_block_io_op(struct xen_blkif *blkif) > return more_to_do; > } > > +static int > +do_block_io_op(blkif_t *blkif) > +{ > + blkif_back_rings_t *blk_rings = &blkif->blk_rings; > + int more_to_do; > + > + do { > + more_to_do = __do_block_io_op(blkif); > + if (more_to_do) > + break; > + > + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); > + } while (more_to_do); > + > + return more_to_do; > +} > + > /* > * Transmutation of the 'struct blkif_request' to a proper 'struct bio' > * and call the 'submit_bio' to pass it to the underlying storage. > @@ -698,7 +716,6 @@ static void make_response(struct xen_blkif *blkif, u64 id, > struct blkif_response resp; > unsigned long flags; > union blkif_back_rings *blk_rings = &blkif->blk_rings; > - int more_to_do = 0; > int notify; > > resp.id = id; > @@ -725,22 +742,7 @@ static void make_response(struct xen_blkif *blkif, u64 id, > } > blk_rings->common.rsp_prod_pvt++; > RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify); > - if (blk_rings->common.rsp_prod_pvt == blk_rings->common.req_cons) { > - /* > - * Tail check for pending requests. Allows frontend to avoid > - * notifications if requests are already in flight (lower > - * overheads and promotes batching). > - */ > - RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); > - > - } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->common)) { > - more_to_do = 1; > - } > - > spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); > - > - if (more_to_do) > - blkif_notify_work(blkif); > if (notify) > notify_remote_via_irq(blkif->irq); > } > -- > 1.7.4.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel