According to the expression, the message should be an 'and' relation.
Created attachment 144779 [details]
patch for _vn_lock()
Thanks for your submission.
I am assuming that the comment is correct but the KASSERT itself is wrong. We probably want to fail when either of the events fail.
I am proposing following change to KASSERT. While here, fixing 80-column wrapping.
- KASSERT((flags & LK_RETRY) == 0 || error == 0,
- ("LK_RETRY set with incompatible flags (0x%x) or an error occured (%d)",
- flags, error));
+ KASSERT((flags & LK_RETRY) == 0 && error == 0,
+ ("LK_RETRY set with incompatible flags (0x%x) or an error "
+ "occured (%d)", flags, error));
I believe the current assertion message is correct. An error from vop_stdlock indicates that the lock operation could not be completed, for example because LK_NOWAIT was set in the flags, or the caller specified an interruptible sleep and a signal was received. In this case, the use of LK_RETRY is incompatible with other flags.
If the FS provides its own vop_lock implementation, the assertion says that it must not return an error if LK_RETRY is set.
Should this be closed as Invalid?
It seems that we could benefit from having a macro like KASSERT_IMPLY which would assert A => B, that is, if A is true, then B must also be true (and B can be anything if A is false). In FreeBSD kernel this gets written as KASSERT(!A || B, ...) and sometimes there is a confusion when mapping back the code to the logical statement.