Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     "Scott Mayhew" <smayhew@redhat.com>
Cc:     trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
        linux-nfs@vger.kernel.org
Subject: Re: [PATCH] NFS: Adjust fs_context error logging
Date:   Tue, 05 Jan 2021 09:40:17 -0500
Message-ID: <AC256A5D-88B8-4B96-8BE4-7BE8B7124027@redhat.com>
In-Reply-To: <20210105135432.1605419-1-smayhew@redhat.com>
References: <20210105135432.1605419-1-smayhew@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On 5 Jan 2021, at 8:54, Scott Mayhew wrote:

> Several existing dprink()/dfprintk() calls were converted to use the new
> mount API logging macros by commit ce8866f0913f ("NFS: Attach
> supplementary error information to fs_context").  If the fs_context was
> not created using fsopen() then it will not have had a log buffer
> allocated for it, and the new mount API logging macros will wind up
> calling printk().
>
> This can result in syslog messages being logged where previously there
> were none... most notably "NFS4: Couldn't follow remote path", which can
> happen if the client is auto-negotiating a protocol version with an NFS
> server that doesn't support the higher v4.x versions.
>
> Convert the nfs_errorf(), nfs_invalf(), and nfs_warnf() macros to check
> for the existence of the fs_context's log buffer and call dprintk() if
> it doesn't exist.  Add nfs_ferrorf(), nfs_finvalf(), and nfs_warnf(),
> which do the same thing but take an NFS debug flag as an argument and
> call dfprintk().  Finally, modify the "NFS4: Couldn't follow remote
> path" message to use nfs_ferrorf().
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=207385
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>

I hope someday we can convert all the old debugging to tracepoints.  I know
you considered just removing the debug lines or converting these to a
tracepoint and decided to fix what we have for now.  It does make for a
better stable fix.

Reviewed-by: Benjamin Coddington <bcodding@redhat.com>