From: Scott Mayhew <smayhew@redhat.com>
To: Trond Myklebust <trondmy@primarydata.com>
Cc: "anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfs: make pnfs_destroy_layout() wait on commit when inode is being evicted
Date: Thu, 30 Nov 2017 09:37:44 -0500 [thread overview]
Message-ID: <20171130143744.odap43pjwh63xhrc@tonberry.usersys.redhat.com> (raw)
In-Reply-To: <1511987540.20638.10.camel@primarydata.com>
On Wed, 29 Nov 2017, Trond Myklebust wrote:
> On Mon, 2017-11-20 at 16:43 -0500, Scott Mayhew wrote:
> > Otherwise nfs_commit_inode() can set I_DIRTY_DATASYNC in the inode's
> > i_state, allowing the following BUG_ON in evict() to be triggered:
> >
> > BUG_ON(inode->i_state != (I_FREEING | I_CLEAR));
> >
> > Fixes: 1f18b82c3437 ("pNFS: Ensure we commit the layout if it has
> > been...")
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > Tested-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> > ---
> > fs/nfs/filelayout/filelayout.c | 2 +-
> > fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
> > fs/nfs/nfs4super.c | 2 +-
> > fs/nfs/pnfs.c | 6 +++---
> > fs/nfs/pnfs.h | 2 +-
> > 5 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/nfs/filelayout/filelayout.c
> > b/fs/nfs/filelayout/filelayout.c
> > index 4e54d8b..28b7228 100644
> > --- a/fs/nfs/filelayout/filelayout.c
> > +++ b/fs/nfs/filelayout/filelayout.c
> > @@ -169,7 +169,7 @@ static int filelayout_async_handle_error(struct
> > rpc_task *task,
> > * i/o and all i/o waiting on the slot table to the
> > MDS until
> > * layout is destroyed and a new valid layout is
> > obtained.
> > */
> > - pnfs_destroy_layout(NFS_I(inode));
> > + pnfs_destroy_layout(NFS_I(inode), 0);
> > rpc_wake_up(&tbl->slot_tbl_waitq);
> > goto reset;
> > /* RPC connection errors */
> > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
> > b/fs/nfs/flexfilelayout/flexfilelayout.c
> > index c75ad98..ab0a1e5 100644
> > --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> > @@ -1088,7 +1088,7 @@ static int
> > ff_layout_async_handle_error_v4(struct rpc_task *task,
> > * i/o and all i/o waiting on the slot table to the
> > MDS until
> > * layout is destroyed and a new valid layout is
> > obtained.
> > */
> > - pnfs_destroy_layout(NFS_I(inode));
> > + pnfs_destroy_layout(NFS_I(inode), 0);
> > rpc_wake_up(&tbl->slot_tbl_waitq);
> > goto reset;
> > /* RPC connection errors */
> > diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
> > index 6fb7cb6..3b4a063 100644
> > --- a/fs/nfs/nfs4super.c
> > +++ b/fs/nfs/nfs4super.c
> > @@ -95,7 +95,7 @@ static void nfs4_evict_inode(struct inode *inode)
> > nfs_inode_return_delegation_noreclaim(inode);
> > /* Note that above delegreturn would trigger pnfs return-on-
> > close */
> > pnfs_return_layout(inode);
> > - pnfs_destroy_layout(NFS_I(inode));
> > + pnfs_destroy_layout(NFS_I(inode), FLUSH_SYNC);
> > /* First call standard NFS clear_inode() code */
> > nfs_clear_inode(inode);
> > }
> >
>
> NACK. If you still have dirty pages when you get to nfs4_evict_inode(),
> then you are screwed no matter what.
>
> This problem should be fixable in nfs_commit_inode() by simply exiting
> after the call to nfs_commit_end() if res == 0.
Okay, I'll try that out. Thanks for looking!
-Scott
>
> --
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> trond.myklebust@primarydata.com
next prev parent reply other threads:[~2017-11-30 14:37 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-20 21:43 [PATCH] nfs: make pnfs_destroy_layout() wait on commit when inode is being evicted Scott Mayhew
2017-11-29 19:40 ` Anna Schumaker
2017-11-29 20:32 ` Trond Myklebust
2017-11-30 14:37 ` Scott Mayhew [this message]
2017-12-08 21:00 ` [PATCH] nfs: don't wait on commit in nfs_commit_inode() if there were no commit requests Scott Mayhew
2017-12-08 21:52 ` Trond Myklebust
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171130143744.odap43pjwh63xhrc@tonberry.usersys.redhat.com \
--to=smayhew@redhat.com \
--cc=anna.schumaker@netapp.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@primarydata.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.