On Mon, Apr 21, 2025 at 03:37:55PM -0700, Rick Macklem wrote: > Hi, > > I just spotted something in the NFS server that seems like > it is a bug, but I thought I'd check. > (Note that I have never seen this cause a problem, but I > think it might if a server file system is being forced > dismounted while the NFS server is accessing it.) > > What I spotted was a few places where: > cnp->cn_lkflags = LK_SHARED | LK_RETRY; > ... > error = VOP_LOOKUP(..); > I don't think LK_RETRY should be set here. > For example vget_finish() uses the flags argument > for a "error = vn_lock(..flags);", which would retry > instead of returning an error when the vnode is VI_DOOMED. > > So, is this a bug that needs to be fixed?
I think LK_RETRY is not appropriate there, and ought to be removed. But it should not be critical by itself. The doomed vnode might cause some further VOP calls to fail, and NFS server must be prepared to that, correctly handling the error. I looked at the output of grep 'cn_lkflags.*LK_RETRY', and it seems that the error handling is proper, although the readdirplus code is somewhat too much to make the claim. As a curiousity, there is one more place with this pattern in zfs.
