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: <f6a1e306-211a-25d0-50c5-63c631ca1e47@redhat.com>
Date:   Tue, 15 Mar 2022 07:54:46 -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 0/7] Introduce nfs-readahead-udev
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>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <20220311190617.3294919-1-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:
> Recent changes in the linux kernel caused NFS readahead to default to
> 128 from the previous default of 15 * rsize. This causes performance
> penalties to some read-heavy workloads, which can be fixed by
> tuning the readahead for that given mount.
> 
> Specifically, the read troughput on a sec=krb5p mount drops by 50-75%
> when comparing the default readahead with a readahead of 15360.
> 
> Previous discussions:
> https://lore.kernel.org/linux-nfs/20210803130717.2890565-1-trbecker@gmail.com/
> I attempted to add a non-kernel option to mount.nfs, and it was
> rejected.
> 
> https://lore.kernel.org/linux-nfs/20210811171402.947156-1-trbecker@gmail.com/
> Attempted to add a mount option to the kernel, rejected as well.
> 
> I had started a separate tool to set the readahead of BDIs, but the
> scope is specifically for NFS, so I would like to get the community
> feeling for having this in nfs-utils.
> 
> This patch series introduces nfs-readahead-udev, a utility to
> automatically set NFS readahead when NFS is mounted. The utility is
> triggered by udev when a new BDI is added, returns to udev the value of
> the readahead that should be used.
> 
> The tool currently supports setting read ahead per mountpoint, nfs major
> version, or by a global default value.
> 
> Thiago Becker (7):
>    Create nfs-readahead-udev
>    readahead: configure udev
>    readahead: create logging facility
>    readahead: only set readahead for nfs devices.
>    readahead: create the configuration file
>    readahead: add mountpoint and fstype options
>    readahead: documentation
> 
>   .gitignore                                    |   6 +
>   configure.ac                                  |   4 +
>   tools/Makefile.am                             |   2 +-
>   tools/nfs-readahead-udev/99-nfs_bdi.rules.in  |   1 +
>   tools/nfs-readahead-udev/Makefile.am          |  26 +++
>   tools/nfs-readahead-udev/config_parser.c      |  25 +++
>   tools/nfs-readahead-udev/config_parser.h      |  14 ++
>   tools/nfs-readahead-udev/list.h               |  48 ++++
>   tools/nfs-readahead-udev/log.h                |  16 ++
>   tools/nfs-readahead-udev/main.c               | 211 ++++++++++++++++++
>   .../nfs-readahead-udev/nfs-readahead-udev.man |  47 ++++
>   tools/nfs-readahead-udev/parser.y             |  85 +++++++
>   tools/nfs-readahead-udev/readahead.conf       |  15 ++
>   tools/nfs-readahead-udev/scanner.l            |  19 ++
>   tools/nfs-readahead-udev/syslog.c             |  47 ++++
>   15 files changed, 565 insertions(+), 1 deletion(-)
>   create mode 100644 tools/nfs-readahead-udev/99-nfs_bdi.rules.in
>   create mode 100644 tools/nfs-readahead-udev/Makefile.am
>   create mode 100644 tools/nfs-readahead-udev/config_parser.c
>   create mode 100644 tools/nfs-readahead-udev/config_parser.h
>   create mode 100644 tools/nfs-readahead-udev/list.h
>   create mode 100644 tools/nfs-readahead-udev/log.h
>   create mode 100644 tools/nfs-readahead-udev/main.c
>   create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man
>   create mode 100644 tools/nfs-readahead-udev/parser.y
>   create mode 100644 tools/nfs-readahead-udev/readahead.conf
>   create mode 100644 tools/nfs-readahead-udev/scanner.l
>   create mode 100644 tools/nfs-readahead-udev/syslog.c
> 
My apologies for waiting late on this... I was on PTO.

I really don't like the name of the is command. It
does not follow any of the naming conventions we used in the
past... Can you please rename the command to nfsrahead.

tia,

steved.