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 14:55:22 +0200 Message-ID: <51A4A93A.7030804@canonical.com> References: <5191272A.7070703@canonical.com> <5193A38B.4080508@canonical.com> <20130528124756.GA3754@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1001699357429781057==" Return-path: In-Reply-To: <20130528124756.GA3754@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) --===============1001699357429781057== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig0B4BDF364A23D4AF230A67A8" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig0B4BDF364A23D4AF230A67A8 Content-Type: multipart/mixed; boundary="------------060701010502090107090709" This is a multi-part message in MIME format. --------------060701010502090107090709 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 being= fatal) as >> that seems closer to the style of the other calls. >=20 > Hm, could you rebase this on top of stable/for-jens-3.10 please? >=20 > I get this: >=20 > 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-blkb= ack/xenbus.c.rej > patching file drivers/block/xen-blkfront.c > Hunk #1 FAILED at 542. > patch: **** malformed patch at line 133: 16 >=20 >=20 I rather suspect another TB fail. Lets try the same on-top-of-Linus and x= en-git as attachements... -Stefan --------------060701010502090107090709 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 e1bc2a0027c20679bfc81beb3f2e888b283a44f2 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] Signed-off-by: Stefan Bader --- drivers/block/xen-blkback/xenbus.c | 5 +++++ drivers/block/xen-blkfront.c | 23 +++++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkba= ck/xenbus.c index 8bfd1bc..25463e3 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -704,6 +704,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 d89ef86..33b5618 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -542,7 +542,8 @@ wait: flush_requests(info); } =20 -static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size) +static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size, + unsigned int physical_sector_size) { struct request_queue *rq; struct blkfront_info *info =3D gd->private_data; @@ -564,6 +565,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. */ @@ -667,7 +669,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; @@ -734,7 +737,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)) { del_gendisk(gd); goto release; } @@ -1395,6 +1398,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; @@ -1437,6 +1441,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 @@ -1483,7 +1497,8 @@ static void blkfront_connect(struct blkfront_info *= info) else info->feature_persistent =3D persistent; =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 --------------060701010502090107090709 Content-Type: text/x-diff; name="0001-blkif.h-Document-the-physical-sector-size-extension.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-blkif.h-Document-the-physical-sector-size-extension.pat"; filename*1="ch" =46rom 8d1023ce11b9067346e9794d95b2876d98484f43 Mon Sep 17 00:00:00 2001 From: Stefan Bader Date: Wed, 22 May 2013 15:11:18 +0200 Subject: [PATCH] blkif.h: Document the physical-sector-size extension Signed-off-by: Stefan Bader --- xen/include/public/io/blkif.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.= h index 97b423b..f7c3366 100644 --- a/xen/include/public/io/blkif.h +++ b/xen/include/public/io/blkif.h @@ -208,12 +208,17 @@ * sector-size * Values: * - * The native sector size, in bytes, of the backend device. + * The logical sector size, in bytes, of the backend device. + * + * physical-sector-size + * Values: + * + * The physical sector size, in bytes, of the backend device. * * sectors * Values: * - * The size of the backend device, expressed in units of its native= + * The size of the backend device, expressed in units of its logica= l * sector size ("sector-size"). * ***********************************************************************= ****** @@ -473,8 +478,9 @@ * NB. first_sect and last_sect in blkif_request_segment, as well as * sector_number in blkif_request, are always expressed in 512-byte unit= s. * However they must be properly aligned to the real sector size of the - * physical disk, which is reported in the "sector-size" node in the bac= kend - * xenbus info. Also the xenbus "sectors" node is expressed in 512-byte = units. + * physical disk, which is reported in the "physical-sector-size" node i= n + * the backend xenbus info. Also the xenbus "sectors" node is expressed = in + * 512-byte units. */ struct blkif_request_segment { grant_ref_t gref; /* reference to I/O buffer frame */ --=20 1.7.9.5 --------------060701010502090107090709-- --------------enig0B4BDF364A23D4AF230A67A8 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/ iQIcBAEBCgAGBQJRpKk6AAoJEOhnXe7L7s6jqEoP/3tNeq/jVu2QHkAcX2EAmDMZ jD1HV2KT/hobVR+DoGmF7lFWglKxwPpXESyOPlJwu/cjwDVBKXjskukpQZT0VtTf pmu+nTewP6K03o8LFOYx7ZAiqK3JSGpWadgvaS9JBzq2XFYcln/wfshVuGB2KwzL CgCr3bxWbVtSy3bG1PGVk/rzDxyUrY1O4PrBEc/R5BMxcjh4luOnxZWa9O09BMMt mIzVUbDZZJQMU1Mb7kD49iMuz0RTBbPiYl31gjO8rF4BLUQKJmKcCwARkFTole3r 0ONhoA0YPXCvlLRI3KKLt8yD3b+I33/S8/YBxachfPkwEmY9Jd01z451ke30u7nl ikQYTkDEQLjBMH9TLdAP3DHJEPGtpTNLfPUcb4oU9g7R+7+XoVUNHnAdSnpmfY6u B1oYTJTX8I3MGGId4dkg+YbWI99MMhV5YXOJbS+hSLSr7uOWVkW3TB5+cZ5IVdwx 8ylOrRocFpuuckkerJ/yiOlLZP1JEs9k///l+Db7Dsuc6YKtVftvAXvqUR5Dd0rC Wi37q+JiFcTNgVfkzd2f/iYGWbneMgTJOoRDAQyY+I7OCzNAAAotMK4j1Nf7eHQp eGqpgXqxiHmCmbeOgQd7ktxAJJJQ2/+HDi1c494H024qACaz7Pj6/Hx/GLPlhnez XlJYC6xrBf/2h/goaC0P =yfW5 -----END PGP SIGNATURE----- --------------enig0B4BDF364A23D4AF230A67A8-- --===============1001699357429781057== 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 --===============1001699357429781057==--