All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release
@ 2012-05-22 12:09 andros
  2012-05-22 12:09 ` [PATCH 1/4] NFSv4.1 Just " andros
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: andros @ 2012-05-22 12:09 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4filelayout.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 474c630..8df91be 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -101,9 +101,6 @@ static void filelayout_reset_write(struct nfs_write_data *data)
 							&hdr->pages,
 							hdr->completion_ops);
 	}
-	/* balance nfs_get_client in filelayout_write_pagelist */
-	nfs_put_client(data->ds_clp);
-	data->ds_clp     = NULL;
 }
 
 static void filelayout_reset_read(struct nfs_read_data *data)
@@ -125,9 +122,6 @@ static void filelayout_reset_read(struct nfs_read_data *data)
 							&hdr->pages,
 							hdr->completion_ops);
 	}
-	/* balance nfs_get_client in filelayout_read_pagelist */
-	nfs_put_client(data->ds_clp);
-	data->ds_clp = NULL;
 }
 
 static int filelayout_async_handle_error(struct rpc_task *task,
@@ -326,8 +320,7 @@ static void filelayout_read_release(void *data)
 {
 	struct nfs_read_data *rdata = data;
 
-	if (!test_bit(NFS_IOHDR_REDO, &rdata->header->flags))
-		nfs_put_client(rdata->ds_clp);
+	nfs_put_client(rdata->ds_clp);
 	rdata->header->mds_ops->rpc_release(data);
 }
 
@@ -424,8 +417,7 @@ static void filelayout_write_release(void *data)
 {
 	struct nfs_write_data *wdata = data;
 
-	if (!test_bit(NFS_IOHDR_REDO, &wdata->header->flags))
-		nfs_put_client(wdata->ds_clp);
+	nfs_put_client(wdata->ds_clp);
 	wdata->header->mds_ops->rpc_release(data);
 }
 
-- 
1.7.7.6


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

* [PATCH 1/4] NFSv4.1 Just use nfs_put_client in filelayout release
  2012-05-22 12:09 [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release andros
@ 2012-05-22 12:09 ` andros
  2012-05-22 16:12   ` Andy Adamson
  2012-05-22 12:09 ` [PATCH 2/4] NFSv4.1 skip rpc_call_done only on disconnected DS slot_table_waitq tasks andros
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: andros @ 2012-05-22 12:09 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4filelayout.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 474c630..8df91be 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -101,9 +101,6 @@ static void filelayout_reset_write(struct nfs_write_data *data)
 							&hdr->pages,
 							hdr->completion_ops);
 	}
-	/* balance nfs_get_client in filelayout_write_pagelist */
-	nfs_put_client(data->ds_clp);
-	data->ds_clp     = NULL;
 }
 
 static void filelayout_reset_read(struct nfs_read_data *data)
@@ -125,9 +122,6 @@ static void filelayout_reset_read(struct nfs_read_data *data)
 							&hdr->pages,
 							hdr->completion_ops);
 	}
-	/* balance nfs_get_client in filelayout_read_pagelist */
-	nfs_put_client(data->ds_clp);
-	data->ds_clp = NULL;
 }
 
 static int filelayout_async_handle_error(struct rpc_task *task,
@@ -326,8 +320,7 @@ static void filelayout_read_release(void *data)
 {
 	struct nfs_read_data *rdata = data;
 
-	if (!test_bit(NFS_IOHDR_REDO, &rdata->header->flags))
-		nfs_put_client(rdata->ds_clp);
+	nfs_put_client(rdata->ds_clp);
 	rdata->header->mds_ops->rpc_release(data);
 }
 
@@ -424,8 +417,7 @@ static void filelayout_write_release(void *data)
 {
 	struct nfs_write_data *wdata = data;
 
-	if (!test_bit(NFS_IOHDR_REDO, &wdata->header->flags))
-		nfs_put_client(wdata->ds_clp);
+	nfs_put_client(wdata->ds_clp);
 	wdata->header->mds_ops->rpc_release(data);
 }
 
-- 
1.7.7.6


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

* [PATCH 2/4] NFSv4.1 skip rpc_call_done only on disconnected DS slot_table_waitq tasks
  2012-05-22 12:09 [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release andros
  2012-05-22 12:09 ` [PATCH 1/4] NFSv4.1 Just " andros
@ 2012-05-22 12:09 ` andros
  2012-05-22 12:09 ` [PATCH 3/4] NFSv4.1 fix null state reference in filelayout_async_handle_error andros
  2012-05-22 12:09 ` [PATCH 4/4] NFSv4.1 do not release page on commit mismatch andros
  3 siblings, 0 replies; 12+ messages in thread
From: andros @ 2012-05-22 12:09 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

We reset all I/O on a disconnected data server through the pgio layer indicated
by the NFS_IOHDR_REDO flag.

Differentiate between on-the-wire tasks returning with an error which must
call rpc_call_done and tasks woken from the data server slot_table_waitq
waiting for a session slot with a status of zero which call rpc_exit in
rpc_prepare and need to skip rpc_call_done.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4filelayout.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index 8df91be..db6f2a6 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -302,7 +302,8 @@ static void filelayout_read_call_done(struct rpc_task *task, void *data)
 
 	dprintk("--> %s task->tk_status %d\n", __func__, task->tk_status);
 
-	if (test_bit(NFS_IOHDR_REDO, &rdata->header->flags))
+	if (test_bit(NFS_IOHDR_REDO, &rdata->header->flags) &&
+	    task->tk_status == 0)
 		return;
 
 	/* Note this may cause RPC to be resent */
@@ -399,7 +400,8 @@ static void filelayout_write_call_done(struct rpc_task *task, void *data)
 {
 	struct nfs_write_data *wdata = data;
 
-	if (test_bit(NFS_IOHDR_REDO, &wdata->header->flags))
+	if (test_bit(NFS_IOHDR_REDO, &wdata->header->flags) &&
+	    task->tk_status == 0)
 		return;
 
 	/* Note this may cause RPC to be resent */
-- 
1.7.7.6


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

* [PATCH 3/4] NFSv4.1 fix null state reference in filelayout_async_handle_error
  2012-05-22 12:09 [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release andros
  2012-05-22 12:09 ` [PATCH 1/4] NFSv4.1 Just " andros
  2012-05-22 12:09 ` [PATCH 2/4] NFSv4.1 skip rpc_call_done only on disconnected DS slot_table_waitq tasks andros
@ 2012-05-22 12:09 ` andros
  2012-05-22 12:09 ` [PATCH 4/4] NFSv4.1 do not release page on commit mismatch andros
  3 siblings, 0 replies; 12+ messages in thread
From: andros @ 2012-05-22 12:09 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4filelayout.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index db6f2a6..cd3d1ce 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -193,8 +193,8 @@ static int filelayout_async_handle_error(struct rpc_task *task,
 		 * layout is destroyed and a new valid layout is obtained.
 		 */
 		set_bit(NFS_LAYOUT_INVALID,
-				&NFS_I(state->inode)->layout->plh_flags);
-		pnfs_destroy_layout(NFS_I(state->inode));
+				&NFS_I(inode)->layout->plh_flags);
+		pnfs_destroy_layout(NFS_I(inode));
 		rpc_wake_up(&tbl->slot_tbl_waitq);
 		goto reset;
 	/* RPC connection errors */
@@ -208,7 +208,7 @@ static int filelayout_async_handle_error(struct rpc_task *task,
 		dprintk("%s DS connection error %d\n", __func__,
 			task->tk_status);
 		if (!filelayout_test_devid_invalid(devid))
-			_pnfs_return_layout(state->inode);
+			_pnfs_return_layout(inode);
 		filelayout_mark_devid_invalid(devid);
 		rpc_wake_up(&tbl->slot_tbl_waitq);
 		nfs4_ds_disconnect(clp);
-- 
1.7.7.6


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

* [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 12:09 [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release andros
                   ` (2 preceding siblings ...)
  2012-05-22 12:09 ` [PATCH 3/4] NFSv4.1 fix null state reference in filelayout_async_handle_error andros
@ 2012-05-22 12:09 ` andros
  2012-05-22 16:24   ` Myklebust, Trond
  3 siblings, 1 reply; 12+ messages in thread
From: andros @ 2012-05-22 12:09 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/write.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index e6fe3d6..c7295de 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1555,6 +1555,8 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)
 		/* We have a mismatch. Write the page again */
 		dprintk(" mismatch\n");
 		nfs_mark_request_dirty(req);
+		nfs_unlock_request(req);
+		continue;
 	next:
 		nfs_unlock_and_release_request(req);
 	}
-- 
1.7.7.6


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

* Re: [PATCH 1/4] NFSv4.1 Just use nfs_put_client in filelayout release
  2012-05-22 12:09 ` [PATCH 1/4] NFSv4.1 Just " andros
@ 2012-05-22 16:12   ` Andy Adamson
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Adamson @ 2012-05-22 16:12 UTC (permalink / raw)
  To: trond.myklebust; +Cc: linux-nfs, Andy Adamson

Sorry - this is a repeat...

-->Andy

On Tue, May 22, 2012 at 8:09 AM,  <andros@netapp.com> wrote:
> From: Andy Adamson <andros@netapp.com>
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4filelayout.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index 474c630..8df91be 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -101,9 +101,6 @@ static void filelayout_reset_write(struct nfs_write_data *data)
>                                                        &hdr->pages,
>                                                        hdr->completion_ops);
>        }
> -       /* balance nfs_get_client in filelayout_write_pagelist */
> -       nfs_put_client(data->ds_clp);
> -       data->ds_clp     = NULL;
>  }
>
>  static void filelayout_reset_read(struct nfs_read_data *data)
> @@ -125,9 +122,6 @@ static void filelayout_reset_read(struct nfs_read_data *data)
>                                                        &hdr->pages,
>                                                        hdr->completion_ops);
>        }
> -       /* balance nfs_get_client in filelayout_read_pagelist */
> -       nfs_put_client(data->ds_clp);
> -       data->ds_clp = NULL;
>  }
>
>  static int filelayout_async_handle_error(struct rpc_task *task,
> @@ -326,8 +320,7 @@ static void filelayout_read_release(void *data)
>  {
>        struct nfs_read_data *rdata = data;
>
> -       if (!test_bit(NFS_IOHDR_REDO, &rdata->header->flags))
> -               nfs_put_client(rdata->ds_clp);
> +       nfs_put_client(rdata->ds_clp);
>        rdata->header->mds_ops->rpc_release(data);
>  }
>
> @@ -424,8 +417,7 @@ static void filelayout_write_release(void *data)
>  {
>        struct nfs_write_data *wdata = data;
>
> -       if (!test_bit(NFS_IOHDR_REDO, &wdata->header->flags))
> -               nfs_put_client(wdata->ds_clp);
> +       nfs_put_client(wdata->ds_clp);
>        wdata->header->mds_ops->rpc_release(data);
>  }
>
> --
> 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] 12+ messages in thread

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 12:09 ` [PATCH 4/4] NFSv4.1 do not release page on commit mismatch andros
@ 2012-05-22 16:24   ` Myklebust, Trond
  2012-05-22 16:30     ` Adamson, Andy
  0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-05-22 16:24 UTC (permalink / raw)
  To: Adamson, Andy; +Cc: linux-nfs

T24gVHVlLCAyMDEyLTA1LTIyIGF0IDA4OjA5IC0wNDAwLCBhbmRyb3NAbmV0YXBwLmNvbSB3cm90
ZToNCj4gRnJvbTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gDQo+IFNpZ25l
ZC1vZmYtYnk6IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQo+IC0tLQ0KPiAgZnMv
bmZzL3dyaXRlLmMgfCAgICAyICsrDQo+ICAxIGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygr
KSwgMCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvd3JpdGUuYyBiL2Zz
L25mcy93cml0ZS5jDQo+IGluZGV4IGU2ZmUzZDYuLmM3Mjk1ZGUgMTAwNjQ0DQo+IC0tLSBhL2Zz
L25mcy93cml0ZS5jDQo+ICsrKyBiL2ZzL25mcy93cml0ZS5jDQo+IEBAIC0xNTU1LDYgKzE1NTUs
OCBAQCBzdGF0aWMgdm9pZCBuZnNfY29tbWl0X3JlbGVhc2VfcGFnZXMoc3RydWN0IG5mc19jb21t
aXRfZGF0YSAqZGF0YSkNCj4gIAkJLyogV2UgaGF2ZSBhIG1pc21hdGNoLiBXcml0ZSB0aGUgcGFn
ZSBhZ2FpbiAqLw0KPiAgCQlkcHJpbnRrKCIgbWlzbWF0Y2hcbiIpOw0KPiAgCQluZnNfbWFya19y
ZXF1ZXN0X2RpcnR5KHJlcSk7DQo+ICsJCW5mc191bmxvY2tfcmVxdWVzdChyZXEpOw0KPiArCQlj
b250aW51ZTsNCj4gIAluZXh0Og0KPiAgCQluZnNfdW5sb2NrX2FuZF9yZWxlYXNlX3JlcXVlc3Qo
cmVxKTsNCj4gIAl9DQoNCldoYXQgaXMgdGhpcyBwYXRjaCB0cnlpbmcgdG8gZml4PyBBcyBmYXIg
YXMgSSBjYW4gc2VlIGl0IHdpbGwgbGVhZCB0byBhDQpyZWZlcmVuY2UgbGVhay4NCg0KLS0gDQpU
cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

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

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 16:24   ` Myklebust, Trond
@ 2012-05-22 16:30     ` Adamson, Andy
  2012-05-22 17:46       ` Myklebust, Trond
  0 siblings, 1 reply; 12+ messages in thread
From: Adamson, Andy @ 2012-05-22 16:30 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Adamson, Andy, linux-nfs


On May 22, 2012, at 12:24 PM, Myklebust, Trond wrote:

> On Tue, 2012-05-22 at 08:09 -0400, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>> 
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> fs/nfs/write.c |    2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> index e6fe3d6..c7295de 100644
>> --- a/fs/nfs/write.c
>> +++ b/fs/nfs/write.c
>> @@ -1555,6 +1555,8 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)
>> 		/* We have a mismatch. Write the page again */
>> 		dprintk(" mismatch\n");
>> 		nfs_mark_request_dirty(req);
>> +		nfs_unlock_request(req);
>> +		continue;
>> 	next:
>> 		nfs_unlock_and_release_request(req);
>> 	}
> 
> What is this patch trying to fix? As far as I can see it will lead to a
> reference leak.

The release of the page drops the refcount to zero. Since it is a mismatch, we want to continue to use the page, so nfs_page_find_request_locked is called we get the WARNING in kref_get, and an Oops in various places in pnfs_update_layout.

Here is the output of added printk's.

-->Andy

May 21 18:25:33 fedora-64-2 kernel: [  152.988921] NFS:       commit (0:35/2 4096@184320)
May 21 18:25:33 fedora-64-2 kernel: [  152.988923]  mismatch req ffff880042dbb800
May 21 18:25:33 fedora-64-2 kernel: [  152.988927] nfs_release_request WB req ffff880042dbb800 wb_kref 1
May 21 18:25:33 fedora-64-2 kernel: [  152.988932] put_lseg: lseg ffff880042d95380 ref 4 valid 0
May 21 18:25:33 fedora-64-2 kernel: [  152.988945] nfs_page_find_request_locked req ffff88003e033800
May 21 18:25:33 fedora-64-2 kernel: [  152.988949] nfs_page_find_request_locked WB req ffff88003e033800 wb_kref 0
May 21 18:25:33 fedora-64-2 kernel: [  152.988953] ------------[ cut here ]------------
May 21 18:25:33 fedora-64-2 kernel: [  152.989082] WARNING: at include/linux/kref.h:41 kref_get+0x20/0x2c [nfs]()




May 21 18:25:33 fedora-64-2 kernel: [  152.989484] kernel BUG at fs/nfs/pnfs.c:584!
May 21 18:25:33 fedora-64-2 kernel: [  152.989488] invalid opcode: 0000 [#1] SMP
May 21 18:25:33 fedora-64-2 kernel: [  152.989493] CPU 1
May 21 18:25:33 fedora-64-2 kernel: [  152.989495] Modules linked in: nfs_layout_nfsv41_files nfs auth_rpcgss nfs_acl fuse lockd rfcomm bnep ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ip6table_filter ip6_tables snd_ens1371 gameport snd_rawmidi snd_ac97_codec ac97_bus snd_seq snd_seq_device btusb snd_pcm joydev bluetooth rfkill ppdev snd_timer vmw_balloon microcode snd parport_pc parport soundcore snd_page_alloc e1000 i2c_piix4 i2c_core shpchp uinput sunrpc mptspi mptscsih mptbase scsi_transport_spi [last unloaded: scsi_wait_scan]
May 21 18:25:33 fedora-64-2 kernel: [  152.989552]
May 21 18:25:33 fedora-64-2 kernel: [  152.989557] Pid: 1595, comm: flush-0:35 Tainted: G        W    3.4.0-rc7+ #5 VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform
May 21 18:25:33 fedora-64-2 kernel: [  152.989566] RIP: 0010:[<ffffffffa02c7377>]  [<ffffffffa02c7377>] pnfs_update_layout+0x460/0x6a6 [nfs]

> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 


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

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 16:30     ` Adamson, Andy
@ 2012-05-22 17:46       ` Myklebust, Trond
  2012-05-22 18:07         ` Andy Adamson
  0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-05-22 17:46 UTC (permalink / raw)
  To: Adamson, Andy, Isaman, Fred; +Cc: linux-nfs

T24gVHVlLCAyMDEyLTA1LTIyIGF0IDE2OjMwICswMDAwLCBBZGFtc29uLCBBbmR5IHdyb3RlOg0K
PiBPbiBNYXkgMjIsIDIwMTIsIGF0IDEyOjI0IFBNLCBNeWtsZWJ1c3QsIFRyb25kIHdyb3RlOg0K
PiANCj4gPiBPbiBUdWUsIDIwMTItMDUtMjIgYXQgMDg6MDkgLTA0MDAsIGFuZHJvc0BuZXRhcHAu
Y29tIHdyb3RlOg0KPiA+PiBGcm9tOiBBbmR5IEFkYW1zb24gPGFuZHJvc0BuZXRhcHAuY29tPg0K
PiA+PiANCj4gPj4gU2lnbmVkLW9mZi1ieTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNv
bT4NCj4gPj4gLS0tDQo+ID4+IGZzL25mcy93cml0ZS5jIHwgICAgMiArKw0KPiA+PiAxIGZpbGVz
IGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gPj4gDQo+ID4+IGRp
ZmYgLS1naXQgYS9mcy9uZnMvd3JpdGUuYyBiL2ZzL25mcy93cml0ZS5jDQo+ID4+IGluZGV4IGU2
ZmUzZDYuLmM3Mjk1ZGUgMTAwNjQ0DQo+ID4+IC0tLSBhL2ZzL25mcy93cml0ZS5jDQo+ID4+ICsr
KyBiL2ZzL25mcy93cml0ZS5jDQo+ID4+IEBAIC0xNTU1LDYgKzE1NTUsOCBAQCBzdGF0aWMgdm9p
ZCBuZnNfY29tbWl0X3JlbGVhc2VfcGFnZXMoc3RydWN0IG5mc19jb21taXRfZGF0YSAqZGF0YSkN
Cj4gPj4gCQkvKiBXZSBoYXZlIGEgbWlzbWF0Y2guIFdyaXRlIHRoZSBwYWdlIGFnYWluICovDQo+
ID4+IAkJZHByaW50aygiIG1pc21hdGNoXG4iKTsNCj4gPj4gCQluZnNfbWFya19yZXF1ZXN0X2Rp
cnR5KHJlcSk7DQo+ID4+ICsJCW5mc191bmxvY2tfcmVxdWVzdChyZXEpOw0KPiA+PiArCQljb250
aW51ZTsNCj4gPj4gCW5leHQ6DQo+ID4+IAkJbmZzX3VubG9ja19hbmRfcmVsZWFzZV9yZXF1ZXN0
KHJlcSk7DQo+ID4+IAl9DQo+ID4gDQo+ID4gV2hhdCBpcyB0aGlzIHBhdGNoIHRyeWluZyB0byBm
aXg/IEFzIGZhciBhcyBJIGNhbiBzZWUgaXQgd2lsbCBsZWFkIHRvIGENCj4gPiByZWZlcmVuY2Ug
bGVhay4NCj4gDQo+IFRoZSByZWxlYXNlIG9mIHRoZSBwYWdlIGRyb3BzIHRoZSByZWZjb3VudCB0
byB6ZXJvLiBTaW5jZSBpdCBpcyBhIG1pc21hdGNoLCB3ZSB3YW50IHRvIGNvbnRpbnVlIHRvIHVz
ZSB0aGUgcGFnZSwgc28gbmZzX3BhZ2VfZmluZF9yZXF1ZXN0X2xvY2tlZCBpcyBjYWxsZWQgd2Ug
Z2V0IHRoZSBXQVJOSU5HIGluIGtyZWZfZ2V0LCBhbmQgYW4gT29wcyBpbiB2YXJpb3VzIHBsYWNl
cyBpbiBwbmZzX3VwZGF0ZV9sYXlvdXQuDQo+IA0KPiBIZXJlIGlzIHRoZSBvdXRwdXQgb2YgYWRk
ZWQgcHJpbnRrJ3MuDQo+IA0KPiAtLT5BbmR5DQo+IA0KPiBNYXkgMjEgMTg6MjU6MzMgZmVkb3Jh
LTY0LTIga2VybmVsOiBbICAxNTIuOTg4OTIxXSBORlM6ICAgICAgIGNvbW1pdCAoMDozNS8yIDQw
OTZAMTg0MzIwKQ0KPiBNYXkgMjEgMTg6MjU6MzMgZmVkb3JhLTY0LTIga2VybmVsOiBbICAxNTIu
OTg4OTIzXSAgbWlzbWF0Y2ggcmVxIGZmZmY4ODAwNDJkYmI4MDANCj4gTWF5IDIxIDE4OjI1OjMz
IGZlZG9yYS02NC0yIGtlcm5lbDogWyAgMTUyLjk4ODkyN10gbmZzX3JlbGVhc2VfcmVxdWVzdCBX
QiByZXEgZmZmZjg4MDA0MmRiYjgwMCB3Yl9rcmVmIDENCj4gTWF5IDIxIDE4OjI1OjMzIGZlZG9y
YS02NC0yIGtlcm5lbDogWyAgMTUyLjk4ODkzMl0gcHV0X2xzZWc6IGxzZWcgZmZmZjg4MDA0MmQ5
NTM4MCByZWYgNCB2YWxpZCAwDQo+IE1heSAyMSAxODoyNTozMyBmZWRvcmEtNjQtMiBrZXJuZWw6
IFsgIDE1Mi45ODg5NDVdIG5mc19wYWdlX2ZpbmRfcmVxdWVzdF9sb2NrZWQgcmVxIGZmZmY4ODAw
M2UwMzM4MDANCj4gTWF5IDIxIDE4OjI1OjMzIGZlZG9yYS02NC0yIGtlcm5lbDogWyAgMTUyLjk4
ODk0OV0gbmZzX3BhZ2VfZmluZF9yZXF1ZXN0X2xvY2tlZCBXQiByZXEgZmZmZjg4MDAzZTAzMzgw
MCB3Yl9rcmVmIDANCj4gTWF5IDIxIDE4OjI1OjMzIGZlZG9yYS02NC0yIGtlcm5lbDogWyAgMTUy
Ljk4ODk1M10gLS0tLS0tLS0tLS0tWyBjdXQgaGVyZSBdLS0tLS0tLS0tLS0tDQo+IE1heSAyMSAx
ODoyNTozMyBmZWRvcmEtNjQtMiBrZXJuZWw6IFsgIDE1Mi45ODkwODJdIFdBUk5JTkc6IGF0IGlu
Y2x1ZGUvbGludXgva3JlZi5oOjQxIGtyZWZfZ2V0KzB4MjAvMHgyYyBbbmZzXSgpDQoNCkFyZSB0
aGVzZSBjb21taXQtdG8tZHMgcmVxdWVzdHM/IEFzIGZhciBhcyBJIGNhbiBzZWUsIHRoZXJlIGlz
IGEgYnVnIGluDQp0cmFuc2Zlcl9jb21taXRfbGlzdCgpIGluIHRoYXQgaXQgbG9ja3MgdGhlIHJl
cXVlc3RzLCBidXQgZG9lcyBub3QNCnJlZmVyZW5jZSB0aGVtLg0KDQpGcmVkPw0KDQotLSANClRy
b25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

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

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 17:46       ` Myklebust, Trond
@ 2012-05-22 18:07         ` Andy Adamson
  2012-05-22 20:40           ` Myklebust, Trond
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Adamson @ 2012-05-22 18:07 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Adamson, Andy, Isaman, Fred, linux-nfs

On Tue, May 22, 2012 at 1:46 PM, Myklebust, Trond
<Trond.Myklebust@netapp.com> wrote:
> On Tue, 2012-05-22 at 16:30 +0000, Adamson, Andy wrote:
>> On May 22, 2012, at 12:24 PM, Myklebust, Trond wrote:
>>
>> > On Tue, 2012-05-22 at 08:09 -0400, andros@netapp.com wrote:
>> >> From: Andy Adamson <andros@netapp.com>
>> >>
>> >> Signed-off-by: Andy Adamson <andros@netapp.com>
>> >> ---
>> >> fs/nfs/write.c |    2 ++
>> >> 1 files changed, 2 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> >> index e6fe3d6..c7295de 100644
>> >> --- a/fs/nfs/write.c
>> >> +++ b/fs/nfs/write.c
>> >> @@ -1555,6 +1555,8 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)
>> >>            /* We have a mismatch. Write the page again */
>> >>            dprintk(" mismatch\n");
>> >>            nfs_mark_request_dirty(req);
>> >> +          nfs_unlock_request(req);
>> >> +          continue;
>> >>    next:
>> >>            nfs_unlock_and_release_request(req);
>> >>    }
>> >
>> > What is this patch trying to fix? As far as I can see it will lead to a
>> > reference leak.
>>
>> The release of the page drops the refcount to zero. Since it is a mismatch, we want to continue to use the page, so nfs_page_find_request_locked is called we get the WARNING in kref_get, and an Oops in various places in pnfs_update_layout.
>>
>> Here is the output of added printk's.
>>
>> -->Andy
>>
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988921] NFS:       commit (0:35/2 4096@184320)
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988923]  mismatch req ffff880042dbb800
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988927] nfs_release_request WB req ffff880042dbb800 wb_kref 1
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988932] put_lseg: lseg ffff880042d95380 ref 4 valid 0
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988945] nfs_page_find_request_locked req ffff88003e033800
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988949] nfs_page_find_request_locked WB req ffff88003e033800 wb_kref 0
>> May 21 18:25:33 fedora-64-2 kernel: [  152.988953] ------------[ cut here ]------------
>> May 21 18:25:33 fedora-64-2 kernel: [  152.989082] WARNING: at include/linux/kref.h:41 kref_get+0x20/0x2c [nfs]()
>
> Are these commit-to-ds requests?

Yes

-->Andy

> As far as I can see, there is a bug in
> transfer_commit_list() in that it locks the requests, but does not
> reference them.
>
> Fred?
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>

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

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 18:07         ` Andy Adamson
@ 2012-05-22 20:40           ` Myklebust, Trond
  2012-05-23 13:35             ` Adamson, Andy
  0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-05-22 20:40 UTC (permalink / raw)
  To: Andy Adamson; +Cc: Adamson, Andy, Isaman, Fred, linux-nfs

T24gVHVlLCAyMDEyLTA1LTIyIGF0IDE0OjA3IC0wNDAwLCBBbmR5IEFkYW1zb24gd3JvdGU6DQo+
IE9uIFR1ZSwgTWF5IDIyLCAyMDEyIGF0IDE6NDYgUE0sIE15a2xlYnVzdCwgVHJvbmQNCj4gPFRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gPiBPbiBUdWUsIDIwMTItMDUtMjIg
YXQgMTY6MzAgKzAwMDAsIEFkYW1zb24sIEFuZHkgd3JvdGU6DQo+ID4+IE9uIE1heSAyMiwgMjAx
MiwgYXQgMTI6MjQgUE0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4+DQo+ID4+ID4gT24g
VHVlLCAyMDEyLTA1LTIyIGF0IDA4OjA5IC0wNDAwLCBhbmRyb3NAbmV0YXBwLmNvbSB3cm90ZToN
Cj4gPj4gPj4gRnJvbTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gPj4gPj4N
Cj4gPj4gPj4gU2lnbmVkLW9mZi1ieTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4N
Cj4gPj4gPj4gLS0tDQo+ID4+ID4+IGZzL25mcy93cml0ZS5jIHwgICAgMiArKw0KPiA+PiA+PiAx
IGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gPj4gPj4N
Cj4gPj4gPj4gZGlmZiAtLWdpdCBhL2ZzL25mcy93cml0ZS5jIGIvZnMvbmZzL3dyaXRlLmMNCj4g
Pj4gPj4gaW5kZXggZTZmZTNkNi4uYzcyOTVkZSAxMDA2NDQNCj4gPj4gPj4gLS0tIGEvZnMvbmZz
L3dyaXRlLmMNCj4gPj4gPj4gKysrIGIvZnMvbmZzL3dyaXRlLmMNCj4gPj4gPj4gQEAgLTE1NTUs
NiArMTU1NSw4IEBAIHN0YXRpYyB2b2lkIG5mc19jb21taXRfcmVsZWFzZV9wYWdlcyhzdHJ1Y3Qg
bmZzX2NvbW1pdF9kYXRhICpkYXRhKQ0KPiA+PiA+PiAgICAgICAgICAgIC8qIFdlIGhhdmUgYSBt
aXNtYXRjaC4gV3JpdGUgdGhlIHBhZ2UgYWdhaW4gKi8NCj4gPj4gPj4gICAgICAgICAgICBkcHJp
bnRrKCIgbWlzbWF0Y2hcbiIpOw0KPiA+PiA+PiAgICAgICAgICAgIG5mc19tYXJrX3JlcXVlc3Rf
ZGlydHkocmVxKTsNCj4gPj4gPj4gKyAgICAgICAgICBuZnNfdW5sb2NrX3JlcXVlc3QocmVxKTsN
Cj4gPj4gPj4gKyAgICAgICAgICBjb250aW51ZTsNCj4gPj4gPj4gICAgbmV4dDoNCj4gPj4gPj4g
ICAgICAgICAgICBuZnNfdW5sb2NrX2FuZF9yZWxlYXNlX3JlcXVlc3QocmVxKTsNCj4gPj4gPj4g
ICAgfQ0KPiA+PiA+DQo+ID4+ID4gV2hhdCBpcyB0aGlzIHBhdGNoIHRyeWluZyB0byBmaXg/IEFz
IGZhciBhcyBJIGNhbiBzZWUgaXQgd2lsbCBsZWFkIHRvIGENCj4gPj4gPiByZWZlcmVuY2UgbGVh
ay4NCj4gPj4NCj4gPj4gVGhlIHJlbGVhc2Ugb2YgdGhlIHBhZ2UgZHJvcHMgdGhlIHJlZmNvdW50
IHRvIHplcm8uIFNpbmNlIGl0IGlzIGEgbWlzbWF0Y2gsIHdlIHdhbnQgdG8gY29udGludWUgdG8g
dXNlIHRoZSBwYWdlLCBzbyBuZnNfcGFnZV9maW5kX3JlcXVlc3RfbG9ja2VkIGlzIGNhbGxlZCB3
ZSBnZXQgdGhlIFdBUk5JTkcgaW4ga3JlZl9nZXQsIGFuZCBhbiBPb3BzIGluIHZhcmlvdXMgcGxh
Y2VzIGluIHBuZnNfdXBkYXRlX2xheW91dC4NCj4gPj4NCj4gPj4gSGVyZSBpcyB0aGUgb3V0cHV0
IG9mIGFkZGVkIHByaW50aydzLg0KPiA+Pg0KPiA+PiAtLT5BbmR5DQo+ID4+DQo+ID4+IE1heSAy
MSAxODoyNTozMyBmZWRvcmEtNjQtMiBrZXJuZWw6IFsgIDE1Mi45ODg5MjFdIE5GUzogICAgICAg
Y29tbWl0ICgwOjM1LzIgNDA5NkAxODQzMjApDQo+ID4+IE1heSAyMSAxODoyNTozMyBmZWRvcmEt
NjQtMiBrZXJuZWw6IFsgIDE1Mi45ODg5MjNdICBtaXNtYXRjaCByZXEgZmZmZjg4MDA0MmRiYjgw
MA0KPiA+PiBNYXkgMjEgMTg6MjU6MzMgZmVkb3JhLTY0LTIga2VybmVsOiBbICAxNTIuOTg4OTI3
XSBuZnNfcmVsZWFzZV9yZXF1ZXN0IFdCIHJlcSBmZmZmODgwMDQyZGJiODAwIHdiX2tyZWYgMQ0K
PiA+PiBNYXkgMjEgMTg6MjU6MzMgZmVkb3JhLTY0LTIga2VybmVsOiBbICAxNTIuOTg4OTMyXSBw
dXRfbHNlZzogbHNlZyBmZmZmODgwMDQyZDk1MzgwIHJlZiA0IHZhbGlkIDANCj4gPj4gTWF5IDIx
IDE4OjI1OjMzIGZlZG9yYS02NC0yIGtlcm5lbDogWyAgMTUyLjk4ODk0NV0gbmZzX3BhZ2VfZmlu
ZF9yZXF1ZXN0X2xvY2tlZCByZXEgZmZmZjg4MDAzZTAzMzgwMA0KPiA+PiBNYXkgMjEgMTg6MjU6
MzMgZmVkb3JhLTY0LTIga2VybmVsOiBbICAxNTIuOTg4OTQ5XSBuZnNfcGFnZV9maW5kX3JlcXVl
c3RfbG9ja2VkIFdCIHJlcSBmZmZmODgwMDNlMDMzODAwIHdiX2tyZWYgMA0KPiA+PiBNYXkgMjEg
MTg6MjU6MzMgZmVkb3JhLTY0LTIga2VybmVsOiBbICAxNTIuOTg4OTUzXSAtLS0tLS0tLS0tLS1b
IGN1dCBoZXJlIF0tLS0tLS0tLS0tLS0NCj4gPj4gTWF5IDIxIDE4OjI1OjMzIGZlZG9yYS02NC0y
IGtlcm5lbDogWyAgMTUyLjk4OTA4Ml0gV0FSTklORzogYXQgaW5jbHVkZS9saW51eC9rcmVmLmg6
NDEga3JlZl9nZXQrMHgyMC8weDJjIFtuZnNdKCkNCj4gPg0KPiA+IEFyZSB0aGVzZSBjb21taXQt
dG8tZHMgcmVxdWVzdHM/DQo+IA0KPiBZZXMNCg0KT0ssIHNvIGhvdyBhYm91dCB0aGUgZm9sbG93
aW5nIGZpeDoNCg0KODwtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0NCkZyb20gNTNiOGVlMzQ2NDYzOTQ2Zjg4YjNlMTYzOWQ2ODhj
Mzg0ZGYxMTY2YyBNb24gU2VwIDE3IDAwOjAwOjAwIDIwMDENCkZyb206IFRyb25kIE15a2xlYnVz
dCA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+DQpEYXRlOiBUdWUsIDIyIE1heSAyMDEyIDE2
OjM2OjI3IC0wNDAwDQpTdWJqZWN0OiBbUEFUQ0hdIE5GU3Y0LjE6IEZpeCBhIGJhZCByZWZlcmVu
Y2UgY291bnQgaXNzdWUgaW4gdGhlIHBORlMgY29tbWl0DQogY29kZQ0KDQpmaWxlbGF5b3V0X3Nj
YW5fY29tbWl0X2xpc3RzIG5lZWRzIHRvIGJ1bXAgdGhlIHJlZmVyZW5jZSBjb3VudCBvbg0KdGhl
IHN0cnVjdCBuZnNfcGFnZSBqdXN0IGxpa2UgbmZzX3NjYW5fY29tbWl0X2xpc3QoKS4NCg0KUmVw
b3J0ZWQtYnk6IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQpTaWduZWQtb2ZmLWJ5
OiBUcm9uZCBNeWtsZWJ1c3QgPFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPg0KLS0tDQogZnMv
bmZzL25mczRmaWxlbGF5b3V0LmMgfCAgICAxICsNCiAxIGZpbGVzIGNoYW5nZWQsIDEgaW5zZXJ0
aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0ZmlsZWxh
eW91dC5jIGIvZnMvbmZzL25mczRmaWxlbGF5b3V0LmMNCmluZGV4IDQ3NGM2MzAuLjMzODQ5ZDMg
MTAwNjQ0DQotLS0gYS9mcy9uZnMvbmZzNGZpbGVsYXlvdXQuYw0KKysrIGIvZnMvbmZzL25mczRm
aWxlbGF5b3V0LmMNCkBAIC0xMTA2LDYgKzExMDYsNyBAQCB0cmFuc2Zlcl9jb21taXRfbGlzdChz
dHJ1Y3QgbGlzdF9oZWFkICpzcmMsIHN0cnVjdCBsaXN0X2hlYWQgKmRzdCwNCiAJbGlzdF9mb3Jf
ZWFjaF9lbnRyeV9zYWZlKHJlcSwgdG1wLCBzcmMsIHdiX2xpc3QpIHsNCiAJCWlmICghbmZzX2xv
Y2tfcmVxdWVzdChyZXEpKQ0KIAkJCWNvbnRpbnVlOw0KKwkJa3JlZl9nZXQoJnJlcS0+d2Jfa3Jl
Zik7DQogCQlpZiAoY29uZF9yZXNjaGVkX2xvY2soY2luZm8tPmxvY2spKQ0KIAkJCWxpc3Rfc2Fm
ZV9yZXNldF9uZXh0KHJlcSwgdG1wLCB3Yl9saXN0KTsNCiAJCW5mc19yZXF1ZXN0X3JlbW92ZV9j
b21taXRfbGlzdChyZXEsIGNpbmZvKTsNCi0tIA0KMS43LjcuNg0KDQoNCi0tIA0KVHJvbmQgTXlr
bGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWts
ZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

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

* Re: [PATCH 4/4] NFSv4.1 do not release page on commit mismatch
  2012-05-22 20:40           ` Myklebust, Trond
@ 2012-05-23 13:35             ` Adamson, Andy
  0 siblings, 0 replies; 12+ messages in thread
From: Adamson, Andy @ 2012-05-23 13:35 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: Andy Adamson, Adamson, Andy, Isaman, Fred, linux-nfs


On May 22, 2012, at 4:40 PM, Myklebust, Trond wrote:

> On Tue, 2012-05-22 at 14:07 -0400, Andy Adamson wrote:
>> On Tue, May 22, 2012 at 1:46 PM, Myklebust, Trond
>> <Trond.Myklebust@netapp.com> wrote:
>>> On Tue, 2012-05-22 at 16:30 +0000, Adamson, Andy wrote:
>>>> On May 22, 2012, at 12:24 PM, Myklebust, Trond wrote:
>>>> 
>>>>> On Tue, 2012-05-22 at 08:09 -0400, andros@netapp.com wrote:
>>>>>> From: Andy Adamson <andros@netapp.com>
>>>>>> 
>>>>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>>>>> ---
>>>>>> fs/nfs/write.c |    2 ++
>>>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>> 
>>>>>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>>>>>> index e6fe3d6..c7295de 100644
>>>>>> --- a/fs/nfs/write.c
>>>>>> +++ b/fs/nfs/write.c
>>>>>> @@ -1555,6 +1555,8 @@ static void nfs_commit_release_pages(struct nfs_commit_data *data)
>>>>>>           /* We have a mismatch. Write the page again */
>>>>>>           dprintk(" mismatch\n");
>>>>>>           nfs_mark_request_dirty(req);
>>>>>> +          nfs_unlock_request(req);
>>>>>> +          continue;
>>>>>>   next:
>>>>>>           nfs_unlock_and_release_request(req);
>>>>>>   }
>>>>> 
>>>>> What is this patch trying to fix? As far as I can see it will lead to a
>>>>> reference leak.
>>>> 
>>>> The release of the page drops the refcount to zero. Since it is a mismatch, we want to continue to use the page, so nfs_page_find_request_locked is called we get the WARNING in kref_get, and an Oops in various places in pnfs_update_layout.
>>>> 
>>>> Here is the output of added printk's.
>>>> 
>>>> -->Andy
>>>> 
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988921] NFS:       commit (0:35/2 4096@184320)
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988923]  mismatch req ffff880042dbb800
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988927] nfs_release_request WB req ffff880042dbb800 wb_kref 1
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988932] put_lseg: lseg ffff880042d95380 ref 4 valid 0
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988945] nfs_page_find_request_locked req ffff88003e033800
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988949] nfs_page_find_request_locked WB req ffff88003e033800 wb_kref 0
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.988953] ------------[ cut here ]------------
>>>> May 21 18:25:33 fedora-64-2 kernel: [  152.989082] WARNING: at include/linux/kref.h:41 kref_get+0x20/0x2c [nfs]()
>>> 
>>> Are these commit-to-ds requests?
>> 
>> Yes
> 
> OK, so how about the following fix:



Yep - works just fine.  Thanks!

-->Andy

> 
> 8<---------------------------------------------------------------
> From 53b8ee346463946f88b3e1639d688c384df1166c Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date: Tue, 22 May 2012 16:36:27 -0400
> Subject: [PATCH] NFSv4.1: Fix a bad reference count issue in the pNFS commit
> code
> 
> filelayout_scan_commit_lists needs to bump the reference count on
> the struct nfs_page just like nfs_scan_commit_list().
> 
> Reported-by: Andy Adamson <andros@netapp.com>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> fs/nfs/nfs4filelayout.c |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index 474c630..33849d3 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -1106,6 +1106,7 @@ transfer_commit_list(struct list_head *src, struct list_head *dst,
> 	list_for_each_entry_safe(req, tmp, src, wb_list) {
> 		if (!nfs_lock_request(req))
> 			continue;
> +		kref_get(&req->wb_kref);
> 		if (cond_resched_lock(cinfo->lock))
> 			list_safe_reset_next(req, tmp, wb_list);
> 		nfs_request_remove_commit_list(req, cinfo);
> -- 
> 1.7.7.6
> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 


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

end of thread, other threads:[~2012-05-23 13:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-22 12:09 [PATCH 1/4] NFSv4.1 just use nfs_put_client in filelayout release andros
2012-05-22 12:09 ` [PATCH 1/4] NFSv4.1 Just " andros
2012-05-22 16:12   ` Andy Adamson
2012-05-22 12:09 ` [PATCH 2/4] NFSv4.1 skip rpc_call_done only on disconnected DS slot_table_waitq tasks andros
2012-05-22 12:09 ` [PATCH 3/4] NFSv4.1 fix null state reference in filelayout_async_handle_error andros
2012-05-22 12:09 ` [PATCH 4/4] NFSv4.1 do not release page on commit mismatch andros
2012-05-22 16:24   ` Myklebust, Trond
2012-05-22 16:30     ` Adamson, Andy
2012-05-22 17:46       ` Myklebust, Trond
2012-05-22 18:07         ` Andy Adamson
2012-05-22 20:40           ` Myklebust, Trond
2012-05-23 13:35             ` Adamson, Andy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.