Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1273992 > unrolled thread

[PATCH, once again] regulator: core: avoid unused variable warning

Back to article view | Back to linux.kernel

Contents

  [PATCH, once again] regulator: core: avoid unused variable warning Arnd Bergmann <arnd@arndb.de> - 2015-11-20 12:40 +0100
    Re: [PATCH, once again] regulator: core: avoid unused variable  warning Mark Brown <broonie@kernel.org> - 2015-11-20 12:50 +0100
      Re: [PATCH, once again] regulator: core: avoid unused variable warning Arnd Bergmann <arnd@arndb.de> - 2015-11-20 13:20 +0100
        Re: [PATCH, once again] regulator: core: avoid unused variable  warning Mark Brown <broonie@kernel.org> - 2015-11-20 13:30 +0100
          Re: [PATCH, once again] regulator: core: avoid unused variable warning Arnd Bergmann <arnd@arndb.de> - 2015-11-20 13:30 +0100

#1273992 — [PATCH, once again] regulator: core: avoid unused variable warning

The second argument of the mutex_lock_nested() helper is only
evaluated if CONFIG_DEBUG_LOCK_ALLOC is set. Otherwise we
get this build warning for the new regulator_lock_supply
function:

drivers/regulator/core.c: In function 'regulator_lock_supply':
drivers/regulator/core.c:142:6: warning: unused variable 'i' [-Wunused-variable]

To avoid the warning, this patch moves the postincrement outside
of the call mutex_lock_nested(), which is enough to shut up
gcc about it.

We had some discussion about changing mutex_lock_nested to an
inline function, which would make the code do the right thing here,
but in the end decided against it, in order to guarantee that
mutex_lock_nested() does not introduced overhead without
CONFIG_DEBUG_LOCK_ALLOC.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 9f01cd4a915 ("regulator: core: introduce function to lock regulators and its supplies")
Link: http://permalink.gmane.org/gmane.linux.kernel/2068900
---
The patch that introduced the warning is now in 4.4-rc1, and I think this
patch is still the least ugly workaround we found.

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 4cf1390784e5..cf5371ee0be4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -142,7 +142,9 @@ static void regulator_lock_supply(struct regulator_dev *rdev)
 	int i = 0;
 
 	while (1) {
-		mutex_lock_nested(&rdev->mutex, i++);
+		mutex_lock_nested(&rdev->mutex, i);
+		i++;
+
 		supply = rdev->supply;
 
 		if (!rdev->supply)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1273998 — Re: [PATCH, once again] regulator: core: avoid unused variable warning

[Multipart message — attachments visible in raw view] — view raw

On Fri, Nov 20, 2015 at 12:30:24PM +0100, Arnd Bergmann wrote:

> The patch that introduced the warning is now in 4.4-rc1, and I think this
> patch is still the least ugly workaround we found.

Can we please at least have a comment explaining that this is working
around lockdep limitations?

[toc] | [prev] | [next] | [standalone]

#1274017

On Friday 20 November 2015 11:41:27 Mark Brown wrote:
> On Fri, Nov 20, 2015 at 12:30:24PM +0100, Arnd Bergmann wrote:
> 
> > The patch that introduced the warning is now in 4.4-rc1, and I think this
> > patch is still the least ugly workaround we found.
> 
> Can we please at least have a comment explaining that this is working
> around lockdep limitations?

Not sure which limitation you are referring to. Maybe you could just
modify the changelog text as you like when applying the patch?

The limitation we talked about in the previous thread was about the maximum
of 8 nesting levels, but my patch here doesn't address that at all.

I tried to capture the fact that mutex_lock_nested() intentionally
doesn't evaluate its second argument when CONFIG_DEBUG_LOCK_ALLOC
is not set, but that appears to be less of a limitation than a
choice of the interface.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274029 — Re: [PATCH, once again] regulator: core: avoid unused variable warning

[Multipart message — attachments visible in raw view] — view raw

On Fri, Nov 20, 2015 at 01:12:00PM +0100, Arnd Bergmann wrote:
> On Friday 20 November 2015 11:41:27 Mark Brown wrote:

> > Can we please at least have a comment explaining that this is working
> > around lockdep limitations?

> Not sure which limitation you are referring to. Maybe you could just
> modify the changelog text as you like when applying the patch?

> I tried to capture the fact that mutex_lock_nested() intentionally
> doesn't evaluate its second argument when CONFIG_DEBUG_LOCK_ALLOC
> is not set, but that appears to be less of a limitation than a
> choice of the interface.

That's the limitation (or intereface choice or whatever) that I'm
talking about - the code looks like a function call so not evaulating
the second argument is surprising.  I'm looking for something in the
code rather than the changelog so it doesn't get cleaned up later.

[toc] | [prev] | [next] | [standalone]

#1274031

On Friday 20 November 2015 12:24:00 Mark Brown wrote:
> On Fri, Nov 20, 2015 at 01:12:00PM +0100, Arnd Bergmann wrote:
> > On Friday 20 November 2015 11:41:27 Mark Brown wrote:
> 
> > > Can we please at least have a comment explaining that this is working
> > > around lockdep limitations?
> 
> > Not sure which limitation you are referring to. Maybe you could just
> > modify the changelog text as you like when applying the patch?
> 
> > I tried to capture the fact that mutex_lock_nested() intentionally
> > doesn't evaluate its second argument when CONFIG_DEBUG_LOCK_ALLOC
> > is not set, but that appears to be less of a limitation than a
> > choice of the interface.
> 
> That's the limitation (or intereface choice or whatever) that I'm
> talking about - the code looks like a function call so not evaulating
> the second argument is surprising.  I'm looking for something in the
> code rather than the changelog so it doesn't get cleaned up later.
> 

Got it. Will send a new version soon.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web