debugfs read operations were returning the contents of an uninitialized u64.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
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
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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
> ---
> 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
>
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.
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.
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.