[v2] counter: ftm-quaddec: use devm_mutex_init()

[PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by Stepan Ionichev 2 weeks ago

ftm_quaddec_probe() calls mutex_init() but neither the cleanup
action nor a remove callback issues a matching mutex_destroy(),
which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
enabled.

Switch to devm_mutex_init() so the mutex is torn down in the same
devm scope it was set up in.

Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Add Fixes tag and note that the leak only shows up under
  CONFIG_DEBUG_MUTEXES (William, applied from the interrupt-cnt thread)

v1: https://lore.kernel.org/all/20260523184351.7567-1-sozdayvek@gmail.com/

 drivers/counter/ftm-quaddec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c
index c47741292..8455f16d6 100644
--- a/drivers/counter/ftm-quaddec.c
+++ b/drivers/counter/ftm-quaddec.c
@@ -292,7 +292,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev)
 	counter->signals = ftm_quaddec_signals;
 	counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals);
 
-	mutex_init(&ftm->ftm_quaddec_mutex);
+	ret = devm_mutex_init(&pdev->dev, &ftm->ftm_quaddec_mutex);
+	if (ret)
+		return ret;
 
 	ftm_quaddec_init(ftm);
 
-- 
2.43.0

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by William Breathitt Gray 1 week, 5 days ago

On Mon, 25 May 2026 20:12:40 +0500, Stepan Ionichev wrote:
> ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> action nor a remove callback issues a matching mutex_destroy(),
> which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> enabled.
> 
> Switch to devm_mutex_init() so the mutex is torn down in the same
> devm scope it was set up in.
> 
> [...]

Applied, thanks!

[1/1] counter: ftm-quaddec: use devm_mutex_init()
      commit: f981309937538ca7425fbf65fe4c339c00e50eca

Best regards,
-- 
William Breathitt Gray <wbg@kernel.org>

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by Joshua Crofts 1 week, 6 days ago

On Mon, 25 May 2026 at 17:14, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> action nor a remove callback issues a matching mutex_destroy(),
> which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> enabled.
>
> Switch to devm_mutex_init() so the mutex is torn down in the same
> devm scope it was set up in.
>
> Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
> v2:
> - Add Fixes tag and note that the leak only shows up under
>   CONFIG_DEBUG_MUTEXES (William, applied from the interrupt-cnt thread)
>
> v1: https://lore.kernel.org/all/20260523184351.7567-1-sozdayvek@gmail.com/
>
>  drivers/counter/ftm-quaddec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c
> index c47741292..8455f16d6 100644
> --- a/drivers/counter/ftm-quaddec.c
> +++ b/drivers/counter/ftm-quaddec.c
> @@ -292,7 +292,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev)
>         counter->signals = ftm_quaddec_signals;
>         counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals);
>
> -       mutex_init(&ftm->ftm_quaddec_mutex);
> +       ret = devm_mutex_init(&pdev->dev, &ftm->ftm_quaddec_mutex);
> +       if (ret)
> +               return ret;
>
>         ftm_quaddec_init(ftm);
>
> --
> 2.43.0
>
>

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards

CJD

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by Jonathan Cameron 1 week, 5 days ago

On Tue, 26 May 2026 12:24:25 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon, 25 May 2026 at 17:14, Stepan Ionichev <sozdayvek@gmail.com> wrote:
> >
> > ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> > action nor a remove callback issues a matching mutex_destroy(),
> > which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> > enabled.
> >
> > Switch to devm_mutex_init() so the mutex is torn down in the same
> > devm scope it was set up in.
> >
> > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
> > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> > ---
> > v2:
> > - Add Fixes tag and note that the leak only shows up under

It's not a leak as such. All that happens is the 'magic' pointer embedded
in the lock is not set NULL. No memory or counters or anything like that leaked
in current mainline where the implementation with CONFIG_DEBUG_MUTEXES is

void mutex_destroy(struct mutex *lock)
{
	DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock));
	lock->magic = NULL;
}

> >   CONFIG_DEBUG_MUTEXES (William, applied from the interrupt-cnt thread)
> >
> > v1: https://lore.kernel.org/all/20260523184351.7567-1-sozdayvek@gmail.com/
> >
> >  drivers/counter/ftm-quaddec.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c
> > index c47741292..8455f16d6 100644
> > --- a/drivers/counter/ftm-quaddec.c
> > +++ b/drivers/counter/ftm-quaddec.c
> > @@ -292,7 +292,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev)
> >         counter->signals = ftm_quaddec_signals;
> >         counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals);
> >
> > -       mutex_init(&ftm->ftm_quaddec_mutex);
> > +       ret = devm_mutex_init(&pdev->dev, &ftm->ftm_quaddec_mutex);
> > +       if (ret)
> > +               return ret;
> >
> >         ftm_quaddec_init(ftm);
> >
> > --
> > 2.43.0
> >
> >  
> 
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
>

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by William Breathitt Gray 1 week, 5 days ago

On Wed, May 27, 2026 at 05:57:58PM +0100, Jonathan Cameron wrote:
> On Tue, 26 May 2026 12:24:25 +0200
> Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> 
> > On Mon, 25 May 2026 at 17:14, Stepan Ionichev <sozdayvek@gmail.com> wrote:
> > >
> > > ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> > > action nor a remove callback issues a matching mutex_destroy(),
> > > which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> > > enabled.
> > >
> > > Switch to devm_mutex_init() so the mutex is torn down in the same
> > > devm scope it was set up in.
> > >
> > > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
> > > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> > > ---
> > > v2:
> > > - Add Fixes tag and note that the leak only shows up under
> 
> It's not a leak as such. All that happens is the 'magic' pointer embedded
> in the lock is not set NULL. No memory or counters or anything like that leaked
> in current mainline where the implementation with CONFIG_DEBUG_MUTEXES is
> 
> void mutex_destroy(struct mutex *lock)
> {
> 	DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock));
> 	lock->magic = NULL;
> }

What is the purpose of the magic pointer? Is it just informational, to
catch instances where any attempt to use the mutex occurs after the
device memory is freed for example?

William Breathitt Gray

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by Jonathan Cameron 1 week, 4 days ago

On Thu, 28 May 2026 06:52:18 +0900
William Breathitt Gray <wbg@kernel.org> wrote:

> On Wed, May 27, 2026 at 05:57:58PM +0100, Jonathan Cameron wrote:
> > On Tue, 26 May 2026 12:24:25 +0200
> > Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> >   
> > > On Mon, 25 May 2026 at 17:14, Stepan Ionichev <sozdayvek@gmail.com> wrote:  
> > > >
> > > > ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> > > > action nor a remove callback issues a matching mutex_destroy(),
> > > > which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> > > > enabled.
> > > >
> > > > Switch to devm_mutex_init() so the mutex is torn down in the same
> > > > devm scope it was set up in.
> > > >
> > > > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
> > > > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> > > > ---
> > > > v2:
> > > > - Add Fixes tag and note that the leak only shows up under  
> > 
> > It's not a leak as such. All that happens is the 'magic' pointer embedded
> > in the lock is not set NULL. No memory or counters or anything like that leaked
> > in current mainline where the implementation with CONFIG_DEBUG_MUTEXES is
> > 
> > void mutex_destroy(struct mutex *lock)
> > {
> > 	DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock));
> > 	lock->magic = NULL;
> > }  
> 
> What is the purpose of the magic pointer? Is it just informational, to
> catch instances where any attempt to use the mutex occurs after the
> device memory is freed for example?
More or less. In some cases the mutex life can be less than the structure
containing it.

> 
> William Breathitt Gray

Re: [PATCH v2] counter: ftm-quaddec: use devm_mutex_init()

Posted by William Breathitt Gray 1 week, 3 days ago

On Thu, May 28, 2026 at 02:07:45PM +0100, Jonathan Cameron wrote:
> On Thu, 28 May 2026 06:52:18 +0900
> William Breathitt Gray <wbg@kernel.org> wrote:
> 
> > On Wed, May 27, 2026 at 05:57:58PM +0100, Jonathan Cameron wrote:
> > > On Tue, 26 May 2026 12:24:25 +0200
> > > Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> > >
> > > > On Mon, 25 May 2026 at 17:14, Stepan Ionichev <sozdayvek@gmail.com> wrote:
> > > > >
> > > > > ftm_quaddec_probe() calls mutex_init() but neither the cleanup
> > > > > action nor a remove callback issues a matching mutex_destroy(),
> > > > > which leaks the lock debug state when CONFIG_DEBUG_MUTEXES is
> > > > > enabled.
> > > > >
> > > > > Switch to devm_mutex_init() so the mutex is torn down in the same
> > > > > devm scope it was set up in.
> > > > >
> > > > > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver")
> > > > > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> > > > > ---
> > > > > v2:
> > > > > - Add Fixes tag and note that the leak only shows up under
> > >
> > > It's not a leak as such. All that happens is the 'magic' pointer embedded
> > > in the lock is not set NULL. No memory or counters or anything like that leaked
> > > in current mainline where the implementation with CONFIG_DEBUG_MUTEXES is
> > >
> > > void mutex_destroy(struct mutex *lock)
> > > {
> > > 	DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock));
> > > 	lock->magic = NULL;
> > > }
> >
> > What is the purpose of the magic pointer? Is it just informational, to
> > catch instances where any attempt to use the mutex occurs after the
> > device memory is freed for example?
> More or less. In some cases the mutex life can be less than the structure
> containing it.

Stepan,

I think Jonathan is right, so I'm going to drop the Fixes tag for this
one, and pick up as well the original revision of your interrupt-cnt
patch without the Fixes tag. Sorry for the noise.

Thank you,

William Breathitt Gray