Currently, functions like synchronize_srcu() do not have lockdep annotations resembling those of other write-side locking primitives. Such annotations might look as follows: lock_acquire(); lock_release(); Such annotations would tell lockdep that synchronize_srcu() acts like an empty critical section that waits for other (read-side) critical sections to finish. This would definitely catch some deadlock, but as pointed out by Paul Mckenney [1], this could also introduce false positives because of irq-safe/unsafe detection. Of course, there are tricks could help with this: might_sleep(); // Existing statement in __synchronize_srcu(). if (IS_ENABLED(CONFIG_PROVE_LOCKING)) { local_irq_disable(); lock_acquire(); lock_release(); local_irq_enable(); } But it would be better for lockdep to provide a separate annonation for functions like synchronize_srcu(), so that people won't need to repeat the ugly tricks above. Therefore introduce lock_sync(), which is simply an lock+unlock pair with no irq safe/unsafe deadlock check. This works because the to-be-annontated functions do not create real critical sections, and there is therefore no way that irq can create extra dependencies. [1]: https://lore.kernel.org/lkml/20180412021233.ewncg5jjuzjw3x62@tardis/
Unified · Split
| @@ -268,6 +268,10 @@ extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, · 267 unmodified lines | |||
| 268 | 268 | ||
| 269 | extern void lock_release(struct lockdep_map *lock, unsigned long ip); | 269 | extern void lock_release(struct lockdep_map *lock, unsigned long ip); |
| 270 | 270 | ||
| 271 | extern void lock_sync(struct lockdep_map *lock, unsigned int subclass, | ||
| 272 | int read, int check, struct lockdep_map *nest_lock, | ||
| 273 | unsigned long ip); | ||
| 274 | |||
| 271 | /* lock_is_held_type() returns */ | 275 | /* lock_is_held_type() returns */ |
| 272 | #define LOCK_STATE_UNKNOWN -1 | 276 | #define LOCK_STATE_UNKNOWN -1 |
| 273 | #define LOCK_STATE_NOT_HELD 0 | 277 | #define LOCK_STATE_NOT_HELD 0 |
| @@ -554,6 +558,7 @@ do { \ · 280 unmodified lines | |||
| 554 | #define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_) | 558 | #define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_) |
| 555 | #define lock_map_acquire_tryread(l) lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_) | 559 | #define lock_map_acquire_tryread(l) lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_) |
| 556 | #define lock_map_release(l) lock_release(l, _THIS_IP_) | 560 | #define lock_map_release(l) lock_release(l, _THIS_IP_) |
| 561 | #define lock_map_sync(l) lock_sync(l, 0, 0, 1, NULL, _THIS_IP_) | ||
| 557 | 562 | ||
| 558 | #ifdef CONFIG_PROVE_LOCKING | 563 | #ifdef CONFIG_PROVE_LOCKING |
| 559 | # define might_lock(lock) \ | 564 | # define might_lock(lock) \ |
| @@ -5693,6 +5693,40 @@ void lock_release(struct lockdep_map *lock, unsigned long ip) · 5692 unmodified lines | |||
| 5693 | } | 5693 | } |
| 5694 | EXPORT_SYMBOL_GPL(lock_release); | 5694 | EXPORT_SYMBOL_GPL(lock_release); |
| 5695 | 5695 | ||
| 5696 | /* | ||
| 5697 | * lock_sync() - A special annotation for synchronize_{s,}rcu()-like API. | ||
| 5698 | * | ||
| 5699 | * No actual critical section is created by the APIs annotated with this: these | ||
| 5700 | * APIs are used to wait for one or multiple critical sections (on other CPUs | ||
| 5701 | * or threads), and it means that calling these APIs inside these critical | ||
| 5702 | * sections is potential deadlock. | ||
| 5703 | * | ||
| 5704 | * This annotation acts as an acquire+release annotation pair with hardirqoff | ||
| 5705 | * being 1. Since there's no critical section, no interrupt can create extra | ||
| 5706 | * dependencies "inside" the annotation, hardirqoff == 1 allows us to avoid | ||
| 5707 | * false positives. | ||
| 5708 | */ | ||
| 5709 | void lock_sync(struct lockdep_map *lock, unsigned subclass, int read, | ||
| 5710 | int check, struct lockdep_map *nest_lock, unsigned long ip) | ||
| 5711 | { | ||
| 5712 | unsigned long flags; | ||
| 5713 | |||
| 5714 | if (unlikely(!lockdep_enabled())) | ||
| 5715 | return; | ||
| 5716 | |||
| 5717 | raw_local_irq_save(flags); | ||
| 5718 | check_flags(flags); | ||
| 5719 | |||
| 5720 | lockdep_recursion_inc(); | ||
| 5721 | __lock_acquire(lock, subclass, 0, read, check, 1, nest_lock, ip, 0, 0); | ||
| 5722 | |||
| 5723 | if (__lock_release(lock, ip)) | ||
| 5724 | check_chain_key(current); | ||
| 5725 | lockdep_recursion_finish(); | ||
| 5726 | raw_local_irq_restore(flags); | ||
| 5727 | } | ||
| 5728 | EXPORT_SYMBOL_GPL(lock_sync); | ||
| 5729 | |||
| 5696 | noinstr int lock_is_held_type(const struct lockdep_map *lock, int read) | 5730 | noinstr int lock_is_held_type(const struct lockdep_map *lock, int read) |
| 5697 | { | 5731 | { |
| 5698 | unsigned long flags; | 5732 | unsigned long flags; |