linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] NFS: Read cleanups
@ 2012-05-01 17:16 Trond Myklebust
  2012-05-01 17:16 ` [PATCH v2 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 17:16 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

Remove unused variables, and reformat some code.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
 fs/nfs/read.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 35e2dce..20a0293 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -341,8 +341,6 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 	struct nfs_read_data *data;
 	size_t rsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
-	int requests = 0;
-	int ret = 0;
 
 	nfs_list_remove_request(req);
 	nfs_list_add_request(req, &hdr->pages);
@@ -358,12 +356,11 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 		data->pages.pagevec[0] = page;
 		nfs_read_rpcsetup(data, len, offset);
 		list_add(&data->list, &hdr->rpc_list);
-		requests++;
 		nbytes -= len;
 		offset += len;
-	} while(nbytes != 0);
+	} while (nbytes != 0);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
-	return ret;
+	return 0;
 out_bad:
 	while (!list_empty(&hdr->rpc_list)) {
 		data = list_first_entry(&hdr->rpc_list, struct nfs_read_data, list);
@@ -387,8 +384,7 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 							  desc->pg_count));
 	if (!data) {
 		desc->pg_completion_ops->error_cleanup(head);
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	pages = data->pages.pagevec;
@@ -402,8 +398,7 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 	nfs_read_rpcsetup(data, desc->pg_count, 0);
 	list_add(&data->list, &hdr->rpc_list);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
-out:
-	return ret;
+	return 0;
 }
 
 int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] NFS: Clean up nfs read and write error paths
  2012-05-01 17:16 [PATCH v2 1/3] NFS: Read cleanups Trond Myklebust
@ 2012-05-01 17:16 ` Trond Myklebust
  2012-05-01 17:16   ` [PATCH v2 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 17:16 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

Move the error handling for nfs_generic_pagein() into a single function.
Ditto for nfs_generic_flush().

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
 fs/nfs/pnfs.c  |    2 --
 fs/nfs/read.c  |   38 +++++++++++++++++++++-----------------
 fs/nfs/write.c |   46 +++++++++++++++++++++++-----------------------
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 39cbac5..6fdeca2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1321,7 +1321,6 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
 	if (ret != 0) {
 		put_lseg(desc->pg_lseg);
 		desc->pg_lseg = NULL;
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	} else
 		pnfs_do_multiple_writes(desc, &hdr->rpc_list, desc->pg_ioflags);
 	if (atomic_dec_and_test(&hdr->refcnt))
@@ -1476,7 +1475,6 @@ pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
 	if (ret != 0) {
 		put_lseg(desc->pg_lseg);
 		desc->pg_lseg = NULL;
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	} else
 		pnfs_do_multiple_reads(desc, &hdr->rpc_list);
 	if (atomic_dec_and_test(&hdr->refcnt))
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 20a0293..1961a19 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -320,6 +320,19 @@ static const struct nfs_pgio_completion_ops nfs_async_read_completion_ops = {
 	.completion = nfs_read_completion,
 };
 
+static void nfs_pagein_error(struct nfs_pageio_descriptor *desc,
+		struct nfs_pgio_header *hdr)
+{
+	set_bit(NFS_IOHDR_REDO, &hdr->flags);
+	while (!list_empty(&hdr->rpc_list)) {
+		struct nfs_read_data *data = list_first_entry(&hdr->rpc_list,
+				struct nfs_read_data, list);
+		list_del(&data->list);
+		nfs_readdata_release(data);
+	}
+	desc->pg_completion_ops->error_cleanup(&desc->pg_list);
+}
+
 /*
  * Generate multiple requests to fill a single page.
  *
@@ -342,33 +355,27 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 	size_t rsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
 
-	nfs_list_remove_request(req);
-	nfs_list_add_request(req, &hdr->pages);
-
 	offset = 0;
 	nbytes = desc->pg_count;
 	do {
 		size_t len = min(nbytes,rsize);
 
 		data = nfs_readdata_alloc(hdr, 1);
-		if (!data)
-			goto out_bad;
+		if (!data) {
+			nfs_pagein_error(desc, hdr);
+			return -ENOMEM;
+		}
 		data->pages.pagevec[0] = page;
 		nfs_read_rpcsetup(data, len, offset);
 		list_add(&data->list, &hdr->rpc_list);
 		nbytes -= len;
 		offset += len;
 	} while (nbytes != 0);
+
+	nfs_list_remove_request(req);
+	nfs_list_add_request(req, &hdr->pages);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
 	return 0;
-out_bad:
-	while (!list_empty(&hdr->rpc_list)) {
-		data = list_first_entry(&hdr->rpc_list, struct nfs_read_data, list);
-		list_del(&data->list);
-		nfs_readdata_release(data);
-	}
-	desc->pg_completion_ops->error_cleanup(&hdr->pages);
-	return -ENOMEM;
 }
 
 static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
@@ -378,12 +385,11 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 	struct page		**pages;
 	struct nfs_read_data    *data;
 	struct list_head *head = &desc->pg_list;
-	int ret = 0;
 
 	data = nfs_readdata_alloc(hdr, nfs_page_array_len(desc->pg_base,
 							  desc->pg_count));
 	if (!data) {
-		desc->pg_completion_ops->error_cleanup(head);
+		nfs_pagein_error(desc, hdr);
 		return -ENOMEM;
 	}
 
@@ -427,8 +433,6 @@ static int nfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
 	if (ret == 0)
 		ret = nfs_do_multiple_reads(&hdr->rpc_list,
 					    desc->pg_rpc_callops);
-	else
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	if (atomic_dec_and_test(&hdr->refcnt))
 		hdr->completion_ops->completion(hdr);
 	return ret;
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 41db7b7..6f263da 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1062,6 +1062,19 @@ static const struct nfs_pgio_completion_ops nfs_async_write_completion_ops = {
 	.completion = nfs_write_completion,
 };
 
+static void nfs_flush_error(struct nfs_pageio_descriptor *desc,
+		struct nfs_pgio_header *hdr)
+{
+	set_bit(NFS_IOHDR_REDO, &hdr->flags);
+	while (!list_empty(&hdr->rpc_list)) {
+		struct nfs_write_data *data = list_first_entry(&hdr->rpc_list,
+				struct nfs_write_data, list);
+		list_del(&data->list);
+		nfs_writedata_release(data);
+	}
+	desc->pg_completion_ops->error_cleanup(&desc->pg_list);
+}
+
 /*
  * Generate multiple small requests to write out a single
  * contiguous dirty area on one page.
@@ -1075,12 +1088,9 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 	size_t wsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
 	int requests = 0;
-	int ret = 0;
 	struct nfs_commit_info cinfo;
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
-	nfs_list_remove_request(req);
-	nfs_list_add_request(req, &hdr->pages);
 
 	if ((desc->pg_ioflags & FLUSH_COND_STABLE) &&
 	    (desc->pg_moreio || nfs_reqs_to_commit(&cinfo) ||
@@ -1094,8 +1104,10 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 		size_t len = min(nbytes, wsize);
 
 		data = nfs_writedata_alloc(hdr, 1);
-		if (!data)
-			goto out_bad;
+		if (!data) {
+			nfs_flush_error(desc, hdr);
+			return -ENOMEM;
+		}
 		data->pages.pagevec[0] = page;
 		nfs_write_rpcsetup(data, len, offset, desc->pg_ioflags, &cinfo);
 		list_add(&data->list, &hdr->rpc_list);
@@ -1103,17 +1115,10 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 		nbytes -= len;
 		offset += len;
 	} while (nbytes != 0);
+	nfs_list_remove_request(req);
+	nfs_list_add_request(req, &hdr->pages);
 	desc->pg_rpc_callops = &nfs_write_common_ops;
-	return ret;
-
-out_bad:
-	while (!list_empty(&hdr->rpc_list)) {
-		data = list_first_entry(&hdr->rpc_list, struct nfs_write_data, list);
-		list_del(&data->list);
-		nfs_writedata_release(data);
-	}
-	desc->pg_completion_ops->error_cleanup(&hdr->pages);
-	return -ENOMEM;
+	return 0;
 }
 
 /*
@@ -1131,15 +1136,13 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc,
 	struct page		**pages;
 	struct nfs_write_data	*data;
 	struct list_head *head = &desc->pg_list;
-	int ret = 0;
 	struct nfs_commit_info cinfo;
 
 	data = nfs_writedata_alloc(hdr, nfs_page_array_len(desc->pg_base,
 							   desc->pg_count));
 	if (!data) {
-		desc->pg_completion_ops->error_cleanup(head);
-		ret = -ENOMEM;
-		goto out;
+		nfs_flush_error(desc, hdr);
+		return -ENOMEM;
 	}
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
@@ -1159,8 +1162,7 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc,
 	nfs_write_rpcsetup(data, desc->pg_count, 0, desc->pg_ioflags, &cinfo);
 	list_add(&data->list, &hdr->rpc_list);
 	desc->pg_rpc_callops = &nfs_write_common_ops;
-out:
-	return ret;
+	return 0;
 }
 
 int nfs_generic_flush(struct nfs_pageio_descriptor *desc,
@@ -1190,8 +1192,6 @@ static int nfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
 		ret = nfs_do_multiple_writes(&hdr->rpc_list,
 					     desc->pg_rpc_callops,
 					     desc->pg_ioflags);
-	else
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	if (atomic_dec_and_test(&hdr->refcnt))
 		hdr->completion_ops->completion(hdr);
 	return ret;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] NFS: Simplify the nfs_read_completion functions
  2012-05-01 17:16 ` [PATCH v2 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
@ 2012-05-01 17:16   ` Trond Myklebust
  2012-05-01 18:37     ` Myklebust, Trond
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 17:16 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

...and correct a bug when NFS_IOHDR_ERROR is set: we should not be
setting PageUptodate/set_page_dirty on a page unless the number of
bytes read <= hdr->good_bytes

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
v2: Actually add the bugfix to direct.c...

 fs/nfs/direct.c |   46 +++++++++++++++++++---------------------------
 fs/nfs/read.c   |   42 +++++++++++++++++-------------------------
 2 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 78d1ead..34027d5 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -253,36 +253,28 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
 		dreq->count += hdr->good_bytes;
 	spin_unlock(&dreq->lock);
 
-	if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-			struct page *page = req->wb_page;
-
-			if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
-				if (bytes > hdr->good_bytes)
-					zero_user(page, 0, PAGE_SIZE);
-				else if (hdr->good_bytes - bytes < PAGE_SIZE)
-					zero_user_segment(page,
-						hdr->good_bytes & ~PAGE_MASK,
-						PAGE_SIZE);
-			}
-			bytes += req->wb_bytes;
-			nfs_list_remove_request(req);
-			if (!PageCompound(page))
-				set_page_dirty(page);
-			nfs_direct_readpage_release(req);
+	while (!list_empty(&hdr->pages)) {
+		struct nfs_page *req = nfs_list_entry(hdr->pages.next);
+		struct page *page = req->wb_page;
+
+		if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
+			if (bytes > hdr->good_bytes)
+				zero_user(page, 0, PAGE_SIZE);
+			else if (hdr->good_bytes - bytes < PAGE_SIZE)
+				zero_user_segment(page,
+					hdr->good_bytes & ~PAGE_MASK,
+					PAGE_SIZE);
 		}
-	} else {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-
-			if (bytes < hdr->good_bytes)
-				if (!PageCompound(req->wb_page))
+		bytes += req->wb_bytes;
+		if (!PageCompound(page)) {
+			if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
+				if (bytes <= hdr->good_bytes)
 					set_page_dirty(req->wb_page);
-			bytes += req->wb_bytes;
-			nfs_list_remove_request(req);
-			nfs_direct_readpage_release(req);
+			} else
+				set_page_dirty(page);
 		}
+		nfs_list_remove_request(req);
+		nfs_direct_readpage_release(req);
 	}
 out_put:
 	if (put_dreq(dreq))
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 1961a19..b81281b 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -179,34 +179,26 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr)
 
 	if (test_bit(NFS_IOHDR_REDO, &hdr->flags))
 		goto out;
-	if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-			struct page *page = req->wb_page;
-
-			if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
-				if (bytes > hdr->good_bytes)
-					zero_user(page, 0, PAGE_SIZE);
-				else if (hdr->good_bytes - bytes < PAGE_SIZE)
-					zero_user_segment(page,
-						hdr->good_bytes & ~PAGE_MASK,
-						PAGE_SIZE);
-			}
-			SetPageUptodate(page);
-			nfs_list_remove_request(req);
-			nfs_readpage_release(req);
-			bytes += PAGE_SIZE;
+	while (!list_empty(&hdr->pages)) {
+		struct nfs_page *req = nfs_list_entry(hdr->pages.next);
+		struct page *page = req->wb_page;
+
+		if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
+			if (bytes > hdr->good_bytes)
+				zero_user(page, 0, PAGE_SIZE);
+			else if (hdr->good_bytes - bytes < PAGE_SIZE)
+				zero_user_segment(page,
+					hdr->good_bytes & ~PAGE_MASK,
+					PAGE_SIZE);
 		}
-	} else {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-
-			bytes += req->wb_bytes;
+		bytes += req->wb_bytes;
+		if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
 			if (bytes <= hdr->good_bytes)
 				SetPageUptodate(req->wb_page);
-			nfs_list_remove_request(req);
-			nfs_readpage_release(req);
-		}
+		} else
+			SetPageUptodate(page);
+		nfs_list_remove_request(req);
+		nfs_readpage_release(req);
 	}
 out:
 	hdr->release(hdr);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/3] NFS: Simplify the nfs_read_completion functions
  2012-05-01 17:16   ` [PATCH v2 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
@ 2012-05-01 18:37     ` Myklebust, Trond
  2012-05-01 18:40       ` [PATCH v3 1/3] NFS: Read cleanups Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Myklebust, Trond @ 2012-05-01 18:37 UTC (permalink / raw)
  To: linux-nfs@vger.kernel.org; +Cc: Isaman, Fred

T24gVHVlLCAyMDEyLTA1LTAxIGF0IDEzOjE2IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6
DQo+IC4uLmFuZCBjb3JyZWN0IGEgYnVnIHdoZW4gTkZTX0lPSERSX0VSUk9SIGlzIHNldDogd2Ug
c2hvdWxkIG5vdCBiZQ0KPiBzZXR0aW5nIFBhZ2VVcHRvZGF0ZS9zZXRfcGFnZV9kaXJ0eSBvbiBh
IHBhZ2UgdW5sZXNzIHRoZSBudW1iZXIgb2YNCj4gYnl0ZXMgcmVhZCA8PSBoZHItPmdvb2RfYnl0
ZXMNCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8VHJvbmQuTXlrbGVidXN0
QG5ldGFwcC5jb20+DQo+IENjOiBGcmVkIElzYW1hbiA8aWlzYW1hbkBuZXRhcHAuY29tPg0KPiAt
LS0NCj4gdjI6IEFjdHVhbGx5IGFkZCB0aGUgYnVnZml4IHRvIGRpcmVjdC5jLi4uDQo+IA0KPiAg
ZnMvbmZzL2RpcmVjdC5jIHwgICA0NiArKysrKysrKysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tDQo+ICBmcy9uZnMvcmVhZC5jICAgfCAgIDQyICsrKysrKysrKysrKysrKysr
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiAgMiBmaWxlcyBjaGFuZ2VkLCAzNiBpbnNlcnRp
b25zKCspLCA1MiBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvZGlyZWN0
LmMgYi9mcy9uZnMvZGlyZWN0LmMNCj4gaW5kZXggNzhkMWVhZC4uMzQwMjdkNSAxMDA2NDQNCj4g
LS0tIGEvZnMvbmZzL2RpcmVjdC5jDQo+ICsrKyBiL2ZzL25mcy9kaXJlY3QuYw0KPiBAQCAtMjUz
LDM2ICsyNTMsMjggQEAgc3RhdGljIHZvaWQgbmZzX2RpcmVjdF9yZWFkX2NvbXBsZXRpb24oc3Ry
dWN0IG5mc19wZ2lvX2hlYWRlciAqaGRyKQ0KPiAgCQlkcmVxLT5jb3VudCArPSBoZHItPmdvb2Rf
Ynl0ZXM7DQo+ICAJc3Bpbl91bmxvY2soJmRyZXEtPmxvY2spOw0KPiAgDQo+IC0JaWYgKCF0ZXN0
X2JpdChORlNfSU9IRFJfRVJST1IsICZoZHItPmZsYWdzKSkgew0KPiAtCQl3aGlsZSAoIWxpc3Rf
ZW1wdHkoJmhkci0+cGFnZXMpKSB7DQo+IC0JCQlzdHJ1Y3QgbmZzX3BhZ2UgKnJlcSA9IG5mc19s
aXN0X2VudHJ5KGhkci0+cGFnZXMubmV4dCk7DQo+IC0JCQlzdHJ1Y3QgcGFnZSAqcGFnZSA9IHJl
cS0+d2JfcGFnZTsNCj4gLQ0KPiAtCQkJaWYgKHRlc3RfYml0KE5GU19JT0hEUl9FT0YsICZoZHIt
PmZsYWdzKSkgew0KPiAtCQkJCWlmIChieXRlcyA+IGhkci0+Z29vZF9ieXRlcykNCj4gLQkJCQkJ
emVyb191c2VyKHBhZ2UsIDAsIFBBR0VfU0laRSk7DQo+IC0JCQkJZWxzZSBpZiAoaGRyLT5nb29k
X2J5dGVzIC0gYnl0ZXMgPCBQQUdFX1NJWkUpDQo+IC0JCQkJCXplcm9fdXNlcl9zZWdtZW50KHBh
Z2UsDQo+IC0JCQkJCQloZHItPmdvb2RfYnl0ZXMgJiB+UEFHRV9NQVNLLA0KPiAtCQkJCQkJUEFH
RV9TSVpFKTsNCj4gLQkJCX0NCj4gLQkJCWJ5dGVzICs9IHJlcS0+d2JfYnl0ZXM7DQo+IC0JCQlu
ZnNfbGlzdF9yZW1vdmVfcmVxdWVzdChyZXEpOw0KPiAtCQkJaWYgKCFQYWdlQ29tcG91bmQocGFn
ZSkpDQo+IC0JCQkJc2V0X3BhZ2VfZGlydHkocGFnZSk7DQo+IC0JCQluZnNfZGlyZWN0X3JlYWRw
YWdlX3JlbGVhc2UocmVxKTsNCj4gKwl3aGlsZSAoIWxpc3RfZW1wdHkoJmhkci0+cGFnZXMpKSB7
DQo+ICsJCXN0cnVjdCBuZnNfcGFnZSAqcmVxID0gbmZzX2xpc3RfZW50cnkoaGRyLT5wYWdlcy5u
ZXh0KTsNCj4gKwkJc3RydWN0IHBhZ2UgKnBhZ2UgPSByZXEtPndiX3BhZ2U7DQo+ICsNCj4gKwkJ
aWYgKHRlc3RfYml0KE5GU19JT0hEUl9FT0YsICZoZHItPmZsYWdzKSkgew0KPiArCQkJaWYgKGJ5
dGVzID4gaGRyLT5nb29kX2J5dGVzKQ0KPiArCQkJCXplcm9fdXNlcihwYWdlLCAwLCBQQUdFX1NJ
WkUpOw0KPiArCQkJZWxzZSBpZiAoaGRyLT5nb29kX2J5dGVzIC0gYnl0ZXMgPCBQQUdFX1NJWkUp
DQo+ICsJCQkJemVyb191c2VyX3NlZ21lbnQocGFnZSwNCj4gKwkJCQkJaGRyLT5nb29kX2J5dGVz
ICYgflBBR0VfTUFTSywNCj4gKwkJCQkJUEFHRV9TSVpFKTsNCj4gIAkJfQ0KPiAtCX0gZWxzZSB7
DQo+IC0JCXdoaWxlICghbGlzdF9lbXB0eSgmaGRyLT5wYWdlcykpIHsNCj4gLQkJCXN0cnVjdCBu
ZnNfcGFnZSAqcmVxID0gbmZzX2xpc3RfZW50cnkoaGRyLT5wYWdlcy5uZXh0KTsNCj4gLQ0KPiAt
CQkJaWYgKGJ5dGVzIDwgaGRyLT5nb29kX2J5dGVzKQ0KPiAtCQkJCWlmICghUGFnZUNvbXBvdW5k
KHJlcS0+d2JfcGFnZSkpDQo+ICsJCWJ5dGVzICs9IHJlcS0+d2JfYnl0ZXM7DQo+ICsJCWlmICgh
UGFnZUNvbXBvdW5kKHBhZ2UpKSB7DQo+ICsJCQlpZiAodGVzdF9iaXQoTkZTX0lPSERSX0VSUk9S
LCAmaGRyLT5mbGFncykpIHsNCj4gKwkJCQlpZiAoYnl0ZXMgPD0gaGRyLT5nb29kX2J5dGVzKQ0K
PiAgCQkJCQlzZXRfcGFnZV9kaXJ0eShyZXEtPndiX3BhZ2UpOw0KDQpPSy4uLiBMb29raW5nIGF0
IHRoaXMgbW9yZSBjbG9zZWx5LCBJIHRoaW5rIHRoYXQgSSdtIHdyb25nIGluIGNhbGxpbmcNCnRo
ZSBvbGQgYmVoYXZpb3VyIGEgYnVnLiBTaW5jZSB3ZSBhcmUgc2F5aW5nIHRoYXQgdGhlIGJ1ZmZl
ciBpcyBnb29kIHVwDQp0byBhbmQgaW5jbHVkaW5nIGhkci0+Z29vZF9ieXRlcyBvZiByZWFkIGJ5
dGVzLCB3ZSBzaG91bGQgbWFyayB0aGUgcGFnZQ0KYXMgYmVpbmcgZGlydHkgc28gbG9uZyBhcyBp
dCBjb250YWlucyBvbmUgb3IgbW9yZSBnb29kIGJ5dGVzLg0KDQpJJ2xsIHJlc2VuZCB0aGUgY2xl
YW51cCB3aXRob3V0IHRoZSBlcnJvbmVvdXMgZml4Li4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0
DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RA
bmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/3] NFS: Read cleanups
  2012-05-01 18:37     ` Myklebust, Trond
@ 2012-05-01 18:40       ` Trond Myklebust
  2012-05-01 18:40         ` [PATCH v3 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 18:40 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

Remove unused variables, and reformat some code.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
 fs/nfs/read.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 35e2dce..20a0293 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -341,8 +341,6 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 	struct nfs_read_data *data;
 	size_t rsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
-	int requests = 0;
-	int ret = 0;
 
 	nfs_list_remove_request(req);
 	nfs_list_add_request(req, &hdr->pages);
@@ -358,12 +356,11 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 		data->pages.pagevec[0] = page;
 		nfs_read_rpcsetup(data, len, offset);
 		list_add(&data->list, &hdr->rpc_list);
-		requests++;
 		nbytes -= len;
 		offset += len;
-	} while(nbytes != 0);
+	} while (nbytes != 0);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
-	return ret;
+	return 0;
 out_bad:
 	while (!list_empty(&hdr->rpc_list)) {
 		data = list_first_entry(&hdr->rpc_list, struct nfs_read_data, list);
@@ -387,8 +384,7 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 							  desc->pg_count));
 	if (!data) {
 		desc->pg_completion_ops->error_cleanup(head);
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	pages = data->pages.pagevec;
@@ -402,8 +398,7 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 	nfs_read_rpcsetup(data, desc->pg_count, 0);
 	list_add(&data->list, &hdr->rpc_list);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
-out:
-	return ret;
+	return 0;
 }
 
 int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/3] NFS: Clean up nfs read and write error paths
  2012-05-01 18:40       ` [PATCH v3 1/3] NFS: Read cleanups Trond Myklebust
@ 2012-05-01 18:40         ` Trond Myklebust
  2012-05-01 18:40           ` [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 18:40 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

Move the error handling for nfs_generic_pagein() into a single function.
Ditto for nfs_generic_flush().

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
 fs/nfs/pnfs.c  |    2 --
 fs/nfs/read.c  |   38 +++++++++++++++++++++-----------------
 fs/nfs/write.c |   46 +++++++++++++++++++++++-----------------------
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 39cbac5..6fdeca2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1321,7 +1321,6 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
 	if (ret != 0) {
 		put_lseg(desc->pg_lseg);
 		desc->pg_lseg = NULL;
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	} else
 		pnfs_do_multiple_writes(desc, &hdr->rpc_list, desc->pg_ioflags);
 	if (atomic_dec_and_test(&hdr->refcnt))
@@ -1476,7 +1475,6 @@ pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
 	if (ret != 0) {
 		put_lseg(desc->pg_lseg);
 		desc->pg_lseg = NULL;
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	} else
 		pnfs_do_multiple_reads(desc, &hdr->rpc_list);
 	if (atomic_dec_and_test(&hdr->refcnt))
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 20a0293..1961a19 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -320,6 +320,19 @@ static const struct nfs_pgio_completion_ops nfs_async_read_completion_ops = {
 	.completion = nfs_read_completion,
 };
 
+static void nfs_pagein_error(struct nfs_pageio_descriptor *desc,
+		struct nfs_pgio_header *hdr)
+{
+	set_bit(NFS_IOHDR_REDO, &hdr->flags);
+	while (!list_empty(&hdr->rpc_list)) {
+		struct nfs_read_data *data = list_first_entry(&hdr->rpc_list,
+				struct nfs_read_data, list);
+		list_del(&data->list);
+		nfs_readdata_release(data);
+	}
+	desc->pg_completion_ops->error_cleanup(&desc->pg_list);
+}
+
 /*
  * Generate multiple requests to fill a single page.
  *
@@ -342,33 +355,27 @@ static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc,
 	size_t rsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
 
-	nfs_list_remove_request(req);
-	nfs_list_add_request(req, &hdr->pages);
-
 	offset = 0;
 	nbytes = desc->pg_count;
 	do {
 		size_t len = min(nbytes,rsize);
 
 		data = nfs_readdata_alloc(hdr, 1);
-		if (!data)
-			goto out_bad;
+		if (!data) {
+			nfs_pagein_error(desc, hdr);
+			return -ENOMEM;
+		}
 		data->pages.pagevec[0] = page;
 		nfs_read_rpcsetup(data, len, offset);
 		list_add(&data->list, &hdr->rpc_list);
 		nbytes -= len;
 		offset += len;
 	} while (nbytes != 0);
+
+	nfs_list_remove_request(req);
+	nfs_list_add_request(req, &hdr->pages);
 	desc->pg_rpc_callops = &nfs_read_common_ops;
 	return 0;
-out_bad:
-	while (!list_empty(&hdr->rpc_list)) {
-		data = list_first_entry(&hdr->rpc_list, struct nfs_read_data, list);
-		list_del(&data->list);
-		nfs_readdata_release(data);
-	}
-	desc->pg_completion_ops->error_cleanup(&hdr->pages);
-	return -ENOMEM;
 }
 
 static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
@@ -378,12 +385,11 @@ static int nfs_pagein_one(struct nfs_pageio_descriptor *desc,
 	struct page		**pages;
 	struct nfs_read_data    *data;
 	struct list_head *head = &desc->pg_list;
-	int ret = 0;
 
 	data = nfs_readdata_alloc(hdr, nfs_page_array_len(desc->pg_base,
 							  desc->pg_count));
 	if (!data) {
-		desc->pg_completion_ops->error_cleanup(head);
+		nfs_pagein_error(desc, hdr);
 		return -ENOMEM;
 	}
 
@@ -427,8 +433,6 @@ static int nfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
 	if (ret == 0)
 		ret = nfs_do_multiple_reads(&hdr->rpc_list,
 					    desc->pg_rpc_callops);
-	else
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	if (atomic_dec_and_test(&hdr->refcnt))
 		hdr->completion_ops->completion(hdr);
 	return ret;
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 2f80aa5..d1e4f81 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1058,6 +1058,19 @@ static const struct nfs_pgio_completion_ops nfs_async_write_completion_ops = {
 	.completion = nfs_write_completion,
 };
 
+static void nfs_flush_error(struct nfs_pageio_descriptor *desc,
+		struct nfs_pgio_header *hdr)
+{
+	set_bit(NFS_IOHDR_REDO, &hdr->flags);
+	while (!list_empty(&hdr->rpc_list)) {
+		struct nfs_write_data *data = list_first_entry(&hdr->rpc_list,
+				struct nfs_write_data, list);
+		list_del(&data->list);
+		nfs_writedata_release(data);
+	}
+	desc->pg_completion_ops->error_cleanup(&desc->pg_list);
+}
+
 /*
  * Generate multiple small requests to write out a single
  * contiguous dirty area on one page.
@@ -1071,12 +1084,9 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 	size_t wsize = desc->pg_bsize, nbytes;
 	unsigned int offset;
 	int requests = 0;
-	int ret = 0;
 	struct nfs_commit_info cinfo;
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
-	nfs_list_remove_request(req);
-	nfs_list_add_request(req, &hdr->pages);
 
 	if ((desc->pg_ioflags & FLUSH_COND_STABLE) &&
 	    (desc->pg_moreio || nfs_reqs_to_commit(&cinfo) ||
@@ -1090,8 +1100,10 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 		size_t len = min(nbytes, wsize);
 
 		data = nfs_writedata_alloc(hdr, 1);
-		if (!data)
-			goto out_bad;
+		if (!data) {
+			nfs_flush_error(desc, hdr);
+			return -ENOMEM;
+		}
 		data->pages.pagevec[0] = page;
 		nfs_write_rpcsetup(data, len, offset, desc->pg_ioflags, &cinfo);
 		list_add(&data->list, &hdr->rpc_list);
@@ -1099,17 +1111,10 @@ static int nfs_flush_multi(struct nfs_pageio_descriptor *desc,
 		nbytes -= len;
 		offset += len;
 	} while (nbytes != 0);
+	nfs_list_remove_request(req);
+	nfs_list_add_request(req, &hdr->pages);
 	desc->pg_rpc_callops = &nfs_write_common_ops;
-	return ret;
-
-out_bad:
-	while (!list_empty(&hdr->rpc_list)) {
-		data = list_first_entry(&hdr->rpc_list, struct nfs_write_data, list);
-		list_del(&data->list);
-		nfs_writedata_release(data);
-	}
-	desc->pg_completion_ops->error_cleanup(&hdr->pages);
-	return -ENOMEM;
+	return 0;
 }
 
 /*
@@ -1127,15 +1132,13 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc,
 	struct page		**pages;
 	struct nfs_write_data	*data;
 	struct list_head *head = &desc->pg_list;
-	int ret = 0;
 	struct nfs_commit_info cinfo;
 
 	data = nfs_writedata_alloc(hdr, nfs_page_array_len(desc->pg_base,
 							   desc->pg_count));
 	if (!data) {
-		desc->pg_completion_ops->error_cleanup(head);
-		ret = -ENOMEM;
-		goto out;
+		nfs_flush_error(desc, hdr);
+		return -ENOMEM;
 	}
 
 	nfs_init_cinfo(&cinfo, desc->pg_inode, desc->pg_dreq);
@@ -1155,8 +1158,7 @@ static int nfs_flush_one(struct nfs_pageio_descriptor *desc,
 	nfs_write_rpcsetup(data, desc->pg_count, 0, desc->pg_ioflags, &cinfo);
 	list_add(&data->list, &hdr->rpc_list);
 	desc->pg_rpc_callops = &nfs_write_common_ops;
-out:
-	return ret;
+	return 0;
 }
 
 int nfs_generic_flush(struct nfs_pageio_descriptor *desc,
@@ -1186,8 +1188,6 @@ static int nfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
 		ret = nfs_do_multiple_writes(&hdr->rpc_list,
 					     desc->pg_rpc_callops,
 					     desc->pg_ioflags);
-	else
-		set_bit(NFS_IOHDR_REDO, &hdr->flags);
 	if (atomic_dec_and_test(&hdr->refcnt))
 		hdr->completion_ops->completion(hdr);
 	return ret;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions
  2012-05-01 18:40         ` [PATCH v3 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
@ 2012-05-01 18:40           ` Trond Myklebust
  2012-05-01 19:12             ` Fred Isaman
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2012-05-01 18:40 UTC (permalink / raw)
  To: linux-nfs; +Cc: Fred Isaman

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
---
 fs/nfs/direct.c |   46 +++++++++++++++++++---------------------------
 fs/nfs/read.c   |   42 +++++++++++++++++-------------------------
 2 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index f17e469..a1d366b 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -243,36 +243,28 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
 		dreq->count += hdr->good_bytes;
 	spin_unlock(&dreq->lock);
 
-	if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-			struct page *page = req->wb_page;
-
-			if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
-				if (bytes > hdr->good_bytes)
-					zero_user(page, 0, PAGE_SIZE);
-				else if (hdr->good_bytes - bytes < PAGE_SIZE)
-					zero_user_segment(page,
-						hdr->good_bytes & ~PAGE_MASK,
-						PAGE_SIZE);
-			}
-			bytes += req->wb_bytes;
-			nfs_list_remove_request(req);
-			if (!PageCompound(page))
-				set_page_dirty(page);
-			nfs_direct_readpage_release(req);
+	while (!list_empty(&hdr->pages)) {
+		struct nfs_page *req = nfs_list_entry(hdr->pages.next);
+		struct page *page = req->wb_page;
+
+		if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
+			if (bytes > hdr->good_bytes)
+				zero_user(page, 0, PAGE_SIZE);
+			else if (hdr->good_bytes - bytes < PAGE_SIZE)
+				zero_user_segment(page,
+					hdr->good_bytes & ~PAGE_MASK,
+					PAGE_SIZE);
 		}
-	} else {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-
-			if (bytes < hdr->good_bytes)
-				if (!PageCompound(req->wb_page))
+		if (!PageCompound(page)) {
+			if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
+				if (bytes < hdr->good_bytes)
 					set_page_dirty(req->wb_page);
-			bytes += req->wb_bytes;
-			nfs_list_remove_request(req);
-			nfs_direct_readpage_release(req);
+			} else
+				set_page_dirty(page);
 		}
+		bytes += req->wb_bytes;
+		nfs_list_remove_request(req);
+		nfs_direct_readpage_release(req);
 	}
 out_put:
 	if (put_dreq(dreq))
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 1961a19..b81281b 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -179,34 +179,26 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr)
 
 	if (test_bit(NFS_IOHDR_REDO, &hdr->flags))
 		goto out;
-	if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-			struct page *page = req->wb_page;
-
-			if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
-				if (bytes > hdr->good_bytes)
-					zero_user(page, 0, PAGE_SIZE);
-				else if (hdr->good_bytes - bytes < PAGE_SIZE)
-					zero_user_segment(page,
-						hdr->good_bytes & ~PAGE_MASK,
-						PAGE_SIZE);
-			}
-			SetPageUptodate(page);
-			nfs_list_remove_request(req);
-			nfs_readpage_release(req);
-			bytes += PAGE_SIZE;
+	while (!list_empty(&hdr->pages)) {
+		struct nfs_page *req = nfs_list_entry(hdr->pages.next);
+		struct page *page = req->wb_page;
+
+		if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
+			if (bytes > hdr->good_bytes)
+				zero_user(page, 0, PAGE_SIZE);
+			else if (hdr->good_bytes - bytes < PAGE_SIZE)
+				zero_user_segment(page,
+					hdr->good_bytes & ~PAGE_MASK,
+					PAGE_SIZE);
 		}
-	} else {
-		while (!list_empty(&hdr->pages)) {
-			struct nfs_page *req = nfs_list_entry(hdr->pages.next);
-
-			bytes += req->wb_bytes;
+		bytes += req->wb_bytes;
+		if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
 			if (bytes <= hdr->good_bytes)
 				SetPageUptodate(req->wb_page);
-			nfs_list_remove_request(req);
-			nfs_readpage_release(req);
-		}
+		} else
+			SetPageUptodate(page);
+		nfs_list_remove_request(req);
+		nfs_readpage_release(req);
 	}
 out:
 	hdr->release(hdr);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions
  2012-05-01 18:40           ` [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
@ 2012-05-01 19:12             ` Fred Isaman
  2012-05-01 19:38               ` Myklebust, Trond
  0 siblings, 1 reply; 9+ messages in thread
From: Fred Isaman @ 2012-05-01 19:12 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs

On Tue, May 1, 2012 at 2:40 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: Fred Isaman <iisaman@netapp.com>
> ---
>  fs/nfs/direct.c |   46 +++++++++++++++++++---------------------------
>  fs/nfs/read.c   |   42 +++++++++++++++++-------------------------
>  2 files changed, 36 insertions(+), 52 deletions(-)
>
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index f17e469..a1d366b 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -243,36 +243,28 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
>                dreq->count += hdr->good_bytes;
>        spin_unlock(&dreq->lock);
>
> -       if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> -               while (!list_empty(&hdr->pages)) {
> -                       struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> -                       struct page *page = req->wb_page;
> -
> -                       if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> -                               if (bytes > hdr->good_bytes)
> -                                       zero_user(page, 0, PAGE_SIZE);
> -                               else if (hdr->good_bytes - bytes < PAGE_SIZE)
> -                                       zero_user_segment(page,
> -                                               hdr->good_bytes & ~PAGE_MASK,
> -                                               PAGE_SIZE);
> -                       }
> -                       bytes += req->wb_bytes;
> -                       nfs_list_remove_request(req);
> -                       if (!PageCompound(page))
> -                               set_page_dirty(page);
> -                       nfs_direct_readpage_release(req);
> +       while (!list_empty(&hdr->pages)) {
> +               struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> +               struct page *page = req->wb_page;
> +
> +               if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> +                       if (bytes > hdr->good_bytes)
> +                               zero_user(page, 0, PAGE_SIZE);
> +                       else if (hdr->good_bytes - bytes < PAGE_SIZE)
> +                               zero_user_segment(page,
> +                                       hdr->good_bytes & ~PAGE_MASK,
> +                                       PAGE_SIZE);
>                }
> -       } else {
> -               while (!list_empty(&hdr->pages)) {
> -                       struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> -
> -                       if (bytes < hdr->good_bytes)
> -                               if (!PageCompound(req->wb_page))
> +               if (!PageCompound(page)) {
> +                       if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> +                               if (bytes < hdr->good_bytes)
>                                        set_page_dirty(req->wb_page);

You can s/req->wb_page/page/

Other than that, looks fine, but I'll note that my intent in writing
it as it was was to move the test_bit out of the loop.
But given the EOF test that was still there, I guess it doesn't make
much difference.

Fred

> -                       bytes += req->wb_bytes;
> -                       nfs_list_remove_request(req);
> -                       nfs_direct_readpage_release(req);
> +                       } else
> +                               set_page_dirty(page);
>                }
> +               bytes += req->wb_bytes;
> +               nfs_list_remove_request(req);
> +               nfs_direct_readpage_release(req);
>        }
>  out_put:
>        if (put_dreq(dreq))
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 1961a19..b81281b 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -179,34 +179,26 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr)
>
>        if (test_bit(NFS_IOHDR_REDO, &hdr->flags))
>                goto out;
> -       if (!test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
> -               while (!list_empty(&hdr->pages)) {
> -                       struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> -                       struct page *page = req->wb_page;
> -
> -                       if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> -                               if (bytes > hdr->good_bytes)
> -                                       zero_user(page, 0, PAGE_SIZE);
> -                               else if (hdr->good_bytes - bytes < PAGE_SIZE)
> -                                       zero_user_segment(page,
> -                                               hdr->good_bytes & ~PAGE_MASK,
> -                                               PAGE_SIZE);
> -                       }
> -                       SetPageUptodate(page);
> -                       nfs_list_remove_request(req);
> -                       nfs_readpage_release(req);
> -                       bytes += PAGE_SIZE;
> +       while (!list_empty(&hdr->pages)) {
> +               struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> +               struct page *page = req->wb_page;
> +
> +               if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
> +                       if (bytes > hdr->good_bytes)
> +                               zero_user(page, 0, PAGE_SIZE);
> +                       else if (hdr->good_bytes - bytes < PAGE_SIZE)
> +                               zero_user_segment(page,
> +                                       hdr->good_bytes & ~PAGE_MASK,
> +                                       PAGE_SIZE);
>                }
> -       } else {
> -               while (!list_empty(&hdr->pages)) {
> -                       struct nfs_page *req = nfs_list_entry(hdr->pages.next);
> -
> -                       bytes += req->wb_bytes;
> +               bytes += req->wb_bytes;
> +               if (test_bit(NFS_IOHDR_ERROR, &hdr->flags)) {
>                        if (bytes <= hdr->good_bytes)
>                                SetPageUptodate(req->wb_page);
> -                       nfs_list_remove_request(req);
> -                       nfs_readpage_release(req);
> -               }
> +               } else
> +                       SetPageUptodate(page);
> +               nfs_list_remove_request(req);
> +               nfs_readpage_release(req);
>        }
>  out:
>        hdr->release(hdr);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions
  2012-05-01 19:12             ` Fred Isaman
@ 2012-05-01 19:38               ` Myklebust, Trond
  0 siblings, 0 replies; 9+ messages in thread
From: Myklebust, Trond @ 2012-05-01 19:38 UTC (permalink / raw)
  To: Isaman, Fred; +Cc: linux-nfs@vger.kernel.org

T24gVHVlLCAyMDEyLTA1LTAxIGF0IDE1OjEyIC0wNDAwLCBGcmVkIElzYW1hbiB3cm90ZToNCj4g
T24gVHVlLCBNYXkgMSwgMjAxMiBhdCAyOjQwIFBNLCBUcm9uZCBNeWtsZWJ1c3QNCj4gPFRyb25k
Lk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gPiBTaWduZWQtb2ZmLWJ5OiBUcm9uZCBN
eWtsZWJ1c3QgPFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPg0KPiA+IENjOiBGcmVkIElzYW1h
biA8aWlzYW1hbkBuZXRhcHAuY29tPg0KPiA+IC0tLQ0KPiA+ICBmcy9uZnMvZGlyZWN0LmMgfCAg
IDQ2ICsrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gPiAg
ZnMvbmZzL3JlYWQuYyAgIHwgICA0MiArKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0NCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCAzNiBpbnNlcnRpb25zKCspLCA1MiBkZWxl
dGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9mcy9uZnMvZGlyZWN0LmMgYi9mcy9uZnMv
ZGlyZWN0LmMNCj4gPiBpbmRleCBmMTdlNDY5Li5hMWQzNjZiIDEwMDY0NA0KPiA+IC0tLSBhL2Zz
L25mcy9kaXJlY3QuYw0KPiA+ICsrKyBiL2ZzL25mcy9kaXJlY3QuYw0KPiA+IEBAIC0yNDMsMzYg
KzI0MywyOCBAQCBzdGF0aWMgdm9pZCBuZnNfZGlyZWN0X3JlYWRfY29tcGxldGlvbihzdHJ1Y3Qg
bmZzX3BnaW9faGVhZGVyICpoZHIpDQo+ID4gICAgICAgICAgICAgICAgZHJlcS0+Y291bnQgKz0g
aGRyLT5nb29kX2J5dGVzOw0KPiA+ICAgICAgICBzcGluX3VubG9jaygmZHJlcS0+bG9jayk7DQo+
ID4NCj4gPiAtICAgICAgIGlmICghdGVzdF9iaXQoTkZTX0lPSERSX0VSUk9SLCAmaGRyLT5mbGFn
cykpIHsNCj4gPiAtICAgICAgICAgICAgICAgd2hpbGUgKCFsaXN0X2VtcHR5KCZoZHItPnBhZ2Vz
KSkgew0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBuZnNfcGFnZSAqcmVxID0g
bmZzX2xpc3RfZW50cnkoaGRyLT5wYWdlcy5uZXh0KTsNCj4gPiAtICAgICAgICAgICAgICAgICAg
ICAgICBzdHJ1Y3QgcGFnZSAqcGFnZSA9IHJlcS0+d2JfcGFnZTsNCj4gPiAtDQo+ID4gLSAgICAg
ICAgICAgICAgICAgICAgICAgaWYgKHRlc3RfYml0KE5GU19JT0hEUl9FT0YsICZoZHItPmZsYWdz
KSkgew0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaWYgKGJ5dGVzID4gaGRy
LT5nb29kX2J5dGVzKQ0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICB6ZXJvX3VzZXIocGFnZSwgMCwgUEFHRV9TSVpFKTsNCj4gPiAtICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIGVsc2UgaWYgKGhkci0+Z29vZF9ieXRlcyAtIGJ5dGVzIDwgUEFHRV9TSVpF
KQ0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB6ZXJvX3VzZXJf
c2VnbWVudChwYWdlLA0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgIGhkci0+Z29vZF9ieXRlcyAmIH5QQUdFX01BU0ssDQo+ID4gLSAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUEFHRV9TSVpFKTsNCj4gPiAtICAg
ICAgICAgICAgICAgICAgICAgICB9DQo+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgYnl0ZXMg
Kz0gcmVxLT53Yl9ieXRlczsNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICBuZnNfbGlzdF9y
ZW1vdmVfcmVxdWVzdChyZXEpOw0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgIGlmICghUGFn
ZUNvbXBvdW5kKHBhZ2UpKQ0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc2V0
X3BhZ2VfZGlydHkocGFnZSk7DQo+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgbmZzX2RpcmVj
dF9yZWFkcGFnZV9yZWxlYXNlKHJlcSk7DQo+ID4gKyAgICAgICB3aGlsZSAoIWxpc3RfZW1wdHko
Jmhkci0+cGFnZXMpKSB7DQo+ID4gKyAgICAgICAgICAgICAgIHN0cnVjdCBuZnNfcGFnZSAqcmVx
ID0gbmZzX2xpc3RfZW50cnkoaGRyLT5wYWdlcy5uZXh0KTsNCj4gPiArICAgICAgICAgICAgICAg
c3RydWN0IHBhZ2UgKnBhZ2UgPSByZXEtPndiX3BhZ2U7DQo+ID4gKw0KPiA+ICsgICAgICAgICAg
ICAgICBpZiAodGVzdF9iaXQoTkZTX0lPSERSX0VPRiwgJmhkci0+ZmxhZ3MpKSB7DQo+ID4gKyAg
ICAgICAgICAgICAgICAgICAgICAgaWYgKGJ5dGVzID4gaGRyLT5nb29kX2J5dGVzKQ0KPiA+ICsg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgemVyb191c2VyKHBhZ2UsIDAsIFBBR0VfU0la
RSk7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgZWxzZSBpZiAoaGRyLT5nb29kX2J5dGVz
IC0gYnl0ZXMgPCBQQUdFX1NJWkUpDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICB6ZXJvX3VzZXJfc2VnbWVudChwYWdlLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBoZHItPmdvb2RfYnl0ZXMgJiB+UEFHRV9NQVNLLA0KPiA+ICsgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBQQUdFX1NJWkUpOw0KPiA+ICAgICAgICAg
ICAgICAgIH0NCj4gPiAtICAgICAgIH0gZWxzZSB7DQo+ID4gLSAgICAgICAgICAgICAgIHdoaWxl
ICghbGlzdF9lbXB0eSgmaGRyLT5wYWdlcykpIHsNCj4gPiAtICAgICAgICAgICAgICAgICAgICAg
ICBzdHJ1Y3QgbmZzX3BhZ2UgKnJlcSA9IG5mc19saXN0X2VudHJ5KGhkci0+cGFnZXMubmV4dCk7
DQo+ID4gLQ0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgIGlmIChieXRlcyA8IGhkci0+Z29v
ZF9ieXRlcykNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlmICghUGFnZUNv
bXBvdW5kKHJlcS0+d2JfcGFnZSkpDQo+ID4gKyAgICAgICAgICAgICAgIGlmICghUGFnZUNvbXBv
dW5kKHBhZ2UpKSB7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgaWYgKHRlc3RfYml0KE5G
U19JT0hEUl9FUlJPUiwgJmhkci0+ZmxhZ3MpKSB7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBpZiAoYnl0ZXMgPCBoZHItPmdvb2RfYnl0ZXMpDQo+ID4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgc2V0X3BhZ2VfZGlydHkocmVxLT53Yl9wYWdlKTsN
Cj4gDQo+IFlvdSBjYW4gcy9yZXEtPndiX3BhZ2UvcGFnZS8NCg0KWWVwLiBXaWxsIGRvLi4uDQoN
Cj4gT3RoZXIgdGhhbiB0aGF0LCBsb29rcyBmaW5lLCBidXQgSSdsbCBub3RlIHRoYXQgbXkgaW50
ZW50IGluIHdyaXRpbmcNCj4gaXQgYXMgaXQgd2FzIHdhcyB0byBtb3ZlIHRoZSB0ZXN0X2JpdCBv
dXQgb2YgdGhlIGxvb3AuDQo+IEJ1dCBnaXZlbiB0aGUgRU9GIHRlc3QgdGhhdCB3YXMgc3RpbGwg
dGhlcmUsIEkgZ3Vlc3MgaXQgZG9lc24ndCBtYWtlDQo+IG11Y2ggZGlmZmVyZW5jZS4NCg0KVGhl
IG90aGVyIHRoaW5nIGlzIHRoYXQgaXQgYmVjb21lcyB1bm5lY2Vzc2FyaWx5IGRpZmZpY3VsdCB0
byByZWFkDQpiZWNhdXNlIG9mIHRoZSBsb29wcyB0aGF0IGRvIG1vc3RseSB0aGUgc2FtZSB0aGlu
ZywgZXhjZXB0IGZvciB0aGlzIG9uZQ0KbGl0dGxlIGJpdCBpbiB0aGUgbWlkZGxlLg0KDQotLSAN
ClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0K
VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-05-01 19:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-01 17:16 [PATCH v2 1/3] NFS: Read cleanups Trond Myklebust
2012-05-01 17:16 ` [PATCH v2 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
2012-05-01 17:16   ` [PATCH v2 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
2012-05-01 18:37     ` Myklebust, Trond
2012-05-01 18:40       ` [PATCH v3 1/3] NFS: Read cleanups Trond Myklebust
2012-05-01 18:40         ` [PATCH v3 2/3] NFS: Clean up nfs read and write error paths Trond Myklebust
2012-05-01 18:40           ` [PATCH v3 3/3] NFS: Simplify the nfs_read_completion functions Trond Myklebust
2012-05-01 19:12             ` Fred Isaman
2012-05-01 19:38               ` Myklebust, Trond

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).