On Mon, Apr 27, 2009 at 12:06:18PM -0400, Chuck Lever wrote:
> On Apr 25, 2009, at 6:03 PM, J. Bruce Fields wrote:
>> Pfft, did it again.
>>
>> --b.
>>
>> On Sat, Apr 25, 2009 at 05:57:45PM -0400, bfields wrote:
>>> On Wed, Apr 01, 2009 at 07:28:03AM +1100, Greg Banks wrote:
>>>> Add two control files to /proc/fs/nfsd:
>>>>
>>>> * "stats_enabled" can be used to disable or enable the gathering
>>>> of per-client and per-export statistics in the server.
>>>>
>>>> * "stats_prune_period" can be used to set the period at
>>>> which the pruning timer runs, in seconds. Unused stats
>>>> entries will survive at most twice that time.
>>>>
>>>> Signed-off-by: Greg Banks <gnb@xxxxxxx>
>>>> ---
>>>>
>>>> fs/nfsd/nfsctl.c | 99 ++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 99 insertions(+)
>>>>
>>>> Index: bfields/fs/nfsd/nfsctl.c
>>>> ===================================================================
>>>> --- bfields.orig/fs/nfsd/nfsctl.c
>>>> +++ bfields/fs/nfsd/nfsctl.c
>>>> @@ -64,6 +64,8 @@ enum {
>>>> NFSD_Versions,
>>>> NFSD_Ports,
>>>> NFSD_MaxBlkSize,
>>>> + NFSD_Stats_Enabled,
>>>> + NFSD_Stats_Prune_Period,
>>>> /*
>>>> * The below MUST come last. Otherwise we leave a hole in
>>>> nfsd_files[]
>>>> * with !CONFIG_NFSD_V4 and simple_fill_super() goes oops
>>>> @@ -92,6 +94,8 @@ static ssize_t write_pool_threads(struct
>>>> static ssize_t write_versions(struct file *file, char *buf, size_t
>>>> size);
>>>> static ssize_t write_ports(struct file *file, char *buf, size_t
>>>> size);
>>>> static ssize_t write_maxblksize(struct file *file, char *buf,
>>>> size_t size);
>>>> +static ssize_t write_stats_enabled(struct file *file, char *buf,
>>>> size_t size);
>>>> +static ssize_t write_stats_prune_period(struct file *file, char
>>>> *buf, size_t size);
>>>> #ifdef CONFIG_NFSD_V4
>>>> static ssize_t write_leasetime(struct file *file, char *buf,
>>>> size_t size);
>>>> static ssize_t write_recoverydir(struct file *file, char *buf,
>>>> size_t size);
>>>> @@ -113,6 +117,8 @@ static ssize_t (*write_op[])(struct file
>>>> [NFSD_Versions] = write_versions,
>>>> [NFSD_Ports] = write_ports,
>>>> [NFSD_MaxBlkSize] = write_maxblksize,
>>>> + [NFSD_Stats_Enabled] = write_stats_enabled,
>>>> + [NFSD_Stats_Prune_Period] = write_stats_prune_period,
>>>> #ifdef CONFIG_NFSD_V4
>>>> [NFSD_Leasetime] = write_leasetime,
>>>> [NFSD_RecoveryDir] = write_recoverydir,
>>>> @@ -1121,6 +1127,97 @@ static ssize_t write_maxblksize(struct f
>>>> return sprintf(buf, "%d\n", nfsd_max_blksize);
>>>> }
>>>>
>>>> +extern int nfsd_stats_enabled;
>>>> +
>>>> +/**
>>>> + * write_stats_enabled - Set or report whether per-client/
>>>> + * per-export stats are enabled.
>>>> + *
>>>> + * Input:
>>>> + * buf: ignored
>>>> + * size: zero
>>>> + *
>>>> + * OR
>>>> + *
>>>> + * Input:
>>>> + * buf: C string containing an unsigned
>>>> + * integer value representing the new value
>>>> + * size: non-zero length of C string in @buf
>>>> + * Output:
>>>> + * On success: passed-in buffer filled with '\n'-terminated C
>>>> string
>>>> + * containing numeric value of the current setting
>>>> + * return code is the size in bytes of the string
>>>> + * On error: return code is zero or a negative errno value
>>>> + */
>>>> +static ssize_t write_stats_enabled(struct file *file, char *buf,
>>>> size_t size)
>>>> +{
>>>> + char *mesg = buf;
>>>> + if (size > 0) {
>>>> + int enabled;
>>>> + int rv = get_int(&mesg, &enabled);
>>>> + if (rv)
>>>> + return rv;
>>>> + /* check `enabled' against allowed range */
>>>> + if (enabled < 0 || enabled > 1)
>>>> + return -EINVAL;
>>>> + /*
>>>> + * We can change the enabled flag at any time without
>>>> + * locking. All it controls is whether stats are
>>>> + * gathered for new incoming NFS calls. Old gathered
>>>> + * stats still sit around in the hash tables until
>>>> + * naturally pruned.
>>>> + */
>>>> + nfsd_stats_enabled = enabled;
>>>> + }
>>>> + return sprintf(buf, "%d\n", nfsd_stats_enabled);
>>>> +}
>>>> +
>>>> +extern int nfsd_stats_prune_period;
>>>> +
>>>> +/**
>>>> + * write_stats_prune_period - Set or report the period for pruning
>>>> + * old per-client/per-export stats entries,
>>>> + * in seconds.
>>>> + *
>>>> + * Input:
>>>> + * buf: ignored
>>>> + * size: zero
>>>> + *
>>>> + * OR
>>>> + *
>>>> + * Input:
>>>> + * buf: C string containing an unsigned
>>>> + * integer value representing the new value
>>>> + * size: non-zero length of C string in @buf
>>>> + * Output:
>>>> + * On success: passed-in buffer filled with '\n'-terminated C
>>>> string
>>>> + * containing numeric value of the current setting
>>>> + * return code is the size in bytes of the string
>>>> + * On error: return code is zero or a negative errno value
>>>> + */
>>>
>>> Just an idle remark, don't worry about this for now, but: we might
>>> want
>>> to rein in this write_*() comment format a little some day. A lot of
>>> the content seems duplicated.
>
> I disagree.
How? The below seems to be an arguing against *removing* the comments,
or removing information from them, neither of which I'd be in favor of.
--b.
> The present nfsctl user space API is entirely ad hoc.
>
> Although sometimes the behavior is the same, each function/file can
> behave slightly differently than the others. We have to be very
> specific about this here because the comments serve as both "user"
> documentation and as code/API specification.
>
> Because this code wasn't adequately documented, features have been added
> over time without a close examination of the operation of other parts of
> this code.
>
> If you really want to simplify the comments, we should consider
> simplifying the API first, imo.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
