On Thu, 2017-04-13 at 16:13 -0400, Anna Schumaker wrote:
>
> On 04/13/2017 03:16 PM, Anna Schumaker wrote:
> >
> >
> > On 04/13/2017 03:07 PM, Olga Kornievskaia wrote:
> > > On Thu, Apr 13, 2017 at 2:51 PM, Trond Myklebust
> > > <trondmy@xxxxxxxxxxxxxxx> wrote:
> > > > On Thu, 2017-04-13 at 14:00 -0400, Olga Kornievskaia wrote:
> > > > > Hi folks,
> > > > >
> > > > > Looking for suggestions on how to fix a kernel oops.
> > > > >
> > > > > It's possible that there is a ctrl-c when the COMMIT is send.
> > > > > In case
> > > > > of the COPY, it calls
> > > > > nfs_commit_file() which calls wait_on_commit() that is
> > > > > interrupted by
> > > > > the crtl-c and frees the nfs_page request. So when
> > > > > asynchronous
> > > > > COMMIT
> > > > > rpc comes back it tried to use the nfs_page request and gets
> > > > > the
> > > > > oops.
> > > > >
> > > >
> > > > Is that call to nfs_free_request() in nfs_commit_file()
> > > > correct?
> > >
> > > yes, nfs_commit_file() creates a new request via
> > > nfs_create_request()
> > > and in the end if calls nfs_free_request();
> > >
> > > > It looks to me as if the same request will be freed in
> > > > nfs_commit_release_pages().
> > >
> > > so nfs_commit_release_pages() thru the
> > > nfs_unlock_and_release_request() is going to call
> > > nfs_release_request() from req->wb_kref.. I'm not sure if this is
> > > setup(?) for the copy commit path?
> > >
> > > Otherwise, it would have seem that we'd be doing a double free
> > > and I
> > > haven't seen that in testing (not that it can't be true)...
> >
> > I haven't seen any double-free messages during my testing either,
> > so I thought it was okay. It's possible I'm wrong, though. I
> > wonder if this is something that memory poisoning can help figure
> > out?
>
> After some experimenting, I can still use the nfs_page after
> nfs_commit_inode() has returned withotu any memory issues.
>
Wait...
OK, so nfs_scan_commit_list() does indeed take a reference to the req-
>wb_kref, so that balances the call to nfs_release_request() in
nfs_commit_release_pages(), however it also means that you should not
be calling nfs_free_request(), since doing so circumvents the
refcounting mechanism.
Secondly, if you want to release the request and you are not sure
whether or not it got cleared off the inode's cinfo commit list yet,
you may also need to lock that request and call
nfs_clear_request_commit().
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
