| Message ID | 20181019152905.32418-6-olga.kornievskaia@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | server-side support for "inter" SSC copy | expand |
On Fri, Oct 19, 2018 at 11:28:57AM -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Mark the open created for the source file on the destination > server. Then if this open is going thru a recovery, then fail > the recovery as we don't need to be recoving a "fake" open. > We need to fail the ongoing READs and vfs_copy_file_range(). Nothing to do with this patch, really, but: I'm not always a stickler about the 80-character-column rule, but, man, nfs4_reclaim_open_state is deeply nested, could that be broken up a little? Also I notice there's a printk("AGLO: ...") in there that you probably didn't mean to leave upstream? --b. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > fs/nfs/nfs4_fs.h | 1 + > fs/nfs/nfs4file.c | 1 + > fs/nfs/nfs4state.c | 14 ++++++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > index f229864..8b00c90 100644 > --- a/fs/nfs/nfs4_fs.h > +++ b/fs/nfs/nfs4_fs.h > @@ -165,6 +165,7 @@ enum { > NFS_STATE_CHANGE_WAIT, /* A state changing operation is outstanding */ > #ifdef CONFIG_NFS_V4_2 > NFS_CLNT_DST_SSC_COPY_STATE, /* dst server open state on client*/ > + NFS_SRV_SSC_COPY_STATE, /* ssc state on the dst server */ > #endif /* CONFIG_NFS_V4_2 */ > }; > > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c > index f82cb05..7909cd4 100644 > --- a/fs/nfs/nfs4file.c > +++ b/fs/nfs/nfs4file.c > @@ -309,6 +309,7 @@ struct file * > if (ctx->state == NULL) > goto out_stateowner; > > + set_bit(NFS_SRV_SSC_COPY_STATE, &ctx->state->flags); > set_bit(NFS_OPEN_STATE, &ctx->state->flags); > memcpy(&ctx->state->open_stateid.other, &stateid->other, > NFS4_STATEID_OTHER_SIZE); > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > index 62ae0fd..b0b82c6 100644 > --- a/fs/nfs/nfs4state.c > +++ b/fs/nfs/nfs4state.c > @@ -1606,6 +1606,9 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > { > struct nfs4_state *state; > int status = 0; > +#ifdef CONFIG_NFS_V4_2 > + bool found_ssc_copy_state = false; > +#endif /* CONFIG_NFS_V4_2 */ > > /* Note: we rely on the sp->so_states list being ordered > * so that we always reclaim open(O_RDWR) and/or open(O_WRITE) > @@ -1625,6 +1628,13 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > continue; > if (state->state == 0) > continue; > +#ifdef CONFIG_NFS_V4_2 > + if (test_bit(NFS_SRV_SSC_COPY_STATE, &state->flags)) { > + nfs4_state_mark_recovery_failed(state, -EIO); > + found_ssc_copy_state = true; > + continue; > + } > +#endif /* CONFIG_NFS_V4_2 */ > refcount_inc(&state->count); > spin_unlock(&sp->so_lock); > status = __nfs4_reclaim_open_state(sp, state, ops); > @@ -1671,6 +1681,10 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > } > raw_write_seqcount_end(&sp->so_reclaim_seqcount); > spin_unlock(&sp->so_lock); > +#ifdef CONFIG_NFS_V4_2 > + if (found_ssc_copy_state) > + return -EIO; > +#endif /* CONFIG_NFS_V4_2 */ > return 0; > out_err: > nfs4_put_open_state(state); > -- > 1.8.3.1
On Thu, Nov 1, 2018 at 4:20 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Oct 19, 2018 at 11:28:57AM -0400, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Mark the open created for the source file on the destination > > server. Then if this open is going thru a recovery, then fail > > the recovery as we don't need to be recoving a "fake" open. > > We need to fail the ongoing READs and vfs_copy_file_range(). > > Nothing to do with this patch, really, but: And because it not related to this patch, then I'm confused. > I'm not always a stickler about the 80-character-column rule, but, man, > nfs4_reclaim_open_state is deeply nested, could that be broken up a > little? really? it's nested? Anna reworked the reclaim state code and nesting has gone done considerably. It's only 2 tabs in? > Also I notice there's a printk("AGLO: ...") in there that you > probably didn't mean to leave upstream? I don't believe I've ever had a printk() in the code. I did have a leftover dprintk() and that has since been removed. It looks like my attempt at providing you with the client side patches might have created a problem where you are reviewing (now) an outdated code. I never expected you to review the client side patches. However, if you are so inclined then could you please just look at my latest submission (v8) of the client patches as oppose to looking the version submitted for the nfsd. > > --b. > > > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > --- > > fs/nfs/nfs4_fs.h | 1 + > > fs/nfs/nfs4file.c | 1 + > > fs/nfs/nfs4state.c | 14 ++++++++++++++ > > 3 files changed, 16 insertions(+) > > > > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h > > index f229864..8b00c90 100644 > > --- a/fs/nfs/nfs4_fs.h > > +++ b/fs/nfs/nfs4_fs.h > > @@ -165,6 +165,7 @@ enum { > > NFS_STATE_CHANGE_WAIT, /* A state changing operation is outstanding */ > > #ifdef CONFIG_NFS_V4_2 > > NFS_CLNT_DST_SSC_COPY_STATE, /* dst server open state on client*/ > > + NFS_SRV_SSC_COPY_STATE, /* ssc state on the dst server */ > > #endif /* CONFIG_NFS_V4_2 */ > > }; > > > > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c > > index f82cb05..7909cd4 100644 > > --- a/fs/nfs/nfs4file.c > > +++ b/fs/nfs/nfs4file.c > > @@ -309,6 +309,7 @@ struct file * > > if (ctx->state == NULL) > > goto out_stateowner; > > > > + set_bit(NFS_SRV_SSC_COPY_STATE, &ctx->state->flags); > > set_bit(NFS_OPEN_STATE, &ctx->state->flags); > > memcpy(&ctx->state->open_stateid.other, &stateid->other, > > NFS4_STATEID_OTHER_SIZE); > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > > index 62ae0fd..b0b82c6 100644 > > --- a/fs/nfs/nfs4state.c > > +++ b/fs/nfs/nfs4state.c > > @@ -1606,6 +1606,9 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > > { > > struct nfs4_state *state; > > int status = 0; > > +#ifdef CONFIG_NFS_V4_2 > > + bool found_ssc_copy_state = false; > > +#endif /* CONFIG_NFS_V4_2 */ > > > > /* Note: we rely on the sp->so_states list being ordered > > * so that we always reclaim open(O_RDWR) and/or open(O_WRITE) > > @@ -1625,6 +1628,13 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > > continue; > > if (state->state == 0) > > continue; > > +#ifdef CONFIG_NFS_V4_2 > > + if (test_bit(NFS_SRV_SSC_COPY_STATE, &state->flags)) { > > + nfs4_state_mark_recovery_failed(state, -EIO); > > + found_ssc_copy_state = true; > > + continue; > > + } > > +#endif /* CONFIG_NFS_V4_2 */ > > refcount_inc(&state->count); > > spin_unlock(&sp->so_lock); > > status = __nfs4_reclaim_open_state(sp, state, ops); > > @@ -1671,6 +1681,10 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs > > } > > raw_write_seqcount_end(&sp->so_reclaim_seqcount); > > spin_unlock(&sp->so_lock); > > +#ifdef CONFIG_NFS_V4_2 > > + if (found_ssc_copy_state) > > + return -EIO; > > +#endif /* CONFIG_NFS_V4_2 */ > > return 0; > > out_err: > > nfs4_put_open_state(state); > > -- > > 1.8.3.1
On Thu, Nov 01, 2018 at 04:38:43PM -0400, Olga Kornievskaia wrote: > On Thu, Nov 1, 2018 at 4:20 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > > > On Fri, Oct 19, 2018 at 11:28:57AM -0400, Olga Kornievskaia wrote: > > > From: Olga Kornievskaia <kolga@netapp.com> > > > > > > Mark the open created for the source file on the destination > > > server. Then if this open is going thru a recovery, then fail > > > the recovery as we don't need to be recoving a "fake" open. > > > We need to fail the ongoing READs and vfs_copy_file_range(). > > > > Nothing to do with this patch, really, but: > > And because it not related to this patch, then I'm confused. > > > I'm not always a stickler about the 80-character-column rule, but, man, > > nfs4_reclaim_open_state is deeply nested, could that be broken up a > > little? > > really? it's nested? Anna reworked the reclaim state code and nesting > has gone done considerably. It's only 2 tabs in? > > > Also I notice there's a printk("AGLO: ...") in there that you > > probably didn't mean to leave upstream? > > I don't believe I've ever had a printk() in the code. I did have a > leftover dprintk() and that has since been removed. Oh, I see, this was all fixed recently upstream by Anna's cb7a8384dc02 "NFS: Split out the body of nfs4_reclaim_open_state()", ignore me. --b.
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index f229864..8b00c90 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -165,6 +165,7 @@ enum { NFS_STATE_CHANGE_WAIT, /* A state changing operation is outstanding */ #ifdef CONFIG_NFS_V4_2 NFS_CLNT_DST_SSC_COPY_STATE, /* dst server open state on client*/ + NFS_SRV_SSC_COPY_STATE, /* ssc state on the dst server */ #endif /* CONFIG_NFS_V4_2 */ }; diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index f82cb05..7909cd4 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -309,6 +309,7 @@ struct file * if (ctx->state == NULL) goto out_stateowner; + set_bit(NFS_SRV_SSC_COPY_STATE, &ctx->state->flags); set_bit(NFS_OPEN_STATE, &ctx->state->flags); memcpy(&ctx->state->open_stateid.other, &stateid->other, NFS4_STATEID_OTHER_SIZE); diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 62ae0fd..b0b82c6 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1606,6 +1606,9 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs { struct nfs4_state *state; int status = 0; +#ifdef CONFIG_NFS_V4_2 + bool found_ssc_copy_state = false; +#endif /* CONFIG_NFS_V4_2 */ /* Note: we rely on the sp->so_states list being ordered * so that we always reclaim open(O_RDWR) and/or open(O_WRITE) @@ -1625,6 +1628,13 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs continue; if (state->state == 0) continue; +#ifdef CONFIG_NFS_V4_2 + if (test_bit(NFS_SRV_SSC_COPY_STATE, &state->flags)) { + nfs4_state_mark_recovery_failed(state, -EIO); + found_ssc_copy_state = true; + continue; + } +#endif /* CONFIG_NFS_V4_2 */ refcount_inc(&state->count); spin_unlock(&sp->so_lock); status = __nfs4_reclaim_open_state(sp, state, ops); @@ -1671,6 +1681,10 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp, const struct nfs } raw_write_seqcount_end(&sp->so_reclaim_seqcount); spin_unlock(&sp->so_lock); +#ifdef CONFIG_NFS_V4_2 + if (found_ssc_copy_state) + return -EIO; +#endif /* CONFIG_NFS_V4_2 */ return 0; out_err: nfs4_put_open_state(state);