From: Anna Schumaker <schumaker.anna@gmail.com>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/4] pNFS: Ensure we return the error if someone kills a waiting layoutget
Date: Tue, 12 Mar 2019 17:22:50 -0400 [thread overview]
Message-ID: <672c7858f622e78973296bc0a504114c7610b761.camel@gmail.com> (raw)
In-Reply-To: <6057b85eea9d1b25189e2c1ead89112c91180ff0.camel@hammerspace.com>
On Tue, 2019-03-12 at 20:12 +0000, Trond Myklebust wrote:
> On Tue, 2019-03-12 at 20:04 +0000, Schumaker, Anna wrote:
> > Hi Trond,
> >
> > I'm seeing a hang when testing xfstests generic/013 on v4.1 with pNFS
> > after this
> > patch:
> >
> > On Wed, 2018-09-05 at 14:07 -0400, Trond Myklebust wrote:
> > > If someone interrupts a wait on one or more outstanding layoutgets
> > > in
> > > pnfs_update_layout() then return the ERESTARTSYS/EINTR error.
> > >
> > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > ---
> > > fs/nfs/pnfs.c | 26 ++++++++++++++++----------
> > > 1 file changed, 16 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > > index e8f232de484f..7d9a51e6b847 100644
> > > --- a/fs/nfs/pnfs.c
> > > +++ b/fs/nfs/pnfs.c
> > > @@ -1740,16 +1740,16 @@ static bool pnfs_within_mdsthreshold(struct
> > > nfs_open_context *ctx,
> > > return ret;
> > > }
> > >
> > > -static bool pnfs_prepare_to_retry_layoutget(struct pnfs_layout_hdr
> > > *lo)
> > > +static int pnfs_prepare_to_retry_layoutget(struct pnfs_layout_hdr
> > > *lo)
> > > {
> > > /*
> > > * send layoutcommit as it can hold up layoutreturn due to lseg
> > > * reference
> > > */
> > > pnfs_layoutcommit_inode(lo->plh_inode, false);
> > > - return !wait_on_bit_action(&lo->plh_flags, NFS_LAYOUT_RETURN,
> > > + return wait_on_bit_action(&lo->plh_flags, NFS_LAYOUT_RETURN,
> > > nfs_wait_bit_killable,
> > > - TASK_UNINTERRUPTIBLE);
> > > + TASK_KILLABLE);
> > > }
> > >
> > > static void nfs_layoutget_begin(struct pnfs_layout_hdr *lo)
> > > @@ -1830,7 +1830,9 @@ pnfs_update_layout(struct inode *ino,
> > > }
> > >
> > > lookup_again:
> > > - nfs4_client_recover_expired_lease(clp);
> > > + lseg = ERR_PTR(nfs4_client_recover_expired_lease(clp));
> > > + if (IS_ERR(lseg))
> > > + goto out;
> > > first = false;
> > > spin_lock(&ino->i_lock);
> > > lo = pnfs_find_alloc_layout(ino, ctx, gfp_flags);
> > > @@ -1863,9 +1865,9 @@ pnfs_update_layout(struct inode *ino,
> > > if (list_empty(&lo->plh_segs) &&
> > > atomic_read(&lo->plh_outstanding) != 0) {
> > > spin_unlock(&ino->i_lock);
> > > - if (wait_var_event_killable(&lo->plh_outstanding,
> > > - atomic_read(&lo-
> > > > plh_outstanding) == 0
> > > - || !list_empty(&lo->plh_segs)))
> > > + lseg = ERR_PTR(wait_var_event_killable(&lo-
> > > > plh_outstanding,
> > > + atomic_read(&lo-
> > > > plh_outstanding)));
> > > + if (IS_ERR(lseg) || !list_empty(&lo->plh_segs))
> >
> > Was dropping the "== 0" condition attached to the atomic_read() here
> > a mistake?
> > I think what's happening is that my client is waiting for
> > plh_outstanding to be
> > anything other than 0 when there isn't any work left to do.
>
> Yes. That's a bug. How about the following patch?
This patch works for me, but for some reason doing "!atomic_read()" takes 8
minutes longer to complete compared to doing "atomic_read() == 0". I have not
run this multiple times to confirm that it's always the case.
Anna
>
> 8<---------------------------------------------------
> From 400417b05f3ec0531544ca5f94e64d838d8b8849 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Tue, 12 Mar 2019 16:04:51 -0400
> Subject: [PATCH] pNFS: Fix a typo in pnfs_update_layout
>
> We're supposed to wait for the outstanding layout count to go to zero,
> but that got lost somehow.
>
> Fixes: d03360aaf5cca ("pNFS: Ensure we return the error if someone...")
> Reported-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/pnfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 8247bd1634cb..7066cd7c7aff 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1889,7 +1889,7 @@ pnfs_update_layout(struct inode *ino,
> atomic_read(&lo->plh_outstanding) != 0) {
> spin_unlock(&ino->i_lock);
> lseg = ERR_PTR(wait_var_event_killable(&lo->plh_outstanding,
> - atomic_read(&lo->plh_outstanding)));
> + !atomic_read(&lo->plh_outstanding)));
> if (IS_ERR(lseg) || !list_empty(&lo->plh_segs))
> goto out_put_layout_hdr;
> pnfs_put_layout_hdr(lo);
> --
> 2.20.1
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
>
>
prev parent reply other threads:[~2019-03-12 21:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-05 18:07 [PATCH 1/4] pNFS: Ensure we return the error if someone kills a waiting layoutget Trond Myklebust
2018-09-05 18:07 ` [PATCH 2/4] NFSv4: Fix a tracepoint Oops in initiate_file_draining() Trond Myklebust
2018-09-05 18:07 ` [PATCH 3/4] NFSv4.1 fix infinite loop on I/O Trond Myklebust
2018-09-05 18:07 ` [PATCH 4/4] NFS: Don't open code clearing of delegation state Trond Myklebust
2019-03-12 20:04 ` [PATCH 1/4] pNFS: Ensure we return the error if someone kills a waiting layoutget Schumaker, Anna
2019-03-12 20:12 ` Trond Myklebust
2019-03-12 21:22 ` Anna Schumaker [this message]
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=672c7858f622e78973296bc0a504114c7610b761.camel@gmail.com \
--to=schumaker.anna@gmail.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@hammerspace.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 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).