Page MenuHomeFreeBSD

vn_lock_pair(): only assert on lkflags1/lkflags2 vp1/vp2 is non-NULL
ClosedPublic

Authored by jah on Feb 23 2024, 5:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 26, 3:38 AM
Unknown Object (File)
Tue, Apr 23, 7:02 AM
Unknown Object (File)
Apr 14 2024, 5:41 PM
Unknown Object (File)
Apr 13 2024, 1:27 AM
Unknown Object (File)
Apr 13 2024, 1:26 AM
Unknown Object (File)
Apr 13 2024, 1:26 AM
Unknown Object (File)
Apr 8 2024, 10:39 PM
Unknown Object (File)
Mar 10 2024, 3:04 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 56189
Build 53077: arc lint + arc unit

Event Timeline

jah requested review of this revision.Feb 23 2024, 5:50 PM

May I ask why? This allows to pass any flags for null vnodes.

In D44046#1004867, @kib wrote:

May I ask why? This allows to pass any flags for null vnodes.

I have some forthcoming unionfs work that (at least in its current implementation) uses vn_lock_pair() in a helper function that takes the locking flags as parameters, and in which the second vnode may be NULL. In general it seems odd that, even when a vnode is NULL, the caller still needs to pass some contrived lock flags for it to appease the assert.

Specifically, 0, which would seem like a sensible default if there is no corresponding vnode to lock, will not work due to the SHARED^EXCLUSIVE assert.

So might be just allow zero flags if corresponding vp is NULL?

In D44046#1004890, @kib wrote:

So might be just allow zero flags if corresponding vp is NULL?

Sure, we could do that, but I'm curious: is there some reason why we should care what the lockflags are if there is no vnode to lock? What I have here seems more straightforward than making specific allowances for NULL vnodes.

In D44046#1004891, @jah wrote:
In D44046#1004890, @kib wrote:

So might be just allow zero flags if corresponding vp is NULL?

Sure, we could do that, but I'm curious: is there some reason why we should care what the lockflags are if there is no vnode to lock? What I have here seems more straightforward than making specific allowances for NULL vnodes.

I am about the reliable checking for the API contracts. Assume that some function calls vn_lock_pair() with externally-specified flags, and corresponding vp could be NULL sometimes. I want such calls to always have correct flags, esp. if vp != NULL is rare or could not be easily checked by normal testing.

In D44046#1004894, @kib wrote:
In D44046#1004891, @jah wrote:
In D44046#1004890, @kib wrote:

So might be just allow zero flags if corresponding vp is NULL?

Sure, we could do that, but I'm curious: is there some reason why we should care what the lockflags are if there is no vnode to lock? What I have here seems more straightforward than making specific allowances for NULL vnodes.

I am about the reliable checking for the API contracts. Assume that some function calls vn_lock_pair() with externally-specified flags, and corresponding vp could be NULL sometimes. I want such calls to always have correct flags, esp. if vp != NULL is rare or could not be easily checked by normal testing.

Ok, I can buy that.

Only allow lkflags to be 0 when the corresponding vnode is NULL

This revision is now accepted and ready to land.Feb 24 2024, 12:04 AM