mbox series

[v5,0/4] add support for CB_RECALL_ANY and the delegation reaper

Message ID 1668656688-22507-1-git-send-email-dai.ngo@oracle.com (mailing list archive)
Headers show
Series add support for CB_RECALL_ANY and the delegation reaper | expand

Message

Dai Ngo Nov. 17, 2022, 3:44 a.m. UTC
This patch series adds:

    . refactor courtesy_client_reaper to a generic low memory
      shrinker.

    . XDR encode and decode function for CB_RECALL_ANY op.

    . the delegation reaper that sends the advisory CB_RECALL_ANY 
      to the clients to release unused delegations.
      There is only one nfsd4_callback added for each nfs4_cleint.
      Access to it must be serialized via the client flag
      NFSD4_CLIENT_CB_RECALL_ANY.

    . Add CB_RECALL_ANY tracepoints.

v2:
    . modify deleg_reaper to check and send CB_RECALL_ANY to client
      only once per 5 secs.
v3:
    . modify nfsd4_cb_recall_any_release to use nn->client_lock to
      protect cl_recall_any_busy and call put_client_renew_locked
      to decrement cl_rpc_users.

v4:
    . move changes in nfs4state.c from patch (1/2) to patch(2/2).
    . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
      to encode CB_RECALL_ANY arguments.
    . add struct nfsd4_cb_recall_any with embedded nfs4_callback
      and params for CB_RECALL_ANY op.
    . replace cl_recall_any_busy boolean with client flag
      NFSD4_CLIENT_CB_RECALL_ANY 
    . add tracepoints for CB_RECALL_ANY

v5:
    . refactor courtesy_client_reaper to a generic low memory
      shrinker
    . merge courtesy client shrinker and delegtion shrinker into
      one.
    . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
      in nfsd/trace.h
    . use __get_sockaddr to display server IP address in
      tracepoints.
    . modify encode_cb_recallany4args to replace sizeof with
      ARRAY_SIZE.

---

Dai Ngo (4):
     NFSD: refactoring courtesy_client_reaper to a generic low memory shrinker
     NFSD: add support for sending CB_RECALL_ANY
     NFSD: add delegation shrinker to react to low memory condition
     NFSD: add CB_RECALL_ANY tracepoints

 fs/nfsd/nfs4callback.c |  62 +++++++++++++++++++++++
 fs/nfsd/nfs4state.c    | 116 +++++++++++++++++++++++++++++++++++++++-----
 fs/nfsd/state.h        |   9 ++++
 fs/nfsd/trace.h        |  49 +++++++++++++++++++
 fs/nfsd/xdr4.h         |   5 ++
 fs/nfsd/xdr4cb.h       |   6 +++
 6 files changed, 234 insertions(+), 13 deletions(-)

Comments

Chuck Lever III Nov. 17, 2022, 2:44 p.m. UTC | #1
> On Nov 16, 2022, at 10:44 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
> 
> This patch series adds:
> 
>    . refactor courtesy_client_reaper to a generic low memory
>      shrinker.
> 
>    . XDR encode and decode function for CB_RECALL_ANY op.
> 
>    . the delegation reaper that sends the advisory CB_RECALL_ANY 
>      to the clients to release unused delegations.
>      There is only one nfsd4_callback added for each nfs4_cleint.
>      Access to it must be serialized via the client flag
>      NFSD4_CLIENT_CB_RECALL_ANY.
> 
>    . Add CB_RECALL_ANY tracepoints.
> 
> v2:
>    . modify deleg_reaper to check and send CB_RECALL_ANY to client
>      only once per 5 secs.
> v3:
>    . modify nfsd4_cb_recall_any_release to use nn->client_lock to
>      protect cl_recall_any_busy and call put_client_renew_locked
>      to decrement cl_rpc_users.
> 
> v4:
>    . move changes in nfs4state.c from patch (1/2) to patch(2/2).
>    . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
>      to encode CB_RECALL_ANY arguments.
>    . add struct nfsd4_cb_recall_any with embedded nfs4_callback
>      and params for CB_RECALL_ANY op.
>    . replace cl_recall_any_busy boolean with client flag
>      NFSD4_CLIENT_CB_RECALL_ANY 
>    . add tracepoints for CB_RECALL_ANY
> 
> v5:
>    . refactor courtesy_client_reaper to a generic low memory
>      shrinker
>    . merge courtesy client shrinker and delegtion shrinker into
>      one.
>    . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
>      in nfsd/trace.h
>    . use __get_sockaddr to display server IP address in
>      tracepoints.
>    . modify encode_cb_recallany4args to replace sizeof with
>      ARRAY_SIZE.

Hi-

I'm going to apply this version of the series with some minor
changes. I'll reply to the individual patches where we can
discuss those.


> ---
> 
> Dai Ngo (4):
>     NFSD: refactoring courtesy_client_reaper to a generic low memory shrinker
>     NFSD: add support for sending CB_RECALL_ANY
>     NFSD: add delegation shrinker to react to low memory condition
>     NFSD: add CB_RECALL_ANY tracepoints
> 
> fs/nfsd/nfs4callback.c |  62 +++++++++++++++++++++++
> fs/nfsd/nfs4state.c    | 116 +++++++++++++++++++++++++++++++++++++++-----
> fs/nfsd/state.h        |   9 ++++
> fs/nfsd/trace.h        |  49 +++++++++++++++++++
> fs/nfsd/xdr4.h         |   5 ++
> fs/nfsd/xdr4cb.h       |   6 +++
> 6 files changed, 234 insertions(+), 13 deletions(-)

--
Chuck Lever
Dai Ngo Nov. 17, 2022, 4:53 p.m. UTC | #2
On 11/17/22 6:44 AM, Chuck Lever III wrote:
>
>> On Nov 16, 2022, at 10:44 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>
>> This patch series adds:
>>
>>     . refactor courtesy_client_reaper to a generic low memory
>>       shrinker.
>>
>>     . XDR encode and decode function for CB_RECALL_ANY op.
>>
>>     . the delegation reaper that sends the advisory CB_RECALL_ANY
>>       to the clients to release unused delegations.
>>       There is only one nfsd4_callback added for each nfs4_cleint.
>>       Access to it must be serialized via the client flag
>>       NFSD4_CLIENT_CB_RECALL_ANY.
>>
>>     . Add CB_RECALL_ANY tracepoints.
>>
>> v2:
>>     . modify deleg_reaper to check and send CB_RECALL_ANY to client
>>       only once per 5 secs.
>> v3:
>>     . modify nfsd4_cb_recall_any_release to use nn->client_lock to
>>       protect cl_recall_any_busy and call put_client_renew_locked
>>       to decrement cl_rpc_users.
>>
>> v4:
>>     . move changes in nfs4state.c from patch (1/2) to patch(2/2).
>>     . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
>>       to encode CB_RECALL_ANY arguments.
>>     . add struct nfsd4_cb_recall_any with embedded nfs4_callback
>>       and params for CB_RECALL_ANY op.
>>     . replace cl_recall_any_busy boolean with client flag
>>       NFSD4_CLIENT_CB_RECALL_ANY
>>     . add tracepoints for CB_RECALL_ANY
>>
>> v5:
>>     . refactor courtesy_client_reaper to a generic low memory
>>       shrinker
>>     . merge courtesy client shrinker and delegtion shrinker into
>>       one.
>>     . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
>>       in nfsd/trace.h
>>     . use __get_sockaddr to display server IP address in
>>       tracepoints.
>>     . modify encode_cb_recallany4args to replace sizeof with
>>       ARRAY_SIZE.
> Hi-
>
> I'm going to apply this version of the series with some minor
> changes. I'll reply to the individual patches where we can
> discuss those.

Thank you Chuck!

-Dai

>
>
>> ---
>>
>> Dai Ngo (4):
>>      NFSD: refactoring courtesy_client_reaper to a generic low memory shrinker
>>      NFSD: add support for sending CB_RECALL_ANY
>>      NFSD: add delegation shrinker to react to low memory condition
>>      NFSD: add CB_RECALL_ANY tracepoints
>>
>> fs/nfsd/nfs4callback.c |  62 +++++++++++++++++++++++
>> fs/nfsd/nfs4state.c    | 116 +++++++++++++++++++++++++++++++++++++++-----
>> fs/nfsd/state.h        |   9 ++++
>> fs/nfsd/trace.h        |  49 +++++++++++++++++++
>> fs/nfsd/xdr4.h         |   5 ++
>> fs/nfsd/xdr4cb.h       |   6 +++
>> 6 files changed, 234 insertions(+), 13 deletions(-)
> --
> Chuck Lever
>
>
>
Chuck Lever III Nov. 17, 2022, 5:04 p.m. UTC | #3
> On Nov 17, 2022, at 11:53 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
> 
> 
> On 11/17/22 6:44 AM, Chuck Lever III wrote:
>> 
>>> On Nov 16, 2022, at 10:44 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>> 
>>> This patch series adds:
>>> 
>>>    . refactor courtesy_client_reaper to a generic low memory
>>>      shrinker.
>>> 
>>>    . XDR encode and decode function for CB_RECALL_ANY op.
>>> 
>>>    . the delegation reaper that sends the advisory CB_RECALL_ANY
>>>      to the clients to release unused delegations.
>>>      There is only one nfsd4_callback added for each nfs4_cleint.
>>>      Access to it must be serialized via the client flag
>>>      NFSD4_CLIENT_CB_RECALL_ANY.
>>> 
>>>    . Add CB_RECALL_ANY tracepoints.
>>> 
>>> v2:
>>>    . modify deleg_reaper to check and send CB_RECALL_ANY to client
>>>      only once per 5 secs.
>>> v3:
>>>    . modify nfsd4_cb_recall_any_release to use nn->client_lock to
>>>      protect cl_recall_any_busy and call put_client_renew_locked
>>>      to decrement cl_rpc_users.
>>> 
>>> v4:
>>>    . move changes in nfs4state.c from patch (1/2) to patch(2/2).
>>>    . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
>>>      to encode CB_RECALL_ANY arguments.
>>>    . add struct nfsd4_cb_recall_any with embedded nfs4_callback
>>>      and params for CB_RECALL_ANY op.
>>>    . replace cl_recall_any_busy boolean with client flag
>>>      NFSD4_CLIENT_CB_RECALL_ANY
>>>    . add tracepoints for CB_RECALL_ANY
>>> 
>>> v5:
>>>    . refactor courtesy_client_reaper to a generic low memory
>>>      shrinker
>>>    . merge courtesy client shrinker and delegtion shrinker into
>>>      one.
>>>    . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
>>>      in nfsd/trace.h
>>>    . use __get_sockaddr to display server IP address in
>>>      tracepoints.
>>>    . modify encode_cb_recallany4args to replace sizeof with
>>>      ARRAY_SIZE.
>> Hi-
>> 
>> I'm going to apply this version of the series with some minor
>> changes. I'll reply to the individual patches where we can
>> discuss those.
> 
> Thank you Chuck!

Changes folded in and pushed to nfsd's for-next. For the trace
point patch, I've rebased your series on top of the patch that
relocates include/trace/events/nfs.h so that the new
show_rca_mask() macro can go in the common nfs.h instead of
the NFSD-specific fs/nfsd/trace.h.

Feel free to pull and test to make sure I didn't do anything
bone-headed.


--
Chuck Lever
Dai Ngo Nov. 17, 2022, 6:43 p.m. UTC | #4
On 11/17/22 9:04 AM, Chuck Lever III wrote:
>
>> On Nov 17, 2022, at 11:53 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>
>>
>> On 11/17/22 6:44 AM, Chuck Lever III wrote:
>>>> On Nov 16, 2022, at 10:44 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>>
>>>> This patch series adds:
>>>>
>>>>     . refactor courtesy_client_reaper to a generic low memory
>>>>       shrinker.
>>>>
>>>>     . XDR encode and decode function for CB_RECALL_ANY op.
>>>>
>>>>     . the delegation reaper that sends the advisory CB_RECALL_ANY
>>>>       to the clients to release unused delegations.
>>>>       There is only one nfsd4_callback added for each nfs4_cleint.
>>>>       Access to it must be serialized via the client flag
>>>>       NFSD4_CLIENT_CB_RECALL_ANY.
>>>>
>>>>     . Add CB_RECALL_ANY tracepoints.
>>>>
>>>> v2:
>>>>     . modify deleg_reaper to check and send CB_RECALL_ANY to client
>>>>       only once per 5 secs.
>>>> v3:
>>>>     . modify nfsd4_cb_recall_any_release to use nn->client_lock to
>>>>       protect cl_recall_any_busy and call put_client_renew_locked
>>>>       to decrement cl_rpc_users.
>>>>
>>>> v4:
>>>>     . move changes in nfs4state.c from patch (1/2) to patch(2/2).
>>>>     . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
>>>>       to encode CB_RECALL_ANY arguments.
>>>>     . add struct nfsd4_cb_recall_any with embedded nfs4_callback
>>>>       and params for CB_RECALL_ANY op.
>>>>     . replace cl_recall_any_busy boolean with client flag
>>>>       NFSD4_CLIENT_CB_RECALL_ANY
>>>>     . add tracepoints for CB_RECALL_ANY
>>>>
>>>> v5:
>>>>     . refactor courtesy_client_reaper to a generic low memory
>>>>       shrinker
>>>>     . merge courtesy client shrinker and delegtion shrinker into
>>>>       one.
>>>>     . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
>>>>       in nfsd/trace.h
>>>>     . use __get_sockaddr to display server IP address in
>>>>       tracepoints.
>>>>     . modify encode_cb_recallany4args to replace sizeof with
>>>>       ARRAY_SIZE.
>>> Hi-
>>>
>>> I'm going to apply this version of the series with some minor
>>> changes. I'll reply to the individual patches where we can
>>> discuss those.
>> Thank you Chuck!
> Changes folded in and pushed to nfsd's for-next. For the trace
> point patch, I've rebased your series on top of the patch that
> relocates include/trace/events/nfs.h so that the new
> show_rca_mask() macro can go in the common nfs.h instead of
> the NFSD-specific fs/nfsd/trace.h.
>
> Feel free to pull and test to make sure I didn't do anything
> bone-headed.

I removed the get_sockaddr temporarily to test show_rca_mask.
It works fine:

[root@nfsvmf24 ~]# trace-cmd report
trace-cmd: No such file or directory
   Error: expected type 4 but read 5
cpus=1
     kworker/u2:6-2297  [000]  1349.863391: nfsd_cb_recall_any:   client 63767ac9:adb1a3fb keep=0 bmval0=RDATA_DLG
     kworker/u2:0-8698  [000]  1349.869652: nfsd_cb_recall_any_done: client 63767ac9:adb1a3fb status=0

Thanks,
-Dai

>
>
> --
> Chuck Lever
>
>
>
Chuck Lever III Nov. 17, 2022, 6:44 p.m. UTC | #5
> On Nov 17, 2022, at 1:43 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
> 
> 
> On 11/17/22 9:04 AM, Chuck Lever III wrote:
>> 
>>> On Nov 17, 2022, at 11:53 AM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>> 
>>> 
>>> On 11/17/22 6:44 AM, Chuck Lever III wrote:
>>>>> On Nov 16, 2022, at 10:44 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>>> 
>>>>> This patch series adds:
>>>>> 
>>>>>    . refactor courtesy_client_reaper to a generic low memory
>>>>>      shrinker.
>>>>> 
>>>>>    . XDR encode and decode function for CB_RECALL_ANY op.
>>>>> 
>>>>>    . the delegation reaper that sends the advisory CB_RECALL_ANY
>>>>>      to the clients to release unused delegations.
>>>>>      There is only one nfsd4_callback added for each nfs4_cleint.
>>>>>      Access to it must be serialized via the client flag
>>>>>      NFSD4_CLIENT_CB_RECALL_ANY.
>>>>> 
>>>>>    . Add CB_RECALL_ANY tracepoints.
>>>>> 
>>>>> v2:
>>>>>    . modify deleg_reaper to check and send CB_RECALL_ANY to client
>>>>>      only once per 5 secs.
>>>>> v3:
>>>>>    . modify nfsd4_cb_recall_any_release to use nn->client_lock to
>>>>>      protect cl_recall_any_busy and call put_client_renew_locked
>>>>>      to decrement cl_rpc_users.
>>>>> 
>>>>> v4:
>>>>>    . move changes in nfs4state.c from patch (1/2) to patch(2/2).
>>>>>    . use xdr_stream_encode_u32 and xdr_stream_encode_uint32_array
>>>>>      to encode CB_RECALL_ANY arguments.
>>>>>    . add struct nfsd4_cb_recall_any with embedded nfs4_callback
>>>>>      and params for CB_RECALL_ANY op.
>>>>>    . replace cl_recall_any_busy boolean with client flag
>>>>>      NFSD4_CLIENT_CB_RECALL_ANY
>>>>>    . add tracepoints for CB_RECALL_ANY
>>>>> 
>>>>> v5:
>>>>>    . refactor courtesy_client_reaper to a generic low memory
>>>>>      shrinker
>>>>>    . merge courtesy client shrinker and delegtion shrinker into
>>>>>      one.
>>>>>    . reposition nfsd_cb_recall_any and nfsd_cb_recall_any_done
>>>>>      in nfsd/trace.h
>>>>>    . use __get_sockaddr to display server IP address in
>>>>>      tracepoints.
>>>>>    . modify encode_cb_recallany4args to replace sizeof with
>>>>>      ARRAY_SIZE.
>>>> Hi-
>>>> 
>>>> I'm going to apply this version of the series with some minor
>>>> changes. I'll reply to the individual patches where we can
>>>> discuss those.
>>> Thank you Chuck!
>> Changes folded in and pushed to nfsd's for-next. For the trace
>> point patch, I've rebased your series on top of the patch that
>> relocates include/trace/events/nfs.h so that the new
>> show_rca_mask() macro can go in the common nfs.h instead of
>> the NFSD-specific fs/nfsd/trace.h.
>> 
>> Feel free to pull and test to make sure I didn't do anything
>> bone-headed.
> 
> I removed the get_sockaddr temporarily to test show_rca_mask.
> It works fine:
> 
> [root@nfsvmf24 ~]# trace-cmd report
> trace-cmd: No such file or directory
>  Error: expected type 4 but read 5
> cpus=1
>    kworker/u2:6-2297  [000]  1349.863391: nfsd_cb_recall_any:   client 63767ac9:adb1a3fb keep=0 bmval0=RDATA_DLG
>    kworker/u2:0-8698  [000]  1349.869652: nfsd_cb_recall_any_done: client 63767ac9:adb1a3fb status=0

Perfect, thank you!


--
Chuck Lever