| Message ID | 1391016982-10562-1-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > The check for whether or not we sent an RPC call in nfs40_sequence_done > is insufficient to decide whether or not we are holding a session slot, > and thus should not be used to decide when to free that slot. > > This patch replaces the RPC_WAS_SENT() test with the correct test for > whether or not slot == NULL. > > Cc: Chuck Lever <chuck.lever@oracle.com> > Cc: stable@vger.kernel.org # 3.12+ > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/nfs4proc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index ae00c3ed733f..493e9cce1f11 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, > struct nfs4_slot *slot = res->sr_slot; > struct nfs4_slot_table *tbl; > > - if (!RPC_WAS_SENT(task)) > + if (slot == NULL) > goto out; When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up? > > tbl = slot->table; > -- > 1.8.5.3 > -- Chuck Lever chuck[dot]lever[at]oracle[dot]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
On Jan 30, 2014, at 11:49, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > >> The check for whether or not we sent an RPC call in nfs40_sequence_done >> is insufficient to decide whether or not we are holding a session slot, >> and thus should not be used to decide when to free that slot. >> >> This patch replaces the RPC_WAS_SENT() test with the correct test for >> whether or not slot == NULL. >> >> Cc: Chuck Lever <chuck.lever@oracle.com> >> Cc: stable@vger.kernel.org # 3.12+ >> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> --- >> fs/nfs/nfs4proc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index ae00c3ed733f..493e9cce1f11 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, >> struct nfs4_slot *slot = res->sr_slot; >> struct nfs4_slot_table *tbl; >> >> - if (!RPC_WAS_SENT(task)) >> + if (slot == NULL) >> goto out; > > When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up? Unfortunately we can’t touch nfs4_sequence_done: it wants to test for whether or not a NFSv4.1 session exists, which requires it to look at slot->table->session. :-( -- Trond Myklebust Linux NFS client maintainer -- 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 Jan 30, 2014, at 8:55 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > > On Jan 30, 2014, at 11:49, Chuck Lever <chuck.lever@oracle.com> wrote: > >> >> On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >> >>> The check for whether or not we sent an RPC call in nfs40_sequence_done >>> is insufficient to decide whether or not we are holding a session slot, >>> and thus should not be used to decide when to free that slot. >>> >>> This patch replaces the RPC_WAS_SENT() test with the correct test for >>> whether or not slot == NULL. >>> >>> Cc: Chuck Lever <chuck.lever@oracle.com> >>> Cc: stable@vger.kernel.org # 3.12+ >>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>> --- >>> fs/nfs/nfs4proc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>> index ae00c3ed733f..493e9cce1f11 100644 >>> --- a/fs/nfs/nfs4proc.c >>> +++ b/fs/nfs/nfs4proc.c >>> @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, >>> struct nfs4_slot *slot = res->sr_slot; >>> struct nfs4_slot_table *tbl; >>> >>> - if (!RPC_WAS_SENT(task)) >>> + if (slot == NULL) >>> goto out; >> >> When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up? > > Unfortunately we can’t touch nfs4_sequence_done: it wants to test for whether or not a NFSv4.1 session exists, which requires it to look at slot->table->session. :-( Move the slot == NULL check to all nfs40_sequence_done() call sites? <shrug> If not, can we get a code comment that explains why that check is done twice in the common path? -- Chuck Lever chuck[dot]lever[at]oracle[dot]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
On Jan 30, 2014, at 12:02, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Jan 30, 2014, at 8:55 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > >> >> On Jan 30, 2014, at 11:49, Chuck Lever <chuck.lever@oracle.com> wrote: >> >>> >>> On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>> >>>> The check for whether or not we sent an RPC call in nfs40_sequence_done >>>> is insufficient to decide whether or not we are holding a session slot, >>>> and thus should not be used to decide when to free that slot. >>>> >>>> This patch replaces the RPC_WAS_SENT() test with the correct test for >>>> whether or not slot == NULL. >>>> >>>> Cc: Chuck Lever <chuck.lever@oracle.com> >>>> Cc: stable@vger.kernel.org # 3.12+ >>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>>> --- >>>> fs/nfs/nfs4proc.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>>> index ae00c3ed733f..493e9cce1f11 100644 >>>> --- a/fs/nfs/nfs4proc.c >>>> +++ b/fs/nfs/nfs4proc.c >>>> @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, >>>> struct nfs4_slot *slot = res->sr_slot; >>>> struct nfs4_slot_table *tbl; >>>> >>>> - if (!RPC_WAS_SENT(task)) >>>> + if (slot == NULL) >>>> goto out; >>> >>> When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up? >> >> Unfortunately we can’t touch nfs4_sequence_done: it wants to test for whether or not a NFSv4.1 session exists, which requires it to look at slot->table->session. :-( > > Move the slot == NULL check to all nfs40_sequence_done() call sites? <shrug> No... -- Trond Myklebust Linux NFS client maintainer -- 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 Jan 30, 2014, at 12:03, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > > On Jan 30, 2014, at 12:02, Chuck Lever <chuck.lever@oracle.com> wrote: > >> >> On Jan 30, 2014, at 8:55 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >> >>> >>> On Jan 30, 2014, at 11:49, Chuck Lever <chuck.lever@oracle.com> wrote: >>> >>>> >>>> On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>>> >>>>> The check for whether or not we sent an RPC call in nfs40_sequence_done >>>>> is insufficient to decide whether or not we are holding a session slot, >>>>> and thus should not be used to decide when to free that slot. >>>>> >>>>> This patch replaces the RPC_WAS_SENT() test with the correct test for >>>>> whether or not slot == NULL. >>>>> >>>>> Cc: Chuck Lever <chuck.lever@oracle.com> >>>>> Cc: stable@vger.kernel.org # 3.12+ >>>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>>>> --- >>>>> fs/nfs/nfs4proc.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>>>> index ae00c3ed733f..493e9cce1f11 100644 >>>>> --- a/fs/nfs/nfs4proc.c >>>>> +++ b/fs/nfs/nfs4proc.c >>>>> @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, >>>>> struct nfs4_slot *slot = res->sr_slot; >>>>> struct nfs4_slot_table *tbl; >>>>> >>>>> - if (!RPC_WAS_SENT(task)) >>>>> + if (slot == NULL) >>>>> goto out; >>>> >>>> When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up? >>> >>> Unfortunately we can’t touch nfs4_sequence_done: it wants to test for whether or not a NFSv4.1 session exists, which requires it to look at slot->table->session. :-( >> >> Move the slot == NULL check to all nfs40_sequence_done() call sites? <shrug> > > No... We could move the entire function minus the slot == NULL test into a helper that could be called by nfs4_sequence_done, but I’m not convinced that particular micro-optimisation is really worth it. -- Trond Myklebust Linux NFS client maintainer -- 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/nfs4proc.c b/fs/nfs/nfs4proc.c index ae00c3ed733f..493e9cce1f11 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task, struct nfs4_slot *slot = res->sr_slot; struct nfs4_slot_table *tbl; - if (!RPC_WAS_SENT(task)) + if (slot == NULL) goto out; tbl = slot->table;
The check for whether or not we sent an RPC call in nfs40_sequence_done is insufficient to decide whether or not we are holding a session slot, and thus should not be used to decide when to free that slot. This patch replaces the RPC_WAS_SENT() test with the correct test for whether or not slot == NULL. Cc: Chuck Lever <chuck.lever@oracle.com> Cc: stable@vger.kernel.org # 3.12+ Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)