Re: [PATCH RFC v15 11/11] NFSD: Show state of courtesy clients in client info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Mar 9, 2022, at 10:09 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote:
> 
> 
> On 3/9/22 12:51 PM, dai.ngo@xxxxxxxxxx wrote:
>> 
>> On 3/9/22 12:14 PM, Chuck Lever III wrote:
>>> 
>>>> On Mar 4, 2022, at 7:37 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote:
>>>> 
>>>> Update client_info_show to show state of courtesy client
>>>> and time since last renew.
>>>> 
>>>> Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
>>>> ---
>>>> fs/nfsd/nfs4state.c | 9 ++++++++-
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>> index bced09014e6b..ed14e0b54537 100644
>>>> --- a/fs/nfsd/nfs4state.c
>>>> +++ b/fs/nfsd/nfs4state.c
>>>> @@ -2439,7 +2439,8 @@ static int client_info_show(struct seq_file *m, void *v)
>>>> {
>>>>     struct inode *inode = m->private;
>>>>     struct nfs4_client *clp;
>>>> -    u64 clid;
>>>> +    u64 clid, hrs;
>>>> +    u32 mins, secs;
>>>> 
>>>>     clp = get_nfsdfs_clp(inode);
>>>>     if (!clp)
>>>> @@ -2451,6 +2452,12 @@ static int client_info_show(struct seq_file *m, void *v)
>>>>         seq_puts(m, "status: confirmed\n");
>>>>     else
>>>>         seq_puts(m, "status: unconfirmed\n");
>>>> +    seq_printf(m, "courtesy client: %s\n",
>>>> +        test_bit(NFSD4_CLIENT_COURTESY, &clp->cl_flags) ? "yes" : "no");
>>> I'm wondering if it would be more economical to combine this
>>> output with the status output just before it so we have only
>>> one of:
>>> 
>>>     seq_puts(m, "status: unconfirmed\n");
>>> 
>>>     seq_puts(m, "status: confirmed\n");
>>> 
>>> or
>>> 
>>>     seq_puts(m, "status: courtesy\n");
>> 
>> make sense, will fix.
> 
> On second thought, I think it's safer to keep this the same
> since there might be scripts out there that depend on it.

The "status:" information was added just a year ago by
472d155a0631 ("nfsd: report client confirmation status
in "info" file"). I don't see any concern in recent commit
history about making the information in this file
machine-readable or backwards-compatible.

Therefore IMO it should be safe and acceptable to use:

  status: confirmed|unconfirmed|courtesy


> -Dai
> 
>> 
>>> 
>>> 
>>>> +    hrs = div_u64_rem(ktime_get_boottime_seconds() - clp->cl_time,
>>>> +                3600, &secs);
>>>> +    mins = div_u64_rem((u64)secs, 60, &secs);
>>>> +    seq_printf(m, "time since last renew: %02ld:%02d:%02d\n", hrs, mins, secs);
>>> Thanks, this seems more friendly than what was here before.
>>> 
>>> However if we replace the fixed courtesy timeout with a
>>> shrinker, I bet some courtesy clients might lie about for
>>> many more that 99 hours. Perhaps the left-most format
>>> specifier could be just "%lu" and the rest could be "%02u".
>>> 
>>> (ie, also turn the "d" into "u" to prevent ever displaying
>>> a negative number of time units).
>> 
>> will fix.
>> 
>> I will wait for your review of the rest of the patches before
>> I submit v16.
>> 
>> Thanks,
>> -Dai
>> 
>>> 
>>> 
>>>>     seq_printf(m, "name: ");
>>>>     seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
>>>>     seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
>>>> -- 
>>>> 2.9.5
>>>> 
>>> -- 
>>> Chuck Lever
>>> 
>>> 
>>> 

--
Chuck Lever







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux