From owner-freebsd-xen@FreeBSD.ORG Wed Sep 4 17:49:40 2013 Return-Path: Delivered-To: freebsd-xen@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 764BC9AE for ; Wed, 4 Sep 2013 17:49:40 +0000 (UTC) (envelope-from gibbs@freebsd.org) Received: from aslan.scsiguy.com (aslan.scsiguy.com [70.89.174.89]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 503D7273F for ; Wed, 4 Sep 2013 17:49:40 +0000 (UTC) Received: from benscottlt.sldomain.com (207-225-98-3.dia.static.qwest.net [207.225.98.3]) (authenticated bits=0) by aslan.scsiguy.com (8.14.7/8.14.5) with ESMTP id r84Hnaaw031340 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 4 Sep 2013 11:49:37 -0600 (MDT) (envelope-from gibbs@freebsd.org) Content-Type: multipart/mixed; boundary="Apple-Mail=_8FB0F9B6-B925-4672-AD4B-78D54204A510" Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\)) Subject: Re: Another blkback issue related to flush From: "Justin T. Gibbs" In-Reply-To: <522768EA.6010809@citrix.com> Date: Wed, 4 Sep 2013 11:49:33 -0600 Message-Id: References: <522768EA.6010809@citrix.com> To: =?iso-8859-1?Q?Roger_Pau_Monn=E9?= X-Mailer: Apple Mail (2.1508) X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (aslan.scsiguy.com [70.89.174.89]); Wed, 04 Sep 2013 11:49:37 -0600 (MDT) Cc: "freebsd-xen@freebsd.org" X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Sep 2013 17:49:40 -0000 --Apple-Mail=_8FB0F9B6-B925-4672-AD4B-78D54204A510 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=iso-8859-1 On Sep 4, 2013, at 11:07 AM, Roger Pau Monn=E9 = wrote: > Hello, >=20 > I've found another issue with blkback handling of flush operations, it > was incorrectly setting one of the bio parameters when using a block > device as backend. The attached patch fixes it, and also includes a > small fix to correctly set the operation when writing the response on > the ring (all responses written by FreeBSD blkback were of type > BLKIF_OP_READ, because nreq->operation was not set). It seems that the req's pendcnt is unused and can be culled. Otherwise, looks good to me. Here's the patch I have in my tree now. -- Justin --Apple-Mail=_8FB0F9B6-B925-4672-AD4B-78D54204A510 Content-Disposition: attachment; filename=blkback.diff Content-Type: application/octet-stream; name="blkback.diff" Content-Transfer-Encoding: 7bit Index: blkback.c =================================================================== --- blkback.c (revision 255179) +++ blkback.c (working copy) @@ -230,7 +230,7 @@ struct xbb_xen_reqlist { int num_children; /** - * Number of I/O requests dispatched to the backend. + * Number of I/O requests still pending on the backend. */ int pendcnt; @@ -327,13 +327,6 @@ struct xbb_xen_req { int nr_512b_sectors; /** - * The number of struct bio requests still outstanding for this - * request on the backend device. This field is only used for - * device (rather than file) backed I/O. - */ - int pendcnt; - - /** * BLKIF_OP code for this request. */ int operation; @@ -1240,6 +1233,7 @@ xbb_get_resources(struct xbb_softc *xbb, struct xb nreq->reqlist = *reqlist; nreq->req_ring_idx = ring_idx; nreq->id = ring_req->id; + nreq->operation = ring_req->operation; if (xbb->abi != BLKIF_PROTOCOL_NATIVE) { bcopy(ring_req, &nreq->ring_req_storage, sizeof(*ring_req)); @@ -2062,7 +2056,6 @@ xbb_dispatch_dev(struct xbb_softc *xbb, struct xbb { struct xbb_dev_data *dev_data; struct bio *bios[XBB_MAX_SEGMENTS_PER_REQLIST]; - struct xbb_xen_req *nreq; off_t bio_offset; struct bio *bio; struct xbb_sg *xbb_sg; @@ -2080,7 +2073,6 @@ xbb_dispatch_dev(struct xbb_softc *xbb, struct xbb bio_idx = 0; if (operation == BIO_FLUSH) { - nreq = STAILQ_FIRST(&reqlist->contig_req_list); bio = g_new_bio(); if (__predict_false(bio == NULL)) { DPRINTF("Unable to allocate bio for BIO_FLUSH\n"); @@ -2094,10 +2086,10 @@ xbb_dispatch_dev(struct xbb_softc *xbb, struct xbb bio->bio_offset = 0; bio->bio_data = 0; bio->bio_done = xbb_bio_done; - bio->bio_caller1 = nreq; + bio->bio_caller1 = reqlist; bio->bio_pblkno = 0; - nreq->pendcnt = 1; + reqlist->pendcnt = 1; SDT_PROBE1(xbb, kernel, xbb_dispatch_dev, flush, device_get_unit(xbb->dev)); --Apple-Mail=_8FB0F9B6-B925-4672-AD4B-78D54204A510 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=iso-8859-1 --Apple-Mail=_8FB0F9B6-B925-4672-AD4B-78D54204A510--