Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <f3910fdd-d107-1f6c-8cfc-1b7f429f8024@redhat.com>
Date:   Thu, 17 Mar 2022 11:37:31 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [RFC v2 PATCH 7/7] readahead: documentation
Content-Language: en-US
To:     Thiago Becker <tbecker@redhat.com>, linux-nfs@vger.kernel.org
Cc:     trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
        kolga@netapp.com
References: <20220311190617.3294919-1-tbecker@redhat.com>
 <20220311190617.3294919-8-tbecker@redhat.com>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <20220311190617.3294919-8-tbecker@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/11/22 2:06 PM, Thiago Becker wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
> Signed-off-by: Thiago Becker <tbecker@redhat.com>
> ---
>   tools/nfs-readahead-udev/Makefile.am          |  2 +
>   .../nfs-readahead-udev/nfs-readahead-udev.man | 47 +++++++++++++++++++
>   tools/nfs-readahead-udev/readahead.conf       | 14 ++++++
>   3 files changed, 63 insertions(+)
>   create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man
> 
> diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
> index 010350aa..eaa9b90e 100644
> --- a/tools/nfs-readahead-udev/Makefile.am
> +++ b/tools/nfs-readahead-udev/Makefile.am
> @@ -10,6 +10,8 @@ udev_rules_DATA = 99-nfs_bdi.rules
>   ra_confdir = $(sysconfdir)
>   ra_conf_DATA = readahead.conf
>   
> +man5_MANS = nfs-readahead-udev.man
> +
>   99-nfs_bdi.rules: 99-nfs_bdi.rules.in $(builddefs)
>   	$(SED) "s|_libexecdir_|@libexecdir@|g" 99-nfs_bdi.rules.in > $@
>   
> diff --git a/tools/nfs-readahead-udev/nfs-readahead-udev.man b/tools/nfs-readahead-udev/nfs-readahead-udev.man
> new file mode 100644
> index 00000000..2477d5b3
> --- /dev/null
> +++ b/tools/nfs-readahead-udev/nfs-readahead-udev.man
> @@ -0,0 +1,47 @@
> +.\" Manpage for nfs-readahead-udev.
> +.nh
> +.ad l
> +.TH man 5 "08 Mar 2022" "1.0" "nfs-readahead-udev man page"
> +.SH NAME
> +
> +nfs-readahead-udev \- Find the readahead for a given NFS mount
> +
> +.SH SYNOPSIS
> +
> +nfs-readahead-udev <device>
> +
> +.SH DESCRIPTION
> +
> +\fInfs-readahead-udev\fR is a tool intended to be used with udev to set the \fIread_ahead_kb\fR parameter of NFS mounts, according to the configuration file (see \fICONFIGURATION\fR). \fIdevice\fR is the device number for the NFS backing device as provided by the kernel.
> +
> +.SH CONFIGURATION
> +
> +The configuration file (\fI/etc/readahead.conf\fR) contains the readahead configuration, and is formatted as follows.
> +
> +<LINES> ::= <LINES> <LINE> | <LINE>
> +
> +<LINE> ::= <TOKENS> <ENDL>
> +
> +<TOKENS> ::= <TOKENS> <TOKEN> | <TOKEN>
> +
> +<TOKEN> ::= default | <PAIR>
> +
> +<PAIR> ::= mountpoint = <mountpoint> | fstype = <nfs|nfs4> | readahead = <readahead>
> +
> +\fImountpoint\fR is the path in the system where the file system is mounted.
> +
> +\fIreadahead\fR is an integer to readahead.
> +
> +\fIfstype\fR is either \fInfs\fR or \fInfs4\fR.
> +
> +.SH SEE ALSO
> +
> +mount.nfs(8), nfs(5), udev(7), bcc-readahead(8)
> +
> +.SH BUGS
> +
> +No known bugs.
> +
> +.SH AUTHOR
I think it might make sense to added some examples
on how the command will be used.

> +
> +Thiago Rafael Becker <trbecker@gmail.com>
> diff --git a/tools/nfs-readahead-udev/readahead.conf b/tools/nfs-readahead-udev/readahead.conf
> index 988b30c7..bce830f1 100644
> --- a/tools/nfs-readahead-udev/readahead.conf
> +++ b/tools/nfs-readahead-udev/readahead.conf
> @@ -1 +1,15 @@
> +# nfs-readahead-udev configuration file.
> +#
> +# This file configures the readahead for nfs mounts when those are anounced by the kernel.
> +# The file is composed on lines that can contain either the default configuration (applied to
> +# any nfs mount that does not match any of the other lines) or a combination of
> +#   mountpoint=<mountpoint> where mountpoint is the mount point for the file system
> +#   fstype=<nfs|nfs4> specifies that this configuration should only apply to a specific nfs
> +#     version.
> +# Every line must contain a readahead option, with the expected readahead value.
>   default				readahead=128
> +
> +# mountpoint=/mnt		readahead=4194304
> +# fstype=nfs			readahead=4194304
> +# fstype=nfs4			readahead=4194304
> +# mountpoint=/mnt	fstype=nfs4	readahead=4194304
Would it make sense to try added these to nfs.conf?

I must admin I'm a bit impressed with your lex and
yacc routines in patch 5, I have not seen those
in a while.. but that does add more dependencies
to nfs-utils and as well as yet another config file
to manage.

steved.