Skip to contentSkip to conversation

NFSv41: fix DIO write_io calculation

PATCH Superseded Peng Tao 3 participants 8 messages Aug 8, 2012 – Aug 9, 2012

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Peng Tao

author
#0 lore raw

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]>
---
 fs/nfs/direct.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fs/nfs/direct.c +2 -2
index 1ba385b..34a6282 100644
@@ -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
Signature · 1 line
1.7.1.262.g5ef3d
author
#1 lore raw

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]>
---
 fs/nfs/nfs4proc.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
fs/nfs/nfs4proc.c +4 -1
index 78b2163..299c311 100644
@@ -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;
 }
Signature · 1 line
1.7.1.262.g5ef3d
#2 lore raw

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

Quoted 16 lines · 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 <[email protected]>
---
 fs/nfs/nfs4proc.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
fs/nfs/nfs4proc.c +0 -0
index 78b2163..299c311 100644

fs/nfs/nfs4proc.c static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s · line 3367

Quoted 7 lines
fs/nfs/nfs4proc.c +3 -1
@@ -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

Quoted 6 lines
fs/nfs/nfs4proc.c +1 -0
 		set_pnfs_layoutdriver(server, fhandle, fsinfo->layouttype);
+	}
 
 	return error;
 }
author
#3 lore raw

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

Quoted 18 lines · Bryan Schumaker
Quoted 17 lines · 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 <[email protected]>
---
 fs/nfs/nfs4proc.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
fs/nfs/nfs4proc.c +0 -0
index 78b2163..299c311 100644
@@ -3362,8 +3362,11 @@ static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s
 
Quoted 8 lines

fs/nfs/nfs4proc.c static int nfs4_proc_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, s · line 3367

Quoted 6 lines
fs/nfs/nfs4proc.c +3 -1
      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.

Signature · 2 lines
Thanks,
Tao
#4 lore raw
In reply to Peng Tao

T24gV2VkLCAyMDEyLTA4LTA4IGF0IDA5OjQ5ICswODAwLCBQZW5nIFRhbyB3cm90ZToNCj4gcG5m c193aXRoaW5fbWRzdGhyZXNob2xkKCkgaXMgY2FsbGVkIGluc2lkZSBwZ19pbml0LiBXZSBuZWVk DQo+IHRvIHNldCByZWFkX2lvL3dyaXRlX2lvIGJlZm9yZSB0aGF0Lg0KDQpXaHk/DQoNCi0tIA0K VHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpU cm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

author
#5 lore raw
Quoted 5 lines · Trond
Quoted 2 lines · Peng Tao

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

Signature · 2 lines
Thanks,
Tao
#6 lore raw

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

author
#7 lore raw
Quoted 8 lines · Trond
Quoted 7 lines · Peng Tao
Quoted 5 lines · Trond
Quoted 2 lines · Peng Tao

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

Why?

Quoted 8 lines
Quoted 5 lines

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.

Signature · 2 lines
Thanks,
Tao