On Tue, 2022-12-13 at 19:00 +0000, Chuck Lever III wrote: > > > On Dec 13, 2022, at 1:08 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > If v4 READDIR operation hits a mountpoint and gets back an error, > > then it will include that entry in the reply and set RDATTR_ERROR for it > > to the error. > > > > That's fine for "normal" exported filesystems, but on the v4root, we > > need to be more careful to only expose the existence of dentries that > > lead to exports. > > > > If the mountd upcall times out while checking to see whether a > > mountpoint on the v4root is exported, then we have no recourse other > > than to fail the whole operation. > > Thank you for chasing this down! > > Failing the whole READDIR when mountd times out might be a bad idea. > If the mountd upcall times out every time, the client can't make > any progress and will continue to emit the failing READDIR request. > > Would it be better to skip the unresolvable entry instead and let > the READDIR succeed without that entry? > Mounting doesn't usually require working READDIR. In that situation, a readdir() might hang (until the client kills), but a lookup of other dentries that aren't perpetually stalled should be ok in this situation. If mountd is that hosed then I think it's unlikely that any progress will be possible anyway. > > > Cc: Steve Dickson <steved@xxxxxxxxxx> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216777 > > Reported-by: JianHong Yin <yin-jianhong@xxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4xdr.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index 2b4ae858c89b..984528ce8d68 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -3588,6 +3588,7 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, > > struct readdir_cd *ccd = ccdv; > > struct nfsd4_readdir *cd = container_of(ccd, struct nfsd4_readdir, common); > > struct xdr_stream *xdr = cd->xdr; > > + struct svc_export *exp = cd->rd_fhp->fh_export; > > int start_offset = xdr->buf->len; > > int cookie_offset; > > u32 name_and_cookie; > > @@ -3629,6 +3630,17 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, > > case nfserr_noent: > > xdr_truncate_encode(xdr, start_offset); > > goto skip_entry; > > + case nfserr_jukebox: > > + /* > > + * The pseudoroot should only display dentries that lead to > > + * exports. If we get EJUKEBOX here, then we can't tell whether > > + * this entry should be included. Just fail the whole READDIR > > + * with NFS4ERR_DELAY in that case, and hope that the situation > > + * will resolve itself by the client's next attempt. > > + */ > > + if (exp->ex_flags & NFSEXP_V4ROOT) > > + goto fail; > > + fallthrough; > > default: > > /* > > * If the client requested the RDATTR_ERROR attribute, > > -- > > 2.38.1 > > > > -- > Chuck Lever > > > -- Jeff Layton <jlayton@xxxxxxxxxx>
