add non-blocking variants of rangelock_rlock() and rangelock_wlock()

[rmacklem]

Summary

D20584 requires a way to acquire rangelocks on two vnodes concurrently without the risk of deadlock.
To do this, an non-blocking variant of at least one of rangelock_rlock() and rangelock_wlock() is needed.
This patch adds both rangelock_rlock_nonblock() and rangelock_wlock_nonblock() to do this and also
vn_rangelock_rlock_nonblock() and vn_rangelock_wlock_nonblock() using the above.

Not sure if rangelock_rlock()/rangelock_wlock() is in any man page?

Test Plan

By putting a long tsleep() in the copy_file_range() patch after the thread acquires the first lock for the first call,
a second copy_file_range() call will hit the conflict and tsleep() after acquiring the other rangelock, then a third
concurrent call to copy_file_range() should have the non-blocking call return NULL.

Trivial tests without the conflicts work ok.

[asomers]

sys/vnode.h

726 ↗

(On Diff #58643)

As far as names go, I suggest the "try" idiom, as in vn_rangelock_try_rlock

[rmacklem]

This variant of the patch has the "_nonblock" suffixes replaced by "_trylock" as suggested by asomers@.
A quick grep of src/sys/sys shows that "nonblock" is used for flags, but "_trylock" is used as a suffix
for non-blocking lock functions.

No logic change from the previous version.

[kib]

kern/kern_rangelock.c

253 ↗

(On Diff #58648)

You should assert that the entry is indeed last, i..e. that its next is NULL.

256 ↗

(On Diff #58648)

I do not think that unlock_locked() is needed. Any non-granted entry can be removed from the list directly, and its removal cannot affect the list of currently granted range locks because it was not granted.

277 ↗

(On Diff #58648)

I disagree with the addition of non-sleepable flag. This would make trylock fail due to issue not related to the range lock state.

I do not this that a right semantic for rangelock try is to not ever sleep, it should be only 'do not sleep waiting for granting the rangelock'.

[rmacklem]

This variant allows a sleep to be done for allocation of a new list entry. It only returns NULL
if it would need to sleep waiting to be able to lock the range.

It also now has an extra argument for rangelock_unlock_locked(), which tells it not to call
rangelock_calc_block() when used to remove the new entry that rangelock_enqueue() added
if trylock != 0 and there is a conflict.
This was what kib@ suggested, except I didn't want to crib the rest of the code in
rangelock_unlock_locked() except the rangelock_calc_block() call.
(It might be better to rename rangelock_unlock_locked() to rangelock_remove_locked(), since
the lock hasn't yet been acquired for this case?)

[rmacklem]

Added replies to inline comments.

kern/kern_rangelock.c

256 ↗

(On Diff #58648)

I agree that the rangelock_calc_block() call in rangelock_unlock_locked() is a no op
in this case. However, I didn't want to duplicate the rest of the code that removes
an entry from the list, so I added an argument to rangelock_unlock_locked() to
make calling rangelock_calc_block() optional.

277 ↗

(On Diff #58648)

Yes. I was thinking "nonblock" should mean doesn't sleep, but "trylock" means
don't sleep for the lock.

[kib]

Feel free to do the rename in the preliminary commit.

kern/kern_rangelock.c

144 ↗

(On Diff #58680)

Please use bool and true/false.

204 ↗

(On Diff #58680)

Same, please use bool.

[rmacklem]

int arguments changed to C99 bool as requested by kib@.

[rmacklem]

sys/vnode.h

726 ↗

(On Diff #58643)

I added _trylock suffix, so the basic function name is recognized as the same as
the one that waits for the lock to be available.

[kib]

I believe that the code is fine from the algorithmic PoV, but I suggest some minor renaming and rearrangement.

kern/kern_rangelock.c

259 ↗

(On Diff #58684)

I think that all code inside if (trylock) {... from start up to the rangelock_unlock_locked() call (except the call itself) would be more naturally placed in the rangelock_unlock_locked() itself. In _unlock_locked(), you would call rangelock_calc_block() if trylock is true, and do reset of rl_currdep as needed, otherwise.

276 ↗

(On Diff #58684)

I suggest rangelock_tryrlock() name.

291 ↗

(On Diff #58684)

rangelock_trywlock()

[rmacklem]

This version of the patch incorporates kib@'s suggestions.