* [PATCH] nfsd: return 0 on reads of fault injection files @ 2012-05-10 19:31 Weston Andros Adamson 2012-05-16 21:16 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Weston Andros Adamson @ 2012-05-10 19:31 UTC (permalink / raw) To: bfields; +Cc: bjschuma, linux-nfs, Weston Andros Adamson debugfs read operations were returning the contents of an uninitialized u64. Signed-off-by: Weston Andros Adamson <dros@netapp.com> --- I found this while working on (forthcoming) fault injection tests for CB_PATH_DOWN. fs/nfsd/fault_inject.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c index 46b7696..ab81144 100644 --- a/fs/nfsd/fault_inject.c +++ b/fs/nfsd/fault_inject.c @@ -62,6 +62,7 @@ static int nfsd_inject_set(void *op_ptr, u64 val) static int nfsd_inject_get(void *data, u64 *val) { + *val = 0; return 0; } -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: return 0 on reads of fault injection files 2012-05-10 19:31 [PATCH] nfsd: return 0 on reads of fault injection files Weston Andros Adamson @ 2012-05-16 21:16 ` J. Bruce Fields 2012-05-16 21:50 ` Adamson, Dros 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2012-05-16 21:16 UTC (permalink / raw) To: Weston Andros Adamson; +Cc: bjschuma, linux-nfs On Thu, May 10, 2012 at 03:31:10PM -0400, Weston Andros Adamson wrote: > debugfs read operations were returning the contents of an uninitialized u64. Thanks, applied. > > Signed-off-by: Weston Andros Adamson <dros@netapp.com> > --- > I found this while working on (forthcoming) fault injection tests for > CB_PATH_DOWN. Did you ever confirm whether the latest nfsd is setting that flag when it should be? --b. > > fs/nfsd/fault_inject.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c > index 46b7696..ab81144 100644 > --- a/fs/nfsd/fault_inject.c > +++ b/fs/nfsd/fault_inject.c > @@ -62,6 +62,7 @@ static int nfsd_inject_set(void *op_ptr, u64 val) > > static int nfsd_inject_get(void *data, u64 *val) > { > + *val = 0; > return 0; > } > > -- > 1.7.4.4 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: return 0 on reads of fault injection files 2012-05-16 21:16 ` J. Bruce Fields @ 2012-05-16 21:50 ` Adamson, Dros 2012-05-16 21:58 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Adamson, Dros @ 2012-05-16 21:50 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs list [-- Attachment #1: Type: text/plain, Size: 2025 bytes --] On May 16, 2012, at 5:16 PM, J. Bruce Fields wrote: > On Thu, May 10, 2012 at 03:31:10PM -0400, Weston Andros Adamson wrote: >> debugfs read operations were returning the contents of an uninitialized u64. > > Thanks, applied. > >> >> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >> --- >> I found this while working on (forthcoming) fault injection tests for >> CB_PATH_DOWN. > > Did you ever confirm whether the latest nfsd is setting that flag when > it should be? No, I haven't messed with it that much - I had other tasks take a higher priority, but I'm back on it as of this afternoon. I'll try nfsd-next to get CB_PATH_DOWN without fault_injection and report back to you. Also, I think we need to modify nfsd (nfsd4_new_conn()) to set cl_cb_state to NFSD4_CB_UNKNOWN on a successful BIND_CONN_TO_SESSION (IFF it's CB_DOWN with the right direction), otherwise the CB_PATH_DOWN flag will be set on every sequence OP and the client will keep sending BIND_CONN_TO_SESSION. The idea is that once we call nfsd4_new_conn, we won't know if the back channel is really up until a callback is attempted. Setting it to CB_UNKNOWN stops the loop of (sequence, bind_conn, sequence, bind_conn, …), but doesn't actually mark it as "UP" until a callback is successful. Expect patches soon. -dros > > --b. > >> >> fs/nfsd/fault_inject.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c >> index 46b7696..ab81144 100644 >> --- a/fs/nfsd/fault_inject.c >> +++ b/fs/nfsd/fault_inject.c >> @@ -62,6 +62,7 @@ static int nfsd_inject_set(void *op_ptr, u64 val) >> >> static int nfsd_inject_get(void *data, u64 *val) >> { >> + *val = 0; >> return 0; >> } >> >> -- >> 1.7.4.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 [-- Attachment #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 1374 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: return 0 on reads of fault injection files 2012-05-16 21:50 ` Adamson, Dros @ 2012-05-16 21:58 ` J. Bruce Fields 2012-05-16 22:10 ` Adamson, Dros 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2012-05-16 21:58 UTC (permalink / raw) To: Adamson, Dros; +Cc: linux-nfs list On Wed, May 16, 2012 at 09:50:32PM +0000, Adamson, Dros wrote: > No, I haven't messed with it that much - I had other tasks take a > higher priority, but I'm back on it as of this afternoon. > > I'll try nfsd-next to get CB_PATH_DOWN without fault_injection and > report back to you. Also, I think we need to modify nfsd > (nfsd4_new_conn()) to set cl_cb_state to NFSD4_CB_UNKNOWN on a > successful BIND_CONN_TO_SESSION (IFF it's CB_DOWN with the right > direction), otherwise the CB_PATH_DOWN flag will be set on every > sequence OP That's intentional: http://www.ietf.org/mail-archive/web/nfsv4/current/msg10840.html > and the client will keep sending BIND_CONN_TO_SESSION. > The idea is that once we call nfsd4_new_conn, we won't know if the > back channel is really up until a callback is attempted. If the client hasn't given us a connection to use as the backchannel, then we *know* the backchannel is down, we don't need to try sending a callback (how could we?). > Setting it to CB_UNKNOWN stops the loop of (sequence, bind_conn, > sequence, bind_conn, …), but doesn't actually mark it as "UP" until a > callback is successful. If the client is attempting to clear the CB_PATH_DOWN flag by sending us a BIND_CONN_TO_SESSION that doesn't actually give us a backchannel, then it's confused. If the client really doesn't care whether the backchannel is down or not then it can ignore the flag--it's a flag, not an error. --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: return 0 on reads of fault injection files 2012-05-16 21:58 ` J. Bruce Fields @ 2012-05-16 22:10 ` Adamson, Dros 2012-05-17 11:33 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Adamson, Dros @ 2012-05-16 22:10 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs list [-- Attachment #1: Type: text/plain, Size: 2102 bytes --] On May 16, 2012, at 5:58 PM, J. Bruce Fields wrote: > On Wed, May 16, 2012 at 09:50:32PM +0000, Adamson, Dros wrote: >> No, I haven't messed with it that much - I had other tasks take a >> higher priority, but I'm back on it as of this afternoon. >> >> I'll try nfsd-next to get CB_PATH_DOWN without fault_injection and >> report back to you. Also, I think we need to modify nfsd >> (nfsd4_new_conn()) to set cl_cb_state to NFSD4_CB_UNKNOWN on a >> successful BIND_CONN_TO_SESSION (IFF it's CB_DOWN with the right >> direction), otherwise the CB_PATH_DOWN flag will be set on every >> sequence OP > > That's intentional: > > http://www.ietf.org/mail-archive/web/nfsv4/current/msg10840.html Right... > >> and the client will keep sending BIND_CONN_TO_SESSION. >> The idea is that once we call nfsd4_new_conn, we won't know if the >> back channel is really up until a callback is attempted. > > If the client hasn't given us a connection to use as the backchannel, > then we *know* the backchannel is down, we don't need to try sending a > callback (how could we?). The point is that the client *has* given us a connection to use as the backchannel with the BIND_CONN_TO_SESSION call. On the server, nfs4_new_conn() was called and was successful, but cl_cb_state is still set to CB_DOWN and will never change. > >> Setting it to CB_UNKNOWN stops the loop of (sequence, bind_conn, >> sequence, bind_conn, …), but doesn't actually mark it as "UP" until a >> callback is successful. > > If the client is attempting to clear the CB_PATH_DOWN flag by sending us > a BIND_CONN_TO_SESSION that doesn't actually give us a backchannel, then > it's confused. The client *is* giving us a backchannel by sending the server a BIND_CONNN_TO_SESSION! The server accepts the call, adds the "new connection" via nfs4_new_conn(), but the problem is the internal state of nfsd never changes from CB_DOWN. -dros > If the client really doesn't care whether the backchannel is down or not > then it can ignore the flag--it's a flag, not an error. > > --b. [-- Attachment #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 1374 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: return 0 on reads of fault injection files 2012-05-16 22:10 ` Adamson, Dros @ 2012-05-17 11:33 ` J. Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: J. Bruce Fields @ 2012-05-17 11:33 UTC (permalink / raw) To: Adamson, Dros; +Cc: linux-nfs list On Wed, May 16, 2012 at 10:10:55PM +0000, Adamson, Dros wrote: > > On May 16, 2012, at 5:58 PM, J. Bruce Fields wrote: > > > On Wed, May 16, 2012 at 09:50:32PM +0000, Adamson, Dros wrote: > >> No, I haven't messed with it that much - I had other tasks take a > >> higher priority, but I'm back on it as of this afternoon. > >> > >> I'll try nfsd-next to get CB_PATH_DOWN without fault_injection and > >> report back to you. Also, I think we need to modify nfsd > >> (nfsd4_new_conn()) to set cl_cb_state to NFSD4_CB_UNKNOWN on a > >> successful BIND_CONN_TO_SESSION (IFF it's CB_DOWN with the right > >> direction), otherwise the CB_PATH_DOWN flag will be set on every > >> sequence OP > > > > That's intentional: > > > > http://www.ietf.org/mail-archive/web/nfsv4/current/msg10840.html > > Right... > > > > >> and the client will keep sending BIND_CONN_TO_SESSION. > >> The idea is that once we call nfsd4_new_conn, we won't know if the > >> back channel is really up until a callback is attempted. > > > > If the client hasn't given us a connection to use as the backchannel, > > then we *know* the backchannel is down, we don't need to try sending a > > callback (how could we?). > > The point is that the client *has* given us a connection to use as the backchannel with the BIND_CONN_TO_SESSION call. > On the server, nfs4_new_conn() was called and was successful, but cl_cb_state is still set to CB_DOWN and will never change. > > > > >> Setting it to CB_UNKNOWN stops the loop of (sequence, bind_conn, > >> sequence, bind_conn, …), but doesn't actually mark it as "UP" until a > >> callback is successful. > > > > If the client is attempting to clear the CB_PATH_DOWN flag by sending us > > a BIND_CONN_TO_SESSION that doesn't actually give us a backchannel, then > > it's confused. > > The client *is* giving us a backchannel by sending the server a BIND_CONNN_TO_SESSION! The server accepts the call, adds the "new connection" via nfs4_new_conn(), but the problem is the internal state of nfsd never changes from CB_DOWN. Oh, I see, thanks! Sorry, I was reading too fast.... Yes, your fix sounds about right. --b. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-17 11:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-10 19:31 [PATCH] nfsd: return 0 on reads of fault injection files Weston Andros Adamson 2012-05-16 21:16 ` J. Bruce Fields 2012-05-16 21:50 ` Adamson, Dros 2012-05-16 21:58 ` J. Bruce Fields 2012-05-16 22:10 ` Adamson, Dros 2012-05-17 11:33 ` J. Bruce Fields
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).