This revision is part of a series. Click on the Stack tab below to see the context.
This series has also been squeezed into D47633 to provide an overall view.
Commit message:
Instead of fiddling directly with 'pr_mtx'.
Details
Details
- Reviewers
bapt - Commits
- rG83fcbbff6b01: MAC/do: Use prison_lock()/prison_unlock()
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
It looks like prison_lock/prison_unlock is not widely used, other than in sys/kern/sysv_msg.c. Should this be done more widely?
Comment Actions
In sys/sys/jail.h:
/* * Lock/unlock a prison. * XXX These exist not so much for general convenience, but to be useable in * the FOREACH_PRISON_DESCENDANT_LOCKED macro which can't handle them in * non-function form as currently defined. */
But I think there is no harm to use them in general. Is there any concern? Or we can adjust this comment?
Comment Actions
Is there any concern? Or we can adjust this comment?
No concern, we should just be (eventually) consistent and either prefer and use these here and in general, or if we don't prefer them we should abandon this review.
Comment Actions
I have the same thought. Although these two functions are trivial, I like its encapsulation and think it should be used in general when possible. It seems we have many mtx_{,un}lock wraps in the code, although mostly in the form of macro, and seems limited to the scope of a single driver.
Comment Actions
No concern, we should just be (eventually) consistent and either prefer and use these here and in general, or if we don't prefer them we should abandon this review.
I have the same thought. Although these two functions are trivial, I like its encapsulation and think it should be used in general when possible. It seems we have many mtx_{,un}lock wraps in the code, although mostly in the form of macro, and seems limited to the scope of a single driver.
I had seen the existing comment, and do not have a strong opinion (but a slight bias towards using prison_lock()/prison_unlock() obviously). Encapsulation is great (in particular it makes it easier to find uses of prison locks) but at the same time it obfuscates the underlying meaning, which in this case is also very simple. It could prove useful if the jail locking protocol changes (i.e., we switch it to R/W locks) but I'm not aware of any such need/plan at this point.
If you're OK with the change, let's keep this revision like that and convert the other uses of pr_mtx separately.