2012-05-28 18:10:42

by Peng Tao

[permalink] [raw]
Subject: [PATCH] pnfsblock: bail out partial page IO

Current block layout driver read/write code assumes page
aligned IO in many places. Add a checker to validate the assumption.

Cc: [email protected]
Signed-off-by: Peng Tao <[email protected]>
---
This is the minimal patch for buffer IO case. I will cook DIO alignment checker
based on it, since DIO isn't needed for stable.

fs/nfs/blocklayout/blocklayout.c | 39 +++++++++++++++++++++++++++++++++++--
1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 7ae8a60..447f8d1 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -228,6 +228,14 @@ bl_end_par_io_read(void *data, int unused)
schedule_work(&rdata->task.u.tk_work);
}

+static bool
+bl_nfspage_aligned(u64 offset, u32 len, u32 blkmask)
+{
+ if ((offset & blkmask) || (len & blkmask))
+ return false;
+ return true;
+}
+
static enum pnfs_try_status
bl_read_pagelist(struct nfs_read_data *rdata)
{
@@ -244,6 +252,9 @@ bl_read_pagelist(struct nfs_read_data *rdata)
dprintk("%s enter nr_pages %u offset %lld count %u\n", __func__,
rdata->pages.npages, f_offset, (unsigned int)rdata->args.count);

+ if (!bl_nfspage_aligned(f_offset, rdata->args.count, PAGE_CACHE_MASK))
+ goto use_mds;
+
par = alloc_parallel(rdata);
if (!par)
goto use_mds;
@@ -552,7 +563,7 @@ bl_write_pagelist(struct nfs_write_data *wdata, int sync)
struct bio *bio = NULL;
struct pnfs_block_extent *be = NULL, *cow_read = NULL;
sector_t isect, last_isect = 0, extent_length = 0;
- struct parallel_io *par;
+ struct parallel_io *par = NULL;
loff_t offset = wdata->args.offset;
size_t count = wdata->args.count;
struct page **pages = wdata->args.pages;
@@ -563,6 +574,10 @@ bl_write_pagelist(struct nfs_write_data *wdata, int sync)
NFS_SERVER(header->inode)->pnfs_blksize >> PAGE_CACHE_SHIFT;

dprintk("%s enter, %Zu@%lld\n", __func__, count, offset);
+ /* Check for alignment first */
+ if (!bl_nfspage_aligned(offset, count, PAGE_CACHE_MASK))
+ goto out_mds;
+
/* At this point, wdata->pages is a (sequential) list of nfs_pages.
* We want to write each, and if there is an error set pnfs_error
* to have it redone using nfs.
@@ -996,14 +1011,32 @@ bl_clear_layoutdriver(struct nfs_server *server)
return 0;
}

+static void
+bl_pg_init_read(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
+{
+ if (!bl_nfspage_aligned(req->wb_offset, req->wb_bytes, PAGE_CACHE_MASK))
+ nfs_pageio_reset_read_mds(pgio);
+ else
+ pnfs_generic_pg_init_read(pgio, req);
+}
+
+static void
+bl_pg_init_write(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
+{
+ if (!bl_nfspage_aligned(req->wb_offset, req->wb_bytes, PAGE_CACHE_MASK))
+ nfs_pageio_reset_write_mds(pgio);
+ else
+ pnfs_generic_pg_init_write(pgio, req);
+}
+
static const struct nfs_pageio_ops bl_pg_read_ops = {
- .pg_init = pnfs_generic_pg_init_read,
+ .pg_init = bl_pg_init_read,
.pg_test = pnfs_generic_pg_test,
.pg_doio = pnfs_generic_pg_readpages,
};

static const struct nfs_pageio_ops bl_pg_write_ops = {
- .pg_init = pnfs_generic_pg_init_write,
+ .pg_init = bl_pg_init_write,
.pg_test = pnfs_generic_pg_test,
.pg_doio = pnfs_generic_pg_writepages,
};
--
1.7.1.262.g5ef3d



2012-05-29 07:07:59

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH] pnfsblock: bail out partial page IO

On 05/28/2012 09:07 PM, Peng Tao wrote:

> Current block layout driver read/write code assumes page
> aligned IO in many places. Add a checker to validate the assumption.
>
> Cc: [email protected]
> Signed-off-by: Peng Tao <[email protected]>
> ---
> This is the minimal patch for buffer IO case. I will cook DIO alignment checker
> based on it, since DIO isn't needed for stable.
>
> fs/nfs/blocklayout/blocklayout.c | 39 +++++++++++++++++++++++++++++++++++--
> 1 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
> index 7ae8a60..447f8d1 100644
> --- a/fs/nfs/blocklayout/blocklayout.c
> +++ b/fs/nfs/blocklayout/blocklayout.c
> @@ -228,6 +228,14 @@ bl_end_par_io_read(void *data, int unused)
> schedule_work(&rdata->task.u.tk_work);
> }
>
> +static bool
> +bl_nfspage_aligned(u64 offset, u32 len, u32 blkmask)
> +{
> + if ((offset & blkmask) || (len & blkmask))
> + return false;
> + return true;
> +}
> +
> static enum pnfs_try_status
> bl_read_pagelist(struct nfs_read_data *rdata)
> {
> @@ -244,6 +252,9 @@ bl_read_pagelist(struct nfs_read_data *rdata)
> dprintk("%s enter nr_pages %u offset %lld count %u\n", __func__,
> rdata->pages.npages, f_offset, (unsigned int)rdata->args.count);
>
> + if (!bl_nfspage_aligned(f_offset, rdata->args.count, PAGE_CACHE_MASK))
> + goto use_mds;
> +


Please put a dprint in the true case so we can see if this hits a lot.
What you are saying is that this should happen very rarely.

After you put the print. Does it hit? what is the test case that causes it?

> par = alloc_parallel(rdata);
> if (!par)
> goto use_mds;
> @@ -552,7 +563,7 @@ bl_write_pagelist(struct nfs_write_data *wdata, int sync)
> struct bio *bio = NULL;
> struct pnfs_block_extent *be = NULL, *cow_read = NULL;
> sector_t isect, last_isect = 0, extent_length = 0;
> - struct parallel_io *par;
> + struct parallel_io *par = NULL;
> loff_t offset = wdata->args.offset;
> size_t count = wdata->args.count;
> struct page **pages = wdata->args.pages;
> @@ -563,6 +574,10 @@ bl_write_pagelist(struct nfs_write_data *wdata, int sync)
> NFS_SERVER(header->inode)->pnfs_blksize >> PAGE_CACHE_SHIFT;
>
> dprintk("%s enter, %Zu@%lld\n", __func__, count, offset);
> + /* Check for alignment first */
> + if (!bl_nfspage_aligned(offset, count, PAGE_CACHE_MASK))
> + goto out_mds;
> +


Also here

> /* At this point, wdata->pages is a (sequential) list of nfs_pages.
> * We want to write each, and if there is an error set pnfs_error
> * to have it redone using nfs.
> @@ -996,14 +1011,32 @@ bl_clear_layoutdriver(struct nfs_server *server)
> return 0;
> }
>
> +static void
> +bl_pg_init_read(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
> +{
> + if (!bl_nfspage_aligned(req->wb_offset, req->wb_bytes, PAGE_CACHE_MASK))
> + nfs_pageio_reset_read_mds(pgio);


Also here

> + else
> + pnfs_generic_pg_init_read(pgio, req);
> +}
> +
> +static void
> +bl_pg_init_write(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
> +{
> + if (!bl_nfspage_aligned(req->wb_offset, req->wb_bytes, PAGE_CACHE_MASK))
> + nfs_pageio_reset_write_mds(pgio);


And here

> + else
> + pnfs_generic_pg_init_write(pgio, req);
> +}
> +
> static const struct nfs_pageio_ops bl_pg_read_ops = {
> - .pg_init = pnfs_generic_pg_init_read,
> + .pg_init = bl_pg_init_read,
> .pg_test = pnfs_generic_pg_test,
> .pg_doio = pnfs_generic_pg_readpages,
> };
>
> static const struct nfs_pageio_ops bl_pg_write_ops = {
> - .pg_init = pnfs_generic_pg_init_write,
> + .pg_init = bl_pg_init_write,
> .pg_test = pnfs_generic_pg_test,
> .pg_doio = pnfs_generic_pg_writepages,
> };


If the prints actually never hit except with some rare applications
than that's fine. But if they do Perhaps a fix is not that hard.

Specially the read case should not be that hard to fix. Because you
are reading into pages, and page boundaries are aligned to page boundaries
in the stream, so all you need to do is read the reminders (at both ends)
as well.

Also the write case is not that hard. You just need to call a read-for-write()
routine that does a sync read, when done continues with today's write.

Cheers
Boaz

2012-05-31 03:33:15

by Peng, Tao

[permalink] [raw]
Subject: RE: [PATCH] pnfsblock: bail out partial page IO

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBCb2F6IEhhcnJvc2ggW21haWx0
bzpiaGFycm9zaEBwYW5hc2FzLmNvbV0NCj4gU2VudDogVHVlc2RheSwgTWF5IDI5LCAyMDEyIDM6
MDcgUE0NCj4gVG86IFBlbmcgVGFvDQo+IENjOiBUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbTsg
bGludXgtbmZzQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIua2VybmVsLm9yZzsgUGVuZywg
VGFvDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIHBuZnNibG9jazogYmFpbCBvdXQgcGFydGlhbCBw
YWdlIElPDQo+IA0KPiBPbiAwNS8yOC8yMDEyIDA5OjA3IFBNLCBQZW5nIFRhbyB3cm90ZToNCj4g
DQo+ID4gQ3VycmVudCBibG9jayBsYXlvdXQgZHJpdmVyIHJlYWQvd3JpdGUgY29kZSBhc3N1bWVz
IHBhZ2UNCj4gPiBhbGlnbmVkIElPIGluIG1hbnkgcGxhY2VzLiBBZGQgYSBjaGVja2VyIHRvIHZh
bGlkYXRlIHRoZSBhc3N1bXB0aW9uLg0KPiA+DQo+ID4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5v
cmcNCj4gPiBTaWduZWQtb2ZmLWJ5OiBQZW5nIFRhbyA8dGFvLnBlbmdAZW1jLmNvbT4NCj4gPiAt
LS0NCj4gPiBUaGlzIGlzIHRoZSBtaW5pbWFsIHBhdGNoIGZvciBidWZmZXIgSU8gY2FzZS4gSSB3
aWxsIGNvb2sgRElPIGFsaWdubWVudCBjaGVja2VyDQo+ID4gYmFzZWQgb24gaXQsIHNpbmNlIERJ
TyBpc24ndCBuZWVkZWQgZm9yIHN0YWJsZS4NCj4gPg0KPiA+ICBmcy9uZnMvYmxvY2tsYXlvdXQv
YmxvY2tsYXlvdXQuYyB8ICAgMzkgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKyst
LQ0KPiA+ICAxIGZpbGVzIGNoYW5nZWQsIDM2IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0p
DQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2Jsb2NrbGF5b3V0L2Jsb2NrbGF5b3V0LmMg
Yi9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQuYw0KPiA+IGluZGV4IDdhZThhNjAuLjQ0
N2Y4ZDEgMTAwNjQ0DQo+ID4gLS0tIGEvZnMvbmZzL2Jsb2NrbGF5b3V0L2Jsb2NrbGF5b3V0LmMN
Cj4gPiArKysgYi9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXQuYw0KPiA+IEBAIC0yMjgs
NiArMjI4LDE0IEBAIGJsX2VuZF9wYXJfaW9fcmVhZCh2b2lkICpkYXRhLCBpbnQgdW51c2VkKQ0K
PiA+ICAJc2NoZWR1bGVfd29yaygmcmRhdGEtPnRhc2sudS50a193b3JrKTsNCj4gPiAgfQ0KPiA+
DQo+ID4gK3N0YXRpYyBib29sDQo+ID4gK2JsX25mc3BhZ2VfYWxpZ25lZCh1NjQgb2Zmc2V0LCB1
MzIgbGVuLCB1MzIgYmxrbWFzaykNCj4gPiArew0KPiA+ICsJaWYgKChvZmZzZXQgJiBibGttYXNr
KSB8fCAobGVuICYgYmxrbWFzaykpDQo+ID4gKwkJcmV0dXJuIGZhbHNlOw0KPiA+ICsJcmV0dXJu
IHRydWU7DQo+ID4gK30NCj4gPiArDQo+ID4gIHN0YXRpYyBlbnVtIHBuZnNfdHJ5X3N0YXR1cw0K
PiA+ICBibF9yZWFkX3BhZ2VsaXN0KHN0cnVjdCBuZnNfcmVhZF9kYXRhICpyZGF0YSkNCj4gPiAg
ew0KPiA+IEBAIC0yNDQsNiArMjUyLDkgQEAgYmxfcmVhZF9wYWdlbGlzdChzdHJ1Y3QgbmZzX3Jl
YWRfZGF0YSAqcmRhdGEpDQo+ID4gIAlkcHJpbnRrKCIlcyBlbnRlciBucl9wYWdlcyAldSBvZmZz
ZXQgJWxsZCBjb3VudCAldVxuIiwgX19mdW5jX18sDQo+ID4gIAkgICAgICAgcmRhdGEtPnBhZ2Vz
Lm5wYWdlcywgZl9vZmZzZXQsICh1bnNpZ25lZCBpbnQpcmRhdGEtPmFyZ3MuY291bnQpOw0KPiA+
DQo+ID4gKwlpZiAoIWJsX25mc3BhZ2VfYWxpZ25lZChmX29mZnNldCwgcmRhdGEtPmFyZ3MuY291
bnQsIFBBR0VfQ0FDSEVfTUFTSykpDQo+ID4gKwkJZ290byB1c2VfbWRzOw0KPiA+ICsNCj4gDQo+
IA0KPiBQbGVhc2UgcHV0IGEgZHByaW50IGluIHRoZSB0cnVlIGNhc2Ugc28gd2UgY2FuIHNlZSBp
ZiB0aGlzIGhpdHMgYSBsb3QuDQo+IFdoYXQgeW91IGFyZSBzYXlpbmcgaXMgdGhhdCB0aGlzIHNo
b3VsZCBoYXBwZW4gdmVyeSByYXJlbHkuDQo+IA0KPiBBZnRlciB5b3UgcHV0IHRoZSBwcmludC4g
RG9lcyBpdCBoaXQ/IHdoYXQgaXMgdGhlIHRlc3QgY2FzZSB0aGF0IGNhdXNlcyBpdD8NCj4gDQo+
ID4gIAlwYXIgPSBhbGxvY19wYXJhbGxlbChyZGF0YSk7DQo+ID4gIAlpZiAoIXBhcikNCj4gPiAg
CQlnb3RvIHVzZV9tZHM7DQo+ID4gQEAgLTU1Miw3ICs1NjMsNyBAQCBibF93cml0ZV9wYWdlbGlz
dChzdHJ1Y3QgbmZzX3dyaXRlX2RhdGEgKndkYXRhLCBpbnQgc3luYykNCj4gPiAgCXN0cnVjdCBi
aW8gKmJpbyA9IE5VTEw7DQo+ID4gIAlzdHJ1Y3QgcG5mc19ibG9ja19leHRlbnQgKmJlID0gTlVM
TCwgKmNvd19yZWFkID0gTlVMTDsNCj4gPiAgCXNlY3Rvcl90IGlzZWN0LCBsYXN0X2lzZWN0ID0g
MCwgZXh0ZW50X2xlbmd0aCA9IDA7DQo+ID4gLQlzdHJ1Y3QgcGFyYWxsZWxfaW8gKnBhcjsNCj4g
PiArCXN0cnVjdCBwYXJhbGxlbF9pbyAqcGFyID0gTlVMTDsNCj4gPiAgCWxvZmZfdCBvZmZzZXQg
PSB3ZGF0YS0+YXJncy5vZmZzZXQ7DQo+ID4gIAlzaXplX3QgY291bnQgPSB3ZGF0YS0+YXJncy5j
b3VudDsNCj4gPiAgCXN0cnVjdCBwYWdlICoqcGFnZXMgPSB3ZGF0YS0+YXJncy5wYWdlczsNCj4g
PiBAQCAtNTYzLDYgKzU3NCwxMCBAQCBibF93cml0ZV9wYWdlbGlzdChzdHJ1Y3QgbmZzX3dyaXRl
X2RhdGEgKndkYXRhLCBpbnQgc3luYykNCj4gPiAgCSAgICBORlNfU0VSVkVSKGhlYWRlci0+aW5v
ZGUpLT5wbmZzX2Jsa3NpemUgPj4gUEFHRV9DQUNIRV9TSElGVDsNCj4gPg0KPiA+ICAJZHByaW50
aygiJXMgZW50ZXIsICVadUAlbGxkXG4iLCBfX2Z1bmNfXywgY291bnQsIG9mZnNldCk7DQo+ID4g
KwkvKiBDaGVjayBmb3IgYWxpZ25tZW50IGZpcnN0ICovDQo+ID4gKwlpZiAoIWJsX25mc3BhZ2Vf
YWxpZ25lZChvZmZzZXQsIGNvdW50LCBQQUdFX0NBQ0hFX01BU0spKQ0KPiA+ICsJCWdvdG8gb3V0
X21kczsNCj4gPiArDQo+IA0KPiANCj4gQWxzbyBoZXJlDQo+IA0KPiA+ICAJLyogQXQgdGhpcyBw
b2ludCwgd2RhdGEtPnBhZ2VzIGlzIGEgKHNlcXVlbnRpYWwpIGxpc3Qgb2YgbmZzX3BhZ2VzLg0K
PiA+ICAJICogV2Ugd2FudCB0byB3cml0ZSBlYWNoLCBhbmQgaWYgdGhlcmUgaXMgYW4gZXJyb3Ig
c2V0IHBuZnNfZXJyb3INCj4gPiAgCSAqIHRvIGhhdmUgaXQgcmVkb25lIHVzaW5nIG5mcy4NCj4g
PiBAQCAtOTk2LDE0ICsxMDExLDMyIEBAIGJsX2NsZWFyX2xheW91dGRyaXZlcihzdHJ1Y3QgbmZz
X3NlcnZlciAqc2VydmVyKQ0KPiA+ICAJcmV0dXJuIDA7DQo+ID4gIH0NCj4gPg0KPiA+ICtzdGF0
aWMgdm9pZA0KPiA+ICtibF9wZ19pbml0X3JlYWQoc3RydWN0IG5mc19wYWdlaW9fZGVzY3JpcHRv
ciAqcGdpbywgc3RydWN0IG5mc19wYWdlICpyZXEpDQo+ID4gK3sNCj4gPiArCWlmICghYmxfbmZz
cGFnZV9hbGlnbmVkKHJlcS0+d2Jfb2Zmc2V0LCByZXEtPndiX2J5dGVzLCBQQUdFX0NBQ0hFX01B
U0spKQ0KPiA+ICsJCW5mc19wYWdlaW9fcmVzZXRfcmVhZF9tZHMocGdpbyk7DQo+IA0KPiANCj4g
QWxzbyBoZXJlDQo+IA0KPiA+ICsJZWxzZQ0KPiA+ICsJCXBuZnNfZ2VuZXJpY19wZ19pbml0X3Jl
YWQocGdpbywgcmVxKTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3RhdGljIHZvaWQNCj4gPiArYmxf
cGdfaW5pdF93cml0ZShzdHJ1Y3QgbmZzX3BhZ2Vpb19kZXNjcmlwdG9yICpwZ2lvLCBzdHJ1Y3Qg
bmZzX3BhZ2UgKnJlcSkNCj4gPiArew0KPiA+ICsJaWYgKCFibF9uZnNwYWdlX2FsaWduZWQocmVx
LT53Yl9vZmZzZXQsIHJlcS0+d2JfYnl0ZXMsIFBBR0VfQ0FDSEVfTUFTSykpDQo+ID4gKwkJbmZz
X3BhZ2Vpb19yZXNldF93cml0ZV9tZHMocGdpbyk7DQo+IA0KPiANCj4gQW5kIGhlcmUNCj4gDQo+
ID4gKwllbHNlDQo+ID4gKwkJcG5mc19nZW5lcmljX3BnX2luaXRfd3JpdGUocGdpbywgcmVxKTsN
Cj4gPiArfQ0KPiA+ICsNCj4gPiAgc3RhdGljIGNvbnN0IHN0cnVjdCBuZnNfcGFnZWlvX29wcyBi
bF9wZ19yZWFkX29wcyA9IHsNCj4gPiAtCS5wZ19pbml0ID0gcG5mc19nZW5lcmljX3BnX2luaXRf
cmVhZCwNCj4gPiArCS5wZ19pbml0ID0gYmxfcGdfaW5pdF9yZWFkLA0KPiA+ICAJLnBnX3Rlc3Qg
PSBwbmZzX2dlbmVyaWNfcGdfdGVzdCwNCj4gPiAgCS5wZ19kb2lvID0gcG5mc19nZW5lcmljX3Bn
X3JlYWRwYWdlcywNCj4gPiAgfTsNCj4gPg0KPiA+ICBzdGF0aWMgY29uc3Qgc3RydWN0IG5mc19w
YWdlaW9fb3BzIGJsX3BnX3dyaXRlX29wcyA9IHsNCj4gPiAtCS5wZ19pbml0ID0gcG5mc19nZW5l
cmljX3BnX2luaXRfd3JpdGUsDQo+ID4gKwkucGdfaW5pdCA9IGJsX3BnX2luaXRfd3JpdGUsDQo+
ID4gIAkucGdfdGVzdCA9IHBuZnNfZ2VuZXJpY19wZ190ZXN0LA0KPiA+ICAJLnBnX2RvaW8gPSBw
bmZzX2dlbmVyaWNfcGdfd3JpdGVwYWdlcywNCj4gPiAgfTsNCj4gDQo+IA0KPiBJZiB0aGUgcHJp
bnRzIGFjdHVhbGx5IG5ldmVyIGhpdCBleGNlcHQgd2l0aCBzb21lIHJhcmUgYXBwbGljYXRpb25z
DQo+IHRoYW4gdGhhdCdzIGZpbmUuIEJ1dCBpZiB0aGV5IGRvIFBlcmhhcHMgYSBmaXggaXMgbm90
IHRoYXQgaGFyZC4NCj4gDQpCb2F6LCBoaSwNCg0KVGhhbmsgeW91IHZlcnkgbXVjaCBmb3IgcmV2
aWV3aW5nIHRoZXNlIHBhdGNoZXMuIFNvcnJ5IHRoYXQgSSd2ZSBiZWVuIGhhdmluZyBzb21lIGlz
c3VlcyB3aXRoIG15IHRlc3RpbmcgZW52aXJvbm1lbnQuIEkgd2lsbCBjb21lIGJhY2sgdG8gdGhp
cyBvbmNlIEkgc29ydCBvdXQgdGhlIGVudmlyb25tZW50IGlzc3Vlcy4NCg0KSSBoYXZlbid0IHRl
c3RlZCB5ZXQgYnV0IEkgdGVuZCB0byBiZWxpZXZlIHlvdSBhcmUgY29ycmVjdC4gRXZlbiBpZiBn
ZW5lcmljIE5GUyBpbml0aWF0ZXMgcmVhZC1tb2RpZnktd3JpdGUgY3ljbGUsIHRoZSBuZnNfcGFn
ZSBwYXNzZWQgdG8gTEQgZG9lc24ndCBhbHdheXMgY29udGFpbiBlbnRpcmUgcGFnZSBidXQganVz
dCB0aGUgcmFuZ2UgdGhhdCBpcyB3cml0dGVuIGJ5IGFwcGxpY2F0aW9uIGJlY2F1c2UgaW4gZmxv
Y2sgb3IgT19EU1lOQyBjYXNlLCBuZnNfdXBkYXRlcGFnZSgpIGRvZXNuJ3QgZXh0ZW5kIHRoZSB3
cml0ZSB0byBlbnRpcmUgcGFnZS4gU28gdGhlIHRlc3QgaW4gcGdfaW5pdCB3aWxsIGZhaWwgZXZl
biB0aG91Z2ggaXQgaXMgc2FmZSBmb3IgTEQgdG8gd3JpdGUgb3V0IHRoZSBlbnRpcmUgcGFnZSBp
biB0aGVzZSBjYXNlcy4NCg0KU28gdG8gc3VtbWFyeSwgSU1PIHRoZSBhbGlnbm1lbnQgdGVzdHMg
d2lsbCBmYWlsIGluIGZvbGxvd2luZyBjYXNlcyB3aGVuIHBhcnRpYWwgcGFnZSB3cml0ZSBpcyBp
c3N1ZWQ6DQoxLiBuZnNfd2FudF9yZWFkX21vZGlmeV93cml0ZSgpIHJldHVybnMgZmFsc2UgKGUu
Zy4sIGZpbGUgb3BlbmVkIE9fV1JPTkxZKQ0KMi4gYnl0ZSByYW5nZSBsb2NraW5nIGlzIHVzZWQN
CjMuIGZpbGUgb3BlbmVkIHdpdGggRF9TWU5DDQoNCkFuZCBvbmx5IHRoZSBmaXJzdCBjYXNlIHdp
bGwgY2F1c2UgZGF0YSBjb3JydXB0aW9uIGlmIHdlIGRvIG5vdCBiYWlsb3V0IHRoZSB3cml0ZS4g
U28gdGhlIGJhaWxvdXQgbG9naWMgaW4gdGhlIHBhdGNoIGlzIG11Y2ggc3RyaWN0ZXIgdGhhbiBp
dCByZWFsbHkgbmVlZHMgdG8gYmUuDQoNCj4gU3BlY2lhbGx5IHRoZSByZWFkIGNhc2Ugc2hvdWxk
IG5vdCBiZSB0aGF0IGhhcmQgdG8gZml4LiBCZWNhdXNlIHlvdQ0KPiBhcmUgcmVhZGluZyBpbnRv
IHBhZ2VzLCBhbmQgcGFnZSBib3VuZGFyaWVzIGFyZSBhbGlnbmVkIHRvIHBhZ2UgYm91bmRhcmll
cw0KPiBpbiB0aGUgc3RyZWFtLCBzbyBhbGwgeW91IG5lZWQgdG8gZG8gaXMgcmVhZCB0aGUgcmVt
aW5kZXJzIChhdCBib3RoIGVuZHMpDQo+IGFzIHdlbGwuDQpZZXMuIEN1cnJlbnQgcmVhZCBjb2Rl
IGlzIHN1ZmZpY2llbnQgZW5vdWdoLg0KDQo+IA0KPiBBbHNvIHRoZSB3cml0ZSBjYXNlIGlzIG5v
dCB0aGF0IGhhcmQuIFlvdSBqdXN0IG5lZWQgdG8gY2FsbCBhIHJlYWQtZm9yLXdyaXRlKCkNCj4g
cm91dGluZSB0aGF0IGRvZXMgYSBzeW5jIHJlYWQsIHdoZW4gZG9uZSBjb250aW51ZXMgd2l0aCB0
b2RheSdzIHdyaXRlLg0KSW4gdGhlIHdyaXRlIGNhc2UsIHJlYWQtZm9yLXdyaXRlIHJvdXRpbmUg
aXMgb25seSBuZWVkZWQgd2hlbiBhcHBsaWNhdGlvbiBkb2VzIHBhcnRpYWwgcGFnZSB3cml0ZSB0
byB1bmluaXRpYWxpemVkIGV4dGVudC4gVGhlcmUgaXMgYWxyZWFkeSBzb21lIHJlYWQtZm9yLXdy
aXRlIGxvZ2ljIHRoZXJlIGJ1dCBpdCBhc3N1bWVzIHBhZ2UgYWxpZ25lZCBJTy4gU28gaXQgbmVl
ZHMgdG8gYmUgZXh0ZW5kZWQgdG8gaGFuZGxlIHBhcnRpYWwgcGFnZXMgYXMgd2VsbC4gQWxzbyB0
aGUgaW8gc3VtbWl0IHJvdXRpbmUgbmVlZCB0byBiZSBjaGFuZ2VkIHRvIGhhbmRsZSBwYXJ0aWFs
IHdyaXRlLCB3aGljaCBpcyBlYXN5IEJUVy4NCg0KSG93ZXZlciwgZXZlbiB3aXRoIGFib3ZlIGNo
YW5nZSwgdGhlIGRpcmVjdCB3cml0ZSBjYXNlIHN0aWxsIG5lZWQgdG8gYmUgYmxvY2sgc2l6ZSBh
bGlnbmVkIGJlY2F1c2Ugb3RoZXJ3aXNlIHdlIG5lZWQgdG8gYWxsb2NhdGUgZXh0cmEgcGFnZXMg
dG8gemVybyBvdXQgcGFydGlhbCB1bmluaXRpYWxpemVkIGV4dGVudHMgYW5kIGl0IHdvdWxkIHJl
cXVpcmUgc2VyaWFsaXphdGlvbiBiZXR3ZWVuIGNvbmN1cnJlbnQgd3JpdGVycy4gT25lIHBvc3Np
YmxlIG9wdGltaXphdGlvbiBpcyB0byBwcmUtY2hlY2sgaW4gYmxfd3JpdGVfcGFnZWxpc3Qgd2hl
dGhlciB0aGUgd3JpdGUgb3V0IHJhbmdlIGNvbnRhaW5zIHVuaW5pdGlhbGl6ZWQgZXh0ZW50cyB3
aGVuIElPIGlzIHNlY3RvciBhbGlnbmVkIGJ1dCBibG9jayBzaXplIHVuYWxpZ25lZC4gQW5kIGlm
IHRoZSByYW5nZSBkb2Vzbid0IGNvbnRhaW4gYW55IHVuaW5pdGlhbGl6ZWQgZXh0ZW50cywgaXQg
Y2FuIHRoZW4gYmUgaXNzdWVkLCBvdGhlcndpc2UgaXQgc3RpbGwgbmVlZCB0byBiZSByZWplY3Rl
ZC4NCg0KVGhhbmtzLA0KVGFvDQoNCg==

2012-05-28 19:20:29

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH] pnfsblock: bail out partial page IO

Peng Tao wrote:

Current block layout driver read/write code assumes page
aligned IO in many places. Add a checker to validate the assumption.

This patch has some problems when compiled for 64 bit.

CC [M] fs/nfs/blocklayout/blocklayout.o
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c: In function ‘bl_read_pagelist’:
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c:255:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c: In function ‘bl_write_pagelist’:
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c:578:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c: In function ‘bl_pg_init_read’:
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c:1017:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c: In function ‘bl_pg_init_write’:
/home/rees/src/pnfs/linux/fs/nfs/blocklayout/blocklayout.c:1026:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]