From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Bader Subject: Re: [PATCH v3] xen-blk(front|back): Handle large physical sector disks Date: Tue, 28 May 2013 20:03:40 +0200 Message-ID: <51A4F17C.6060203@canonical.com> References: <5191272A.7070703@canonical.com> <5193A38B.4080508@canonical.com> <20130528124756.GA3754@phenom.dumpdata.com> <51A4A93A.7030804@canonical.com> <20130528161751.GA5035@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1438201776802186337==" Return-path: In-Reply-To: <20130528161751.GA5035@phenom.dumpdata.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: "xen-devel@lists.xensource.com" , Jan Beulich , =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --===============1438201776802186337== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig2F9FF443E661BC0D37A1F8FE" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig2F9FF443E661BC0D37A1F8FE Content-Type: multipart/mixed; boundary="------------090904050407030502020005" This is a multi-part message in MIME format. --------------090904050407030502020005 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 28.05.2013 18:17, Konrad Rzeszutek Wilk wrote: > On Tue, May 28, 2013 at 02:55:22PM +0200, Stefan Bader wrote: >> On 28.05.2013 14:47, Konrad Rzeszutek Wilk wrote: >>> On Wed, May 15, 2013 at 05:02:35PM +0200, Stefan Bader wrote: >>>> Changed the xenbus_gather but kept the xenbus_dev_error (without bei= ng fatal) as >>>> that seems closer to the style of the other calls. >>> >>> Hm, could you rebase this on top of stable/for-jens-3.10 please? >>> >>> I get this: >>> >>> patching file drivers/block/xen-blkback/xenbus.c >>> Hunk #1 FAILED at 704. >>> 1 out of 1 hunk FAILED -- saving rejects to file drivers/block/xen-bl= kback/xenbus.c.rej >>> patching file drivers/block/xen-blkfront.c >>> Hunk #1 FAILED at 542. >>> patch: **** malformed patch at line 133: 16 >>> >>> >> >> I rather suspect another TB fail. Lets try the same on-top-of-Linus an= d xen-git >> as attachements... >=20 > So better, but stable/for-jens-3.10 has some extra changes. >=20 > patch -p1 < /tmp/0001-xen-blk-Use-physical-sector-size-for-setup.patch = > patching file drivers/block/xen-blkback/xenbus.c > Hunk #1 succeeded at 782 (offset 78 lines). > patching file drivers/block/xen-blkfront.c > Hunk #1 FAILED at 542. > Hunk #2 succeeded at 629 (offset 65 lines). > Hunk #3 succeeded at 736 (offset 68 lines). > Hunk #4 FAILED at 736. > Hunk #5 succeeded at 1698 (offset 301 lines). > Hunk #6 succeeded at 1748 (offset 308 lines). > Hunk #7 succeeded at 1811 with fuzz 2 (offset 315 lines). > 2 out of 7 hunks FAILED -- saving rejects to file drivers/block/xen-blk= front.c.rej >=20 >> >> -Stefan >> >=20 Here a quickly done rebase. Best needs a compile test before but I did no= t get to it. --------------090904050407030502020005 Content-Type: text/x-diff; name="0001-xen-blk-Use-physical-sector-size-for-setup.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-xen-blk-Use-physical-sector-size-for-setup.patch" =46rom 4f90805519c9380a9b88853112ed57cc6bd51759 Mon Sep 17 00:00:00 2001 From: Stefan Bader Date: Mon, 13 May 2013 16:28:15 +0200 Subject: [PATCH] xen/blk: Use physical sector size for setup Currently xen-blkback passes the logical sector size over xenbus and xen-blkfront sets up the paravirt disk with that logical block size. But newer drives usually have the logical sector size set to 512 for compatibility reasons and would show the actual sector size only in physical sector size. This results in the device being partitioned and accessed in dom0 with the correct sector size, but the guest thinks 512 bytes is the correct block size. And that results in poor performance. To fix this, blkback gets modified to pass also physical-sector-size over xenbus and blkfront to use both values to set up the paravirt disk. I did not just change the passed in sector-size because I am not sure having a bigger logical sector size than the physical one is valid (and that would happen if a newer dom0 kernel hits an older domU kernel). Also this way a domU set up before should still be accessible (just some tools might detect the unaligned setup). [v2: Make xenbus write failure non-fatal] [v3: Use xenbus_scanf instead of xenbus_gather] [v4: Rebased against segment changes] Signed-off-by: Stefan Bader --- drivers/block/xen-blkback/xenbus.c | 5 +++++ drivers/block/xen-blkfront.c | 21 ++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkba= ck/xenbus.c index 4a4749c..7b06f94 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -782,6 +782,11 @@ again: dev->nodename); goto abort; } + err =3D xenbus_printf(xbt, dev->nodename, "physical-sector-size", "%u",= + bdev_physical_block_size(be->blkif->vbd.bdev)); + if (err) + xenbus_dev_error(dev, err, "writing %s/physical-sector-size", + dev->nodename); =20 err =3D xenbus_transaction_end(xbt, 0); if (err =3D=3D -EAGAIN) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index bac8cf3..f1f75c6 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -607,6 +607,7 @@ wait: } =20 static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size, + unsigned int physical_sector_size, unsigned int segments) { struct request_queue *rq; @@ -629,6 +630,7 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u= 16 sector_size, =20 /* Hard sector size and max sectors impersonate the equiv. hardware. */= blk_queue_logical_block_size(rq, sector_size); + blk_queue_physical_block_size(rq, physical_sector_size); blk_queue_max_hw_sectors(rq, 512); =20 /* Each segment in a request is up to an aligned page in size. */ @@ -735,7 +737,8 @@ static char *encode_disk_name(char *ptr, unsigned int= n) =20 static int xlvbd_alloc_gendisk(blkif_sector_t capacity, struct blkfront_info *info, - u16 vdisk_info, u16 sector_size) + u16 vdisk_info, u16 sector_size, + unsigned int physical_sector_size) { struct gendisk *gd; int nr_minors =3D 1; @@ -802,7 +805,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacit= y, gd->driverfs_dev =3D &(info->xbdev->dev); set_capacity(gd, capacity); =20 - if (xlvbd_init_blk_queue(gd, sector_size, + if (xlvbd_init_blk_queue(gd, sector_size, physical_sector_size, info->max_indirect_segments ? : BLKIF_MAX_SEGMENTS_PER_REQUEST)) { del_gendisk(gd); @@ -1696,6 +1699,7 @@ static void blkfront_connect(struct blkfront_info *= info) { unsigned long long sectors; unsigned long sector_size; + unsigned int physical_sector_size; unsigned int binfo; int err; int barrier, flush, discard, persistent; @@ -1745,6 +1749,16 @@ static void blkfront_connect(struct blkfront_info = *info) return; } =20 + /* + * physcial-sector-size is a newer field, so old backends may not + * provide this. Assume physical sector size to be the same as + * sector_size in that case. + */ + err =3D xenbus_scanf(XBT_NIL, info->xbdev->otherend, + "physical-sector-size", "%u", &physical_sector_size); + if (err !=3D 1) + physical_sector_size =3D sector_size; + info->feature_flush =3D 0; info->flush_op =3D 0; =20 @@ -1798,7 +1812,8 @@ static void blkfront_connect(struct blkfront_info *= info) return; } =20 - err =3D xlvbd_alloc_gendisk(sectors, info, binfo, sector_size); + err =3D xlvbd_alloc_gendisk(sectors, info, binfo, sector_size, + physical_sector_size); if (err) { xenbus_dev_fatal(info->xbdev, err, "xlvbd_add at %s", info->xbdev->otherend); --=20 1.7.9.5 --------------090904050407030502020005-- --------------enig2F9FF443E661BC0D37A1F8FE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCgAGBQJRpPF8AAoJEOhnXe7L7s6jvg4QAMkeqXWDbpEyTVw2FzBd3ECJ u8P+2yQz5TQ+PonkBDd5CqB+SyLUN+6pBRuEEUEKBIfkYtsn5umYuyJhjbwcCRvC rBWYwT8SC0hR5kpjCmsAgEV2/D9uRiCrb6Z2KtT4G+VhkAlxWz4EXoblOAT96Yfy x5+0vGy93ymqwhEWI0tM5NSUJ5Pog+zIWOBA9MlXoZrjj/SR8WQhFEuuaH+mzEfs sqqIZvH0/9tY2+8SWUvTXyTeTu5TBQG2o26UatqnSigdyaTYYsf6HRX8umVH72O1 ofguJGg41iVhVi0DwOEDx/metygeLQlQFNSA54vB7czBkx1FdUp0Eyt5eq9NV8vS nWJ6BPZhlubHiqnv9JE5KOMTl9HvRD4nugbG8eu7rdR0UVJw/L3pP55YBh0XYhzC RaLV8xGY4o1matCIeCMUFrgpbgj1ejEfpy+G8SblrbdWFmRjemZsiYsmtFHrM4Kt VLIAsYXALiqyK/Dx/jDk08jCwtGnny9K17tzwB8SNmB95L+sFhxfoI+4M7e9o0Qh q+YDrW2ovXlWFQDya35XK/t53GJXH4U6wZi1CjKEt1775c2rwFRbnFYnDiEqeoBD SYS7l3ya8J3ht/6+PgzMBon4omhvxOJOjw8pjadqGAImVo0VSHw1ixl3b/Avi4XX jlTFsoZ2os4+1TkudVZ4 =u9m0 -----END PGP SIGNATURE----- --------------enig2F9FF443E661BC0D37A1F8FE-- --===============1438201776802186337== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============1438201776802186337==--