lib/uklock: Remove the rwlock_{upgrade,downgrade} functions #1090
Conversation
In the current form they impose very strong synchronization requirements, making it impossible to implement a more efficient read-write lock design. Most rwlocks sidestep this by either using a weaker `try_upgrade` variant or having a separate `lock_upgradable` function (allowing multiple readers but only one upgradable lock/writer lock). This removes these two functions for now to prevent new code relying on these functions. This change is breaking because it already was part of the previous release. Signed-off-by: Marco Schlumpp <marco@unikraft.io>
|
✅ Checkpatch passed Beep boop! I ran Unikraft's
|
unikraft-bot
added
area/lib
There was a problem hiding this comment.
I actually considered using uk_rwlock_upgrade these past days in some code, but wasn't convinced the implementation guaranteed atomic upgrades (i.e. "cutting in front of the writer wait queue") and IMO in that case it's no different to runlock + wlock. That alone is a good argument for either a rewrite or removal.
However, if implementing atomic upgrades is performance-prohibitive as you suggest, then removing these functions now is the best call, before Unikraft components start relying on them (and we're in 0.x so the API is somewhat flexible). If we later really need upgradable locks, we can look into the alternatives you mentioned.
Thanks!
Reviewed-by: Andrei Tatar andrei@unikraft.io
In the current form they impose very strong synchronization requirements, making it impossible to implement a more efficient read-write lock design. Most rwlocks sidestep this by either using a weaker `try_upgrade` variant or having a separate `lock_upgradable` function (allowing multiple readers but only one upgradable lock/writer lock). This removes these two functions for now to prevent new code relying on these functions. This change is breaking because it already was part of the previous release. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Andrei Tatar <andrei@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1090
Prerequisite checklist
checkpatch.ukon your commit series before opening this PR;Base target
Additional configuration
Description of changes
In the current form they impose very strong synchronization requirements, making it impossible to implement a more efficient read-write lock design.
Most rwlocks sidestep this by either using a weaker
try_upgradevariant or having a separatelock_upgradablefunction (allowing multiple readers but only one upgradable lock/writer lock).This removes these two functions for now to prevent new code relying on these functions. This change is breaking because it already was part of the previous release.