Pull locking fixes from Ingo Molnar: "An rtmutex (PI-futex) deadlock scenario fix, plus a locking documentation fix" * 'locking-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: futex: Handle early deadlock return correctly futex: Fix barrier comment
Unified · Split
| @@ -2221,11 +2221,11 @@ static inline struct futex_hash_bucket *queue_lock(struct futex_q *q) · 2220 unmodified lines | ||
| 2221 | 2221 | * decrement the counter at queue_unlock() when some error has |
| 2222 | 2222 | * occurred and we don't end up adding the task to the list. |
| 2223 | 2223 | */ |
| 2224 | − hb_waiters_inc(hb); | |
| 2224 | + hb_waiters_inc(hb); /* implies smp_mb(); (A) */ | |
| 2225 | 2225 | |
| 2226 | 2226 | q->lock_ptr = &hb->lock; |
| 2227 | 2227 | |
| 2228 | − spin_lock(&hb->lock); /* implies smp_mb(); (A) */ | |
| 2228 | + spin_lock(&hb->lock); | |
| 2229 | 2229 | return hb; |
| 2230 | 2230 | } |
| 2231 | 2231 | |
| @@ -2861,35 +2861,39 @@ retry_private: · 629 unmodified lines | ||
| 2861 | 2861 | * and BUG when futex_unlock_pi() interleaves with this. |
| 2862 | 2862 | * |
| 2863 | 2863 | * Therefore acquire wait_lock while holding hb->lock, but drop the |
| 2864 | − * latter before calling rt_mutex_start_proxy_lock(). This still fully | |
| 2865 | − * serializes against futex_unlock_pi() as that does the exact same | |
| 2866 | − * lock handoff sequence. | |
| 2864 | + * latter before calling __rt_mutex_start_proxy_lock(). This | |
| 2865 | + * interleaves with futex_unlock_pi() -- which does a similar lock | |
| 2866 | + * handoff -- such that the latter can observe the futex_q::pi_state | |
| 2867 | + * before __rt_mutex_start_proxy_lock() is done. | |
| 2867 | 2868 | */ |
| 2868 | 2869 | raw_spin_lock_irq(&q.pi_state->pi_mutex.wait_lock); |
| 2869 | 2870 | spin_unlock(q.lock_ptr); |
| 2871 | + /* | |
| 2872 | + * __rt_mutex_start_proxy_lock() unconditionally enqueues the @rt_waiter | |
| 2873 | + * such that futex_unlock_pi() is guaranteed to observe the waiter when | |
| 2874 | + * it sees the futex_q::pi_state. | |
| 2875 | + */ | |
| 2870 | 2876 | ret = __rt_mutex_start_proxy_lock(&q.pi_state->pi_mutex, &rt_waiter, current); |
| 2871 | 2877 | raw_spin_unlock_irq(&q.pi_state->pi_mutex.wait_lock); |
| 2872 | 2878 | |
| 2873 | 2879 | if (ret) { |
| 2874 | 2880 | if (ret == 1) |
| 2875 | 2881 | ret = 0; |
| 2876 | − | |
| 2877 | − spin_lock(q.lock_ptr); | |
| 2878 | − goto no_block; | |
| 2882 | + goto cleanup; | |
| 2879 | 2883 | } |
| 2880 | 2884 | |
| 2881 | − | |
| 2882 | 2885 | if (unlikely(to)) |
| 2883 | 2886 | hrtimer_start_expires(&to->timer, HRTIMER_MODE_ABS); |
| 2884 | 2887 | |
| 2885 | 2888 | ret = rt_mutex_wait_proxy_lock(&q.pi_state->pi_mutex, to, &rt_waiter); |
| 2886 | 2889 | |
| 2890 | +cleanup: | |
| 2887 | 2891 | spin_lock(q.lock_ptr); |
| 2888 | 2892 | /* |
| 2889 | − * If we failed to acquire the lock (signal/timeout), we must | |
| 2893 | + * If we failed to acquire the lock (deadlock/signal/timeout), we must | |
| 2890 | 2894 | * first acquire the hb->lock before removing the lock from the |
| 2891 | − * rt_mutex waitqueue, such that we can keep the hb and rt_mutex | |
| 2892 | − * wait lists consistent. | |
| 2895 | + * rt_mutex waitqueue, such that we can keep the hb and rt_mutex wait | |
| 2896 | + * lists consistent. | |
| 2893 | 2897 | * |
| 2894 | 2898 | * In particular; it is important that futex_unlock_pi() can not |
| 2895 | 2899 | * observe this inconsistency. |
| @@ -3013,6 +3017,10 @@ retry: · 117 unmodified lines | ||
| 3013 | 3017 | * there is no point where we hold neither; and therefore |
| 3014 | 3018 | * wake_futex_pi() must observe a state consistent with what we |
| 3015 | 3019 | * observed. |
| 3020 | + * | |
| 3021 | + * In particular; this forces __rt_mutex_start_proxy() to | |
| 3022 | + * complete such that we're guaranteed to observe the | |
| 3023 | + * rt_waiter. Also see the WARN in wake_futex_pi(). | |
| 3016 | 3024 | */ |
| 3017 | 3025 | raw_spin_lock_irq(&pi_state->pi_mutex.wait_lock); |
| 3018 | 3026 | spin_unlock(&hb->lock); |
| @@ -1726,12 +1726,33 @@ void rt_mutex_proxy_unlock(struct rt_mutex *lock, · 1725 unmodified lines | ||
| 1726 | 1726 | rt_mutex_set_owner(lock, NULL); |
| 1727 | 1727 | } |
| 1728 | 1728 | |
| 1729 | +/** | |
| 1730 | + * __rt_mutex_start_proxy_lock() - Start lock acquisition for another task | |
| 1731 | + * @lock: the rt_mutex to take | |
| 1732 | + * @waiter: the pre-initialized rt_mutex_waiter | |
| 1733 | + * @task: the task to prepare | |
| 1734 | + * | |
| 1735 | + * Starts the rt_mutex acquire; it enqueues the @waiter and does deadlock | |
| 1736 | + * detection. It does not wait, see rt_mutex_wait_proxy_lock() for that. | |
| 1737 | + * | |
| 1738 | + * NOTE: does _NOT_ remove the @waiter on failure; must either call | |
| 1739 | + * rt_mutex_wait_proxy_lock() or rt_mutex_cleanup_proxy_lock() after this. | |
| 1740 | + * | |
| 1741 | + * Returns: | |
| 1742 | + * 0 - task blocked on lock | |
| 1743 | + * 1 - acquired the lock for task, caller should wake it up | |
| 1744 | + * <0 - error | |
| 1745 | + * | |
| 1746 | + * Special API call for PI-futex support. | |
| 1747 | + */ | |
| 1729 | 1748 | int __rt_mutex_start_proxy_lock(struct rt_mutex *lock, |
| 1730 | 1749 | struct rt_mutex_waiter *waiter, |
| 1731 | 1750 | struct task_struct *task) |
| 1732 | 1751 | { |
| 1733 | 1752 | int ret; |
| 1734 | 1753 | |
| 1754 | + lockdep_assert_held(&lock->wait_lock); | |
| 1755 | + | |
| 1735 | 1756 | if (try_to_take_rt_mutex(lock, task, NULL)) |
| 1736 | 1757 | return 1; |
| 1737 | 1758 | |
| @@ -1749,9 +1770,6 @@ int __rt_mutex_start_proxy_lock(struct rt_mutex *lock, · 11 unmodified lines | ||
| 1749 | 1770 | ret = 0; |
| 1750 | 1771 | } |
| 1751 | 1772 | |
| 1752 | − if (unlikely(ret)) | |
| 1753 | − remove_waiter(lock, waiter); | |
| 1754 | − | |
| 1755 | 1773 | debug_rt_mutex_print_deadlock(waiter); |
| 1756 | 1774 | |
| 1757 | 1775 | return ret; |
| @@ -1763,12 +1781,18 @@ int __rt_mutex_start_proxy_lock(struct rt_mutex *lock, · 5 unmodified lines | ||
| 1763 | 1781 | * @waiter: the pre-initialized rt_mutex_waiter |
| 1764 | 1782 | * @task: the task to prepare |
| 1765 | 1783 | * |
| 1784 | + * Starts the rt_mutex acquire; it enqueues the @waiter and does deadlock | |
| 1785 | + * detection. It does not wait, see rt_mutex_wait_proxy_lock() for that. | |
| 1786 | + * | |
| 1787 | + * NOTE: unlike __rt_mutex_start_proxy_lock this _DOES_ remove the @waiter | |
| 1788 | + * on failure. | |
| 1789 | + * | |
| 1766 | 1790 | * Returns: |
| 1767 | 1791 | * 0 - task blocked on lock |
| 1768 | 1792 | * 1 - acquired the lock for task, caller should wake it up |
| 1769 | 1793 | * <0 - error |
| 1770 | 1794 | * |
| 1771 | − * Special API call for FUTEX_REQUEUE_PI support. | |
| 1795 | + * Special API call for PI-futex support. | |
| 1772 | 1796 | */ |
| 1773 | 1797 | int rt_mutex_start_proxy_lock(struct rt_mutex *lock, |
| 1774 | 1798 | struct rt_mutex_waiter *waiter, |
| @@ -1778,6 +1802,8 @@ int rt_mutex_start_proxy_lock(struct rt_mutex *lock, · 3 unmodified lines | ||
| 1778 | 1802 | |
| 1779 | 1803 | raw_spin_lock_irq(&lock->wait_lock); |
| 1780 | 1804 | ret = __rt_mutex_start_proxy_lock(lock, waiter, task); |
| 1805 | + if (unlikely(ret)) | |
| 1806 | + remove_waiter(lock, waiter); | |
| 1781 | 1807 | raw_spin_unlock_irq(&lock->wait_lock); |
| 1782 | 1808 | |
| 1783 | 1809 | return ret; |
| @@ -1845,7 +1871,8 @@ int rt_mutex_wait_proxy_lock(struct rt_mutex *lock, · 61 unmodified lines | ||
| 1845 | 1871 | * @lock: the rt_mutex we were woken on |
| 1846 | 1872 | * @waiter: the pre-initialized rt_mutex_waiter |
| 1847 | 1873 | * |
| 1848 | − * Attempt to clean up after a failed rt_mutex_wait_proxy_lock(). | |
| 1874 | + * Attempt to clean up after a failed __rt_mutex_start_proxy_lock() or | |
| 1875 | + * rt_mutex_wait_proxy_lock(). | |
| 1849 | 1876 | * |
| 1850 | 1877 | * Unless we acquired the lock; we're still enqueued on the wait-list and can |
| 1851 | 1878 | * in fact still be granted ownership until we're removed. Therefore we can |