Page MenuHomeFreeBSD

factor out the code that checks to see if vnode_pager_setsize() can be called with the NFS node lock held
Needs ReviewPublic

Authored by rmacklem on Sep 27 2019, 1:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 5:11 AM
Unknown Object (File)
Oct 12 2023, 12:52 AM
Unknown Object (File)
Sep 30 2023, 7:06 PM
Unknown Object (File)
Sep 23 2023, 12:17 AM
Unknown Object (File)
Jul 3 2023, 2:31 AM
Unknown Object (File)
May 10 2023, 4:54 PM
Subscribers

Details

Reviewers
kib
Summary

This patch factors out the code snippet that checks to see if vnode_pager_setsize() can be called with
the NFS node locked (added by r352457) into a separate function called nfscl_checksetsize().
This is done so that this function can be used elsewhere in the NFS client code that calls vnode_pager_setsize()
without checking to see if it safe to do so with the NFS node locked.

The only semantic change is that nfscl_loadattrcache() only calls nfscl_checksetsize() for VREG files where
the size has changed. This was what I believe kib@ suggested in the review of D21762.

Test Plan

Has been tested with the connectathon test suite and a kernel build over NFS. Since the semantics
are now very close to what r352457 and r252528 provided, I do not expect problems with it.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 26781

Event Timeline

As an alternative to creating a function called nfscl_checksetsize(), what about adding a "can_sleep"
flag argument to vnode_pager_setsize().
If can_sleep is "false", then it would return EWOULDBLOCK for the "shrinking" case.
(If can_sleep is "true" it would retain current behaviour.) Yes, the function would no
longer be void, but would return (0) except for this case.

This would avoid any "raceyness" w.r.t. doing the check in nfscl_checksetsize() without holding
the VM_OBJECT_WLOCK() and would clarify what the NFS code is doing when it calls the
function with can_sleep == "false" while holding the NFS node mutex.

This patch would then simply call vnode_pager_setsize(vp, nsize, false) instead of nfscl_checksetsize().

So, what do you think?

This version of the patch replaces nfscl_checksetsize() with vnode_pager_setsize_nonblock().
The latter is a macro which just calls vnode_pager_setsize_mightsleep() with can_sleep == false.

vnode_pager_setsize() is replaced by vnode_pager_setsize_mightsleep() which takes an extra
Boolean argument "can_sleep". When this argument is "true", the behaviour remains the same,
however when it is "false", the function returns EWOULDBLOCK if it might have to sleep while
reducing the vm_object's size. It returns 0 otherwise.

Doing this avoids duplication of the code and any "raciness" caused by checking that the object
is being reduced in size without the VM_OBJECT_WLOCK() held.
However, I think the main advantage is that it documents when vnode_pager_setsize gets called with
a mutex lock (or similar) is held and handles this safely.