Page MenuHomeFreeBSD

nfsclient: access v_mount only after the vnode is locked
ClosedPublic

Authored by kib on Sep 26 2022, 6:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 21, 10:16 AM
Unknown Object (File)
Oct 5 2024, 11:02 AM
Unknown Object (File)
Sep 30 2024, 1:55 PM
Unknown Object (File)
Sep 24 2024, 3:45 AM
Unknown Object (File)
Sep 17 2024, 11:09 AM
Unknown Object (File)
Sep 16 2024, 5:40 PM
Unknown Object (File)
Sep 13 2024, 5:33 AM
Unknown Object (File)
Sep 12 2024, 11:28 AM
Subscribers

Details

Summary

and we checked that it is not reclaimed.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Sep 26 2022, 6:13 PM
This revision is now accepted and ready to land.Sep 27 2022, 12:52 AM

I do have a question?

How did v_data get referenced in the unpatched code?
Do you mean the NFS specific structure on the mount point?

I do have a question?

How did v_data get referenced in the unpatched code?
Do you mean the NFS specific structure on the mount point?

Sorry, it is v_mount not v_data. I fixed the commit message, thanks.

kib retitled this revision from nfsclient: access v_data only after the vnode is locked to nfsclient: access v_mount only after the vnode is locked.Sep 27 2022, 11:42 AM
sys/fs/nfsclient/nfs_clvnops.c
3925

If invp == outvp then the loop above will never terminate, and there is a double-unlock below. Probably it's better to handle that as a separate case at the beginning of the function, since we do not require the mountpoint mutex to check invp == outvp.

Avoid invp == outvp case for locking vnodes.
Reorg and clean code, avoid using ap->a_XXX for locals.

This revision now requires review to proceed.Sep 27 2022, 1:39 PM

Looks ok to me. It seems rather wasteful that we have to uselessly acquire three mutexes each time VOP_COPY_FILE_RANGE is called on a NFSv3 mount. Was it ever considered to have separate vnode ops tables for different NFS major versions? I assume that the version is constant for the lifetime of the mount.

This revision is now accepted and ready to land.Sep 27 2022, 1:51 PM

Looks fine to me. Good catch w.r.t. invp == outvp.

During development, NFSv4 compounds were based on NFSv3
RPCs, so they share a lot of code. It would not have made sense
to have a separate VOP table,

Now that NFSv4.2 has an assortment of operations unlike NFSv3,
it might make sense, for your example and others.