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@xxxxxxxxxx wrote:
> >> From: Andy Adamson <andros@xxxxxxxxxx>
> >>
> >> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
> >> ---
> >> 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? 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@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
