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

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

[Peng Tao]

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

Cc: Andy Adamson <andros@netapp.com>
Signed-off-by: Peng Tao <tao.peng@emc.com>
---
 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

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

[Peng Tao]

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 <bjschuma@netapp.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 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

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

[Bryan Schumaker]

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 <bjschuma@netapp.com>
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
>  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;
>  }
> 

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

[Peng Tao]

On Wed, Aug 8, 2012 at 8:49 PM, Bryan Schumaker <bjschuma@netapp.com> 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 <bjschuma@netapp.com>
>> Signed-off-by: Peng Tao <bergwolf@gmail.com>
>> ---
>>  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

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

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTA4LTA4IGF0IDA5OjQ5ICswODAwLCBQZW5nIFRhbyB3cm90ZToNCj4gcG5m
c193aXRoaW5fbWRzdGhyZXNob2xkKCkgaXMgY2FsbGVkIGluc2lkZSBwZ19pbml0LiBXZSBuZWVk
DQo+IHRvIHNldCByZWFkX2lvL3dyaXRlX2lvIGJlZm9yZSB0aGF0Lg0KDQpXaHk/DQoNCi0tIA0K
VHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpU
cm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

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

[Peng Tao]

On Thu, Aug 9, 2012 at 3:03 AM, Myklebust, Trond
<Trond.Myklebust@netapp.com> 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

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

[Myklebust, Trond]

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

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

[Peng Tao]

On Thu, Aug 9, 2012 at 10:25 PM, Myklebust, Trond
<Trond.Myklebust@netapp.com> wrote:
> On Thu, 2012-08-09 at 10:36 +0800, Peng Tao wrote:
>> On Thu, Aug 9, 2012 at 3:03 AM, Myklebust, Trond
>> <Trond.Myklebust@netapp.com> 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