| Message ID | 1468537115-65826-1-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Jul 14, 2016, at 18:58, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > > The non-retry error path is currently broken and ends up releasing the > reference to the layout twice. It also can end up clearing the > NFS_LAYOUT_FIRST_LAYOUTGET flag twice, causing a race. > > In addition, the retry path will fail to decrement the plh_outstanding > counter. > > Fixes: 183d9e7b112aa ("pnfs: rework LAYOUTGET retry handling") > Cc: stable@vger.kernel.org # 4.7 > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/pnfs.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > index 0fbe734cc38c..73b0dc90265a 100644 > --- a/fs/nfs/pnfs.c > +++ b/fs/nfs/pnfs.c > @@ -1645,6 +1645,7 @@ lookup_again: > lseg = send_layoutget(lo, ctx, &stateid, &arg, &timeout, gfp_flags); > trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, > PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET); > + atomic_dec(&lo->plh_outstanding); > if (IS_ERR(lseg)) { > switch(PTR_ERR(lseg)) { > case -ERECALLCONFLICT: > @@ -1652,26 +1653,25 @@ lookup_again: > lseg = NULL; > /* Fallthrough */ > case -EAGAIN: > - pnfs_put_layout_hdr(lo); > - if (first) > - pnfs_clear_first_layoutget(lo); > - if (lseg) { > - trace_pnfs_update_layout(ino, pos, count, > - iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); > - goto lookup_again; > - } > - /* Fallthrough */ > + break; > default: > if (!nfs_error_is_fatal(PTR_ERR(lseg))) { > pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); > lseg = NULL; > } > } > + if (lseg) { > + pnfs_put_layout_hdr(lo); Yes, I’m aware I need to move this to after the trace_pnfs_update_layout() below (and before “goto lookup_again”). I managed to send out an earlier version of the patch that is being committed to linux-next. > + if (first) > + pnfs_clear_first_layoutget(lo); > + trace_pnfs_update_layout(ino, pos, count, > + iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); > + goto lookup_again; > + } > } else { > pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); > } > > - atomic_dec(&lo->plh_outstanding); > out_put_layout_hdr: > if (first) > pnfs_clear_first_layoutget(lo); > -- > 2.7.4 > -- 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
On Thu, 2016-07-14 at 18:58 -0400, Trond Myklebust wrote: > The non-retry error path is currently broken and ends up releasing the > reference to the layout twice. It also can end up clearing the > NFS_LAYOUT_FIRST_LAYOUTGET flag twice, causing a race. > > In addition, the retry path will fail to decrement the plh_outstanding > counter. > > Fixes: 183d9e7b112aa ("pnfs: rework LAYOUTGET retry handling") > Cc: stable@vger.kernel.org # 4.7 > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/pnfs.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > index 0fbe734cc38c..73b0dc90265a 100644 > --- a/fs/nfs/pnfs.c > +++ b/fs/nfs/pnfs.c > @@ -1645,6 +1645,7 @@ lookup_again: > lseg = send_layoutget(lo, ctx, &stateid, &arg, &timeout, gfp_flags); > trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, > PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET); > + atomic_dec(&lo->plh_outstanding); > if (IS_ERR(lseg)) { > switch(PTR_ERR(lseg)) { > case -ERECALLCONFLICT: > @@ -1652,26 +1653,25 @@ lookup_again: > lseg = NULL; > /* Fallthrough */ > case -EAGAIN: > - pnfs_put_layout_hdr(lo); > - if (first) > - pnfs_clear_first_layoutget(lo); > - if (lseg) { > - trace_pnfs_update_layout(ino, pos, count, > - iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); > - goto lookup_again; > - } > - /* Fallthrough */ > + break; > default: > if (!nfs_error_is_fatal(PTR_ERR(lseg))) { > pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); > lseg = NULL; > } > } > + if (lseg) { > + pnfs_put_layout_hdr(lo); > + if (first) > + pnfs_clear_first_layoutget(lo); > + trace_pnfs_update_layout(ino, pos, count, > + iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); > + goto lookup_again; > + } > } else { > pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); > } > > - atomic_dec(&lo->plh_outstanding); > out_put_layout_hdr: > if (first) > pnfs_clear_first_layoutget(lo); The whole set looks good to me. Nice catches all around! Reviewed-by: Jeff Layton <jlayton@redhat.com> -- 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
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index 0fbe734cc38c..73b0dc90265a 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -1645,6 +1645,7 @@ lookup_again: lseg = send_layoutget(lo, ctx, &stateid, &arg, &timeout, gfp_flags); trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg, PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET); + atomic_dec(&lo->plh_outstanding); if (IS_ERR(lseg)) { switch(PTR_ERR(lseg)) { case -ERECALLCONFLICT: @@ -1652,26 +1653,25 @@ lookup_again: lseg = NULL; /* Fallthrough */ case -EAGAIN: - pnfs_put_layout_hdr(lo); - if (first) - pnfs_clear_first_layoutget(lo); - if (lseg) { - trace_pnfs_update_layout(ino, pos, count, - iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); - goto lookup_again; - } - /* Fallthrough */ + break; default: if (!nfs_error_is_fatal(PTR_ERR(lseg))) { pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); lseg = NULL; } } + if (lseg) { + pnfs_put_layout_hdr(lo); + if (first) + pnfs_clear_first_layoutget(lo); + trace_pnfs_update_layout(ino, pos, count, + iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY); + goto lookup_again; + } } else { pnfs_layout_clear_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode)); } - atomic_dec(&lo->plh_outstanding); out_put_layout_hdr: if (first) pnfs_clear_first_layoutget(lo);
The non-retry error path is currently broken and ends up releasing the reference to the layout twice. It also can end up clearing the NFS_LAYOUT_FIRST_LAYOUTGET flag twice, causing a race. In addition, the retry path will fail to decrement the plh_outstanding counter. Fixes: 183d9e7b112aa ("pnfs: rework LAYOUTGET retry handling") Cc: stable@vger.kernel.org # 4.7 Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/pnfs.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)