2021-02-16 08:28:02

by Christian König

[permalink] [raw]
Subject: [PATCH] mutex: nuke mutex_trylock_recursive

The last user went away in the 5.11 cycle.

Signed-off-by: Christian König <[email protected]>
---
include/linux/mutex.h | 25 -------------------------
kernel/locking/mutex.c | 10 ----------
scripts/checkpatch.pl | 6 ------
3 files changed, 41 deletions(-)

diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index dcd185cbfe79..0cd631a19727 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -199,29 +199,4 @@ extern void mutex_unlock(struct mutex *lock);

extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);

-/*
- * These values are chosen such that FAIL and SUCCESS match the
- * values of the regular mutex_trylock().
- */
-enum mutex_trylock_recursive_enum {
- MUTEX_TRYLOCK_FAILED = 0,
- MUTEX_TRYLOCK_SUCCESS = 1,
- MUTEX_TRYLOCK_RECURSIVE,
-};
-
-/**
- * mutex_trylock_recursive - trylock variant that allows recursive locking
- * @lock: mutex to be locked
- *
- * This function should not be used, _ever_. It is purely for hysterical GEM
- * raisins, and once those are gone this will be removed.
- *
- * Returns:
- * - MUTEX_TRYLOCK_FAILED - trylock failed,
- * - MUTEX_TRYLOCK_SUCCESS - lock acquired,
- * - MUTEX_TRYLOCK_RECURSIVE - we already owned the lock.
- */
-extern /* __deprecated */ __must_check enum mutex_trylock_recursive_enum
-mutex_trylock_recursive(struct mutex *lock);
-
#endif /* __LINUX_MUTEX_H */
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 5352ce50a97e..adb935090768 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -86,16 +86,6 @@ bool mutex_is_locked(struct mutex *lock)
}
EXPORT_SYMBOL(mutex_is_locked);

-__must_check enum mutex_trylock_recursive_enum
-mutex_trylock_recursive(struct mutex *lock)
-{
- if (unlikely(__mutex_owner(lock) == current))
- return MUTEX_TRYLOCK_RECURSIVE;
-
- return mutex_trylock(lock);
-}
-EXPORT_SYMBOL(mutex_trylock_recursive);
-
static inline unsigned long __owner_flags(unsigned long owner)
{
return owner & MUTEX_FLAGS;
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 92e888ed939f..15f7f4fa6b99 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7069,12 +7069,6 @@ sub process {
}
}

-# check for mutex_trylock_recursive usage
- if ($line =~ /mutex_trylock_recursive/) {
- ERROR("LOCKING",
- "recursive locking is bad, do not use this ever.\n" . $herecurr);
- }
-
# check for lockdep_set_novalidate_class
if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ ||
$line =~ /__lockdep_no_validate__\s*\)/ ) {
--
2.25.1


2021-02-16 09:32:54

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] mutex: nuke mutex_trylock_recursive

On Tue, Feb 16, 2021 at 09:21:46AM +0100, Christian K?nig wrote:
> The last user went away in the 5.11 cycle.
>
> Signed-off-by: Christian K?nig <[email protected]>

Nice.

Reviewed-by: Daniel Vetter <[email protected]>

I think would be good to still stuff this into 5.12 before someone
resurrects this zombie.
-Daniel


> ---
> include/linux/mutex.h | 25 -------------------------
> kernel/locking/mutex.c | 10 ----------
> scripts/checkpatch.pl | 6 ------
> 3 files changed, 41 deletions(-)
>
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index dcd185cbfe79..0cd631a19727 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -199,29 +199,4 @@ extern void mutex_unlock(struct mutex *lock);
>
> extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
>
> -/*
> - * These values are chosen such that FAIL and SUCCESS match the
> - * values of the regular mutex_trylock().
> - */
> -enum mutex_trylock_recursive_enum {
> - MUTEX_TRYLOCK_FAILED = 0,
> - MUTEX_TRYLOCK_SUCCESS = 1,
> - MUTEX_TRYLOCK_RECURSIVE,
> -};
> -
> -/**
> - * mutex_trylock_recursive - trylock variant that allows recursive locking
> - * @lock: mutex to be locked
> - *
> - * This function should not be used, _ever_. It is purely for hysterical GEM
> - * raisins, and once those are gone this will be removed.
> - *
> - * Returns:
> - * - MUTEX_TRYLOCK_FAILED - trylock failed,
> - * - MUTEX_TRYLOCK_SUCCESS - lock acquired,
> - * - MUTEX_TRYLOCK_RECURSIVE - we already owned the lock.
> - */
> -extern /* __deprecated */ __must_check enum mutex_trylock_recursive_enum
> -mutex_trylock_recursive(struct mutex *lock);
> -
> #endif /* __LINUX_MUTEX_H */
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 5352ce50a97e..adb935090768 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -86,16 +86,6 @@ bool mutex_is_locked(struct mutex *lock)
> }
> EXPORT_SYMBOL(mutex_is_locked);
>
> -__must_check enum mutex_trylock_recursive_enum
> -mutex_trylock_recursive(struct mutex *lock)
> -{
> - if (unlikely(__mutex_owner(lock) == current))
> - return MUTEX_TRYLOCK_RECURSIVE;
> -
> - return mutex_trylock(lock);
> -}
> -EXPORT_SYMBOL(mutex_trylock_recursive);
> -
> static inline unsigned long __owner_flags(unsigned long owner)
> {
> return owner & MUTEX_FLAGS;
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 92e888ed939f..15f7f4fa6b99 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -7069,12 +7069,6 @@ sub process {
> }
> }
>
> -# check for mutex_trylock_recursive usage
> - if ($line =~ /mutex_trylock_recursive/) {
> - ERROR("LOCKING",
> - "recursive locking is bad, do not use this ever.\n" . $herecurr);
> - }
> -
> # check for lockdep_set_novalidate_class
> if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ ||
> $line =~ /__lockdep_no_validate__\s*\)/ ) {
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-02-16 10:19:20

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] mutex: nuke mutex_trylock_recursive

On Tue, Feb 16, 2021 at 10:29:00AM +0100, Daniel Vetter wrote:
> On Tue, Feb 16, 2021 at 09:21:46AM +0100, Christian K?nig wrote:
> > The last user went away in the 5.11 cycle.
> >
> > Signed-off-by: Christian K?nig <[email protected]>
>
> Nice.
>
> Reviewed-by: Daniel Vetter <[email protected]>
>
> I think would be good to still stuff this into 5.12 before someone
> resurrects this zombie.

Already done:

https://lkml.kernel.org/r/161296556531.23325.10473355259841906876.tip-bot2@tip-bot2

2021-02-16 12:41:22

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] mutex: nuke mutex_trylock_recursive



Am 16.02.21 um 11:13 schrieb Peter Zijlstra:
> On Tue, Feb 16, 2021 at 10:29:00AM +0100, Daniel Vetter wrote:
>> On Tue, Feb 16, 2021 at 09:21:46AM +0100, Christian König wrote:
>>> The last user went away in the 5.11 cycle.
>>>
>>> Signed-off-by: Christian König <[email protected]>
>> Nice.
>>
>> Reviewed-by: Daniel Vetter <[email protected]>
>>
>> I think would be good to still stuff this into 5.12 before someone
>> resurrects this zombie.
> Already done:
>
> https://lkml.kernel.org/r/161296556531.23325.10473355259841906876.tip-bot2@tip-bot2

One less bad concept to worry about.

But your patch is missing to remove the checkpatch.pl check for this :)

Cheers,
Christian.

2021-02-16 12:46:49

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] mutex: nuke mutex_trylock_recursive

On Tue, Feb 16, 2021 at 01:38:49PM +0100, Christian K?nig wrote:
>
>
> Am 16.02.21 um 11:13 schrieb Peter Zijlstra:
> > On Tue, Feb 16, 2021 at 10:29:00AM +0100, Daniel Vetter wrote:
> > > On Tue, Feb 16, 2021 at 09:21:46AM +0100, Christian K?nig wrote:
> > > > The last user went away in the 5.11 cycle.
> > > >
> > > > Signed-off-by: Christian K?nig <[email protected]>
> > > Nice.
> > >
> > > Reviewed-by: Daniel Vetter <[email protected]>
> > >
> > > I think would be good to still stuff this into 5.12 before someone
> > > resurrects this zombie.
> > Already done:
> >
> > https://lkml.kernel.org/r/161296556531.23325.10473355259841906876.tip-bot2@tip-bot2
>
> One less bad concept to worry about.
>
> But your patch is missing to remove the checkpatch.pl check for this :)

The next patch does that, look at the "Thread overview:" at the bottom
of the things.