Add a couple APIs used by kernel/bpf/stackmap.c only: - mmap_read_trylock_non_owner() - mmap_read_unlock_non_owner() (may be called from a work queue). It's still not ideal that bpf/stackmap subverts the lock ownership in this way. Thanks to Peter Zijlstra for suggesting this API as the least-ugly way of addressing this in the short term.
Unified · Split
| @@ -56,4 +56,18 @@ static inline void mmap_read_unlock(struct mm_struct *mm) · 55 unmodified lines | |||
| 56 | up_read(&mm->mmap_sem); | 56 | up_read(&mm->mmap_sem); |
| 57 | } | 57 | } |
| 58 | 58 | ||
| 59 | static inline bool mmap_read_trylock_non_owner(struct mm_struct *mm) | ||
| 60 | { | ||
| 61 | if (down_read_trylock(&mm->mmap_sem)) { | ||
| 62 | rwsem_release(&mm->mmap_sem.dep_map, _RET_IP_); | ||
| 63 | return true; | ||
| 64 | } | ||
| 65 | return false; | ||
| 66 | } | ||
| 67 | |||
| 68 | static inline void mmap_read_unlock_non_owner(struct mm_struct *mm) | ||
| 69 | { | ||
| 70 | up_read_non_owner(&mm->mmap_sem); | ||
| 71 | } | ||
| 72 | |||
| 59 | #endif /* _LINUX_MMAP_LOCK_H */ | 73 | #endif /* _LINUX_MMAP_LOCK_H */ |
| @@ -33,7 +33,7 @@ struct bpf_stack_map { · 32 unmodified lines | |||
| 33 | /* irq_work to run up_read() for build_id lookup in nmi context */ | 33 | /* irq_work to run up_read() for build_id lookup in nmi context */ |
| 34 | struct stack_map_irq_work { | 34 | struct stack_map_irq_work { |
| 35 | struct irq_work irq_work; | 35 | struct irq_work irq_work; |
| 36 | struct rw_semaphore *sem; | 36 | struct mm_struct *mm; |
| 37 | }; | 37 | }; |
| 38 | 38 | ||
| 39 | static void do_up_read(struct irq_work *entry) | 39 | static void do_up_read(struct irq_work *entry) |
| @@ -44,8 +44,7 @@ static void do_up_read(struct irq_work *entry) · 4 unmodified lines | |||
| 44 | return; | 44 | return; |
| 45 | 45 | ||
| 46 | work = container_of(entry, struct stack_map_irq_work, irq_work); | 46 | work = container_of(entry, struct stack_map_irq_work, irq_work); |
| 47 | up_read_non_owner(work->sem); | 47 | mmap_read_unlock_non_owner(work->mm); |
| 48 | work->sem = NULL; | ||
| 49 | } | 48 | } |
| 50 | 49 | ||
| 51 | static DEFINE_PER_CPU(struct stack_map_irq_work, up_read_work); | 50 | static DEFINE_PER_CPU(struct stack_map_irq_work, up_read_work); |
| @@ -317,7 +316,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs, · 265 unmodified lines | |||
| 317 | * with build_id. | 316 | * with build_id. |
| 318 | */ | 317 | */ |
| 319 | if (!user || !current || !current->mm || irq_work_busy || | 318 | if (!user || !current || !current->mm || irq_work_busy || |
| 320 | mmap_read_trylock(current->mm) == 0) { | 319 | !mmap_read_trylock_non_owner(current->mm)) { |
| 321 | /* cannot access current->mm, fall back to ips */ | 320 | /* cannot access current->mm, fall back to ips */ |
| 322 | for (i = 0; i < trace_nr; i++) { | 321 | for (i = 0; i < trace_nr; i++) { |
| 323 | id_offs[i].status = BPF_STACK_BUILD_ID_IP; | 322 | id_offs[i].status = BPF_STACK_BUILD_ID_IP; |
| @@ -342,16 +341,10 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs, · 18 unmodified lines | |||
| 342 | } | 341 | } |
| 343 | 342 | ||
| 344 | if (!work) { | 343 | if (!work) { |
| 345 | mmap_read_unlock(current->mm); | 344 | mmap_read_unlock_non_owner(current->mm); |
| 346 | } else { | 345 | } else { |
| 347 | work->sem = ¤t->mm->mmap_sem; | 346 | work->mm = current->mm; |
| 348 | irq_work_queue(&work->irq_work); | 347 | irq_work_queue(&work->irq_work); |
| 349 | /* | ||
| 350 | * The irq_work will release the mmap_sem with | ||
| 351 | * up_read_non_owner(). The rwsem_release() is called | ||
| 352 | * here to release the lock from lockdep's perspective. | ||
| 353 | */ | ||
| 354 | rwsem_release(¤t->mm->mmap_sem.dep_map, _RET_IP_); | ||
| 355 | } | 348 | } |
| 356 | } | 349 | } |
| 357 | 350 | ||