Re: [PATCH rfc] nfs: propagate readlink errors in nfs_symlink_filler

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2024-05-30 at 21:08 +0300, Sagi Grimberg wrote:
> 
> 
> On 23/05/2024 0:19, Sagi Grimberg wrote:
> > 
> > > > So what do you suggest we do here? IMO at a minimum NFS should
> > > > retry
> > > > once similar
> > > > to nfs4_file_open (it would probably address 99.9% of the use-
> > > > cases
> > > > where symlinks are
> > > > not overwritten in a high enough frequency for the client to
> > > > see 2
> > > > consecutive stale readlink
> > > > rpc rplies).
> > > > 
> > > > I can send a patch paired with a vfs ESTALE conversion patch?
> > > > alternatively retry locally in NFS...
> > > > I would like to understand your position here.
> > > > 
> > > Looking more closely at nfs_get_link(), it is obvious that it can
> > > already return ESTALE (thanks to the call to
> > > nfs_revalidate_mapping())
> > > and looking at do_readlinkat(), it has already been plumbed
> > > through
> > > with a call to retry_estale().
> > > 
> > > So I think we can take your patch as is, since it doesn't add any
> > > error
> > > cases that callers of readlink() don't have to handle already.
> > 
> > Sounds good.
> > 
> > > 
> > > We might still want to think about cleaning up the output of the
> > > VFS in
> > > all these cases, so that we don't return ESTALE when it isn't
> > > allowed
> > > by POSIX, but that would be a separate task.
> > > 
> > 
> > Yes, I can follow up on that later...
> > 
> 
> Hey Trond,
> is there anything else you are expecting to see before this is taken
> to 
> your tree?
> 

It's already queued in my testing branch:
https://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=shortlog;h=refs/heads/testing
I'll probably push that into the linux-next branch over the weekend.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com