2012-08-08 01:49:55

by Peng Tao

[permalink] [raw]
Subject: [PATCH 1/2] NFSv41: fix DIO write_io calculation

pnfs_within_mdsthreshold() is called inside pg_init. We need
to set read_io/write_io before that.

Cc: Andy Adamson <[email protected]>
Signed-off-by: Peng Tao <[email protected]>
---
fs/nfs/direct.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 1ba385b..34a6282 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -463,10 +463,10 @@ static ssize_t nfs_direct_read(struct kiocb *iocb, const struct iovec *iov,
if (!is_sync_kiocb(iocb))
dreq->iocb = iocb;

+ NFS_I(inode)->read_io += iov_length(iov, nr_segs);
result = nfs_direct_read_schedule_iovec(dreq, iov, nr_segs, pos, uio);
if (!result)
result = nfs_direct_wait(dreq);
- NFS_I(inode)->read_io += result;
out_release:
nfs_direct_req_release(dreq);
out:
@@ -814,6 +814,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
get_dreq(dreq);
atomic_inc(&inode->i_dio_count);

+ NFS_I(dreq->inode)->write_io += iov_length(iov, nr_segs);
for (seg = 0; seg < nr_segs; seg++) {
const struct iovec *vec = &iov[seg];
result = nfs_direct_write_schedule_segment(&desc, vec, pos, uio);
@@ -825,7 +826,6 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
pos += vec->iov_len;
}
nfs_pageio_complete(&desc);
- NFS_I(dreq->inode)->write_io += desc.pg_bytes_written;

/*
* If no bytes were started, return the error, and let the
--
1.7.1.262.g5ef3d



2012-08-09 14:25:43

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv41: fix DIO write_io calculation

T24gVGh1LCAyMDEyLTA4LTA5IGF0IDEwOjM2ICswODAwLCBQZW5nIFRhbyB3cm90ZToNCj4gT24g
VGh1LCBBdWcgOSwgMjAxMiBhdCAzOjAzIEFNLCBNeWtsZWJ1c3QsIFRyb25kDQo+IDxUcm9uZC5N
eWtsZWJ1c3RAbmV0YXBwLmNvbT4gd3JvdGU6DQo+ID4gT24gV2VkLCAyMDEyLTA4LTA4IGF0IDA5
OjQ5ICswODAwLCBQZW5nIFRhbyB3cm90ZToNCj4gPj4gcG5mc193aXRoaW5fbWRzdGhyZXNob2xk
KCkgaXMgY2FsbGVkIGluc2lkZSBwZ19pbml0LiBXZSBuZWVkDQo+ID4+IHRvIHNldCByZWFkX2lv
L3dyaXRlX2lvIGJlZm9yZSB0aGF0Lg0KPiA+DQo+ID4gV2h5Pw0KPiBJZiB3ZSBkb24ndCBzZXQg
cmVhZF9pby93cml0ZV9pbyBiZWZvcmUgY2FsbGluZyBwZ19pbml0LCB3ZSBmYWlsDQo+IHBuZnNf
d2l0aGluX21kc3RocmVzaG9sZCgpIGFuZCBJTyBnb2VzIHRvIE1EUy4NCj4gQSBzaW1wbGUgdGVz
dCBjYXNlOg0KPiBkZCBpZj1mb28gb2Y9L21udC9wbmZzL2JhciBicz0xME0gY291bnQ9MSBvZmxh
Zz1kaXJlY3QNCj4gDQoNCk15IHBvaW50IHdhcyB0aGF0IHRoaXMgZXhwbGFuYXRpb24gbmVlZHMg
dG8gZ28gaW50byB0aGUgY2hhbmdlbG9nIG9mIHRoZQ0KY29tbWl0Lg0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-08-08 14:49:20

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS41: fix error of setting blocklayoutdriver

On Wed, Aug 8, 2012 at 8:49 PM, Bryan Schumaker <[email protected]> wrote:
> On 08/07/2012 09:49 PM, Peng Tao wrote:
>> After commit e38eb650 (NFS: set_pnfs_layoutdriver() from
>> nfs4_proc_fsinfo()), set_pnfs_layoutdriver() is called inside
>> nfs4_proc_fsinfo(), but pnfs_blksize is not set. It causes setting
>> blocklayoutdriver failure and pnfsblock mount failure.
>>
>> Cc: Bryan Schumaker <[email protected]>
>> Signed-off-by: Peng Tao <[email protected]>
>> ---
>> fs/nfs/nfs4proc.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 78b2163..299c311 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3362,8 +3362,11 @@ static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s
>>
>> nfs_fattr_init(fsinfo->fattr);
>> error = nfs4_do_fsinfo(server, fhandle, fsinfo);
>> - if (error == 0)
>> + if (error == 0) {
>> + /* block layout checks this! */
>> + server->pnfs_blksize = fsinfo->blksize;
>
> fs/nfs/client.c:nfs_server_set_fsinfo() also sets this field, so I'm surprised there was a problem. You should remove the same line from nfs_server_set_fsinfo() if you're going to set it here.
Before commit e38eb650, nfs_server_set_fsinfo() is called before
setting layoutdriver. And now it is called after. And block layout
driver checks pnfs_blksize so it gets broken.

I will remove the line in nfs_server_set_fsinfo() and resend the patch.

--
Thanks,
Tao

2012-08-08 12:49:35

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 2/2] NFS41: fix error of setting blocklayoutdriver

On 08/07/2012 09:49 PM, Peng Tao wrote:
> After commit e38eb650 (NFS: set_pnfs_layoutdriver() from
> nfs4_proc_fsinfo()), set_pnfs_layoutdriver() is called inside
> nfs4_proc_fsinfo(), but pnfs_blksize is not set. It causes setting
> blocklayoutdriver failure and pnfsblock mount failure.
>
> Cc: Bryan Schumaker <[email protected]>
> Signed-off-by: Peng Tao <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 78b2163..299c311 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3362,8 +3362,11 @@ static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s
>
> nfs_fattr_init(fsinfo->fattr);
> error = nfs4_do_fsinfo(server, fhandle, fsinfo);
> - if (error == 0)
> + if (error == 0) {
> + /* block layout checks this! */
> + server->pnfs_blksize = fsinfo->blksize;

fs/nfs/client.c:nfs_server_set_fsinfo() also sets this field, so I'm surprised there was a problem. You should remove the same line from nfs_server_set_fsinfo() if you're going to set it here.

- Bryan

> set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
> + }
>
> return error;
> }
>


2012-08-09 02:36:46

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv41: fix DIO write_io calculation

On Thu, Aug 9, 2012 at 3:03 AM, Myklebust, Trond
<[email protected]> wrote:
> On Wed, 2012-08-08 at 09:49 +0800, Peng Tao wrote:
>> pnfs_within_mdsthreshold() is called inside pg_init. We need
>> to set read_io/write_io before that.
>
> Why?
If we don't set read_io/write_io before calling pg_init, we fail
pnfs_within_mdsthreshold() and IO goes to MDS.
A simple test case:
dd if=foo of=/mnt/pnfs/bar bs=10M count=1 oflag=direct

--
Thanks,
Tao

2012-08-08 01:50:04

by Peng Tao

[permalink] [raw]
Subject: [PATCH 2/2] NFS41: fix error of setting blocklayoutdriver

After commit e38eb650 (NFS: set_pnfs_layoutdriver() from
nfs4_proc_fsinfo()), set_pnfs_layoutdriver() is called inside
nfs4_proc_fsinfo(), but pnfs_blksize is not set. It causes setting
blocklayoutdriver failure and pnfsblock mount failure.

Cc: Bryan Schumaker <[email protected]>
Signed-off-by: Peng Tao <[email protected]>
---
fs/nfs/nfs4proc.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 78b2163..299c311 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3362,8 +3362,11 @@ static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s

nfs_fattr_init(fsinfo->fattr);
error = nfs4_do_fsinfo(server, fhandle, fsinfo);
- if (error == 0)
+ if (error == 0) {
+ /* block layout checks this! */
+ server->pnfs_blksize = fsinfo->blksize;
set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
+ }

return error;
}
--
1.7.1.262.g5ef3d


2012-08-08 19:04:17

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv41: fix DIO write_io calculation

T24gV2VkLCAyMDEyLTA4LTA4IGF0IDA5OjQ5ICswODAwLCBQZW5nIFRhbyB3cm90ZToNCj4gcG5m
c193aXRoaW5fbWRzdGhyZXNob2xkKCkgaXMgY2FsbGVkIGluc2lkZSBwZ19pbml0LiBXZSBuZWVk
DQo+IHRvIHNldCByZWFkX2lvL3dyaXRlX2lvIGJlZm9yZSB0aGF0Lg0KDQpXaHk/DQoNCi0tIA0K
VHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpU
cm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

2012-08-09 14:32:06

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH 1/2] NFSv41: fix DIO write_io calculation

On Thu, Aug 9, 2012 at 10:25 PM, Myklebust, Trond
<[email protected]> wrote:
> On Thu, 2012-08-09 at 10:36 +0800, Peng Tao wrote:
>> On Thu, Aug 9, 2012 at 3:03 AM, Myklebust, Trond
>> <[email protected]> wrote:
>> > On Wed, 2012-08-08 at 09:49 +0800, Peng Tao wrote:
>> >> pnfs_within_mdsthreshold() is called inside pg_init. We need
>> >> to set read_io/write_io before that.
>> >
>> > Why?
>> If we don't set read_io/write_io before calling pg_init, we fail
>> pnfs_within_mdsthreshold() and IO goes to MDS.
>> A simple test case:
>> dd if=foo of=/mnt/pnfs/bar bs=10M count=1 oflag=direct
>>
>
> My point was that this explanation needs to go into the changelog of the
> commit.
OK. Will resend the patch with proper changelog.

--
Thanks,
Tao