All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Boichat <drinkcat@chromium.org>
To: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Antti Palosaari <crope@iki.fi>, Ingo Molnar <mingo@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Bard Liao <bardliao@realtek.com>,
	Oder Chiou <oder_chiou@realtek.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.de>,
	alsa-devel@alsa-project.org,
	Anatol Pomozov <anatol.pomozov@gmail.com>
Subject: Re: [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep
Date: Fri, 26 Jun 2015 11:16:24 +0800	[thread overview]
Message-ID: <CANMq1KA2C1ZYUd_s_R5DWn-Hy_XZZQ_aaR47VLpPsNvcvxEQ9w@mail.gmail.com> (raw)
In-Reply-To: <20150625160817.GT14071@sirena.org.uk>

On Fri, Jun 26, 2015 at 12:08 AM, Mark Brown <broonie@kernel.org> wrote:
[...]
>> >As far as I can tell we're likely to end up needing a key per regmap or
>> >something similar.
>
>> Since the number of lockdep classes itself is also limited we should avoid
>> creating extra lockdep classes when we can. I think the approach which
>> having the option of specifying a lockdep class in the regmap config will be
>> ok. The only case it can't handle if we nest instances with the same config,
>> but I don't really see valid use scases for that at the moment.
>
> Oh, ffs.  This just keeps getting better.  I hadn't been aware of that
> limitation.  We still have the problem that this needs to be something
> users can understand rather than something that's just "define something
> here in one of your drivers if you're running into problems with
> spurious warnings" here.  That's always been the biggest problem here
> (once we got past the "what is this supposed to do in the first place?"
> issues).

I found that V4L2 uses separate lockdep classes for each of their
v4l2_ctrl. This was introduced in 6cd247ef22e "[media] v4l2-ctrls:
eliminate lockdep false alarms for struct v4l2_ctrl_handler.lock"
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6cd247ef22e),
so we could possibly take that approach.

On my system, I have:
# cat /proc/lockdep_stats
 lock-classes:                         1241 [max: 8191]
 direct dependencies:                  7364 [max: 32768]
 indirect dependencies:               27686
 all direct dependencies:            158097
 dependency chains:                   10011 [max: 65536]
 dependency chain hlocks:             38887 [max: 327680]
 in-hardirq chains:                      92
 in-softirq chains:                     372
 in-process chains:                    9547
 stack-trace entries:                107703 [max: 524288]

So, at least on that platform, there is some room to grow...

I'm just afraid that implementing this may require creating a bunch of
macros to wrap all regmap_init_[i2c/spi/...] functions, as the lockdep
classes need to be statically allocated... Unless we find a different
solution than what V4L2 does.

WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Boichat <drinkcat@chromium.org>
To: Mark Brown <broonie@kernel.org>
Cc: Oder Chiou <oder_chiou@realtek.com>,
	alsa-devel@alsa-project.org, Lars-Peter Clausen <lars@metafoo.de>,
	Anatol Pomozov <anatol.pomozov@gmail.com>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Takashi Iwai <tiwai@suse.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
	Antti Palosaari <crope@iki.fi>, Bard Liao <bardliao@realtek.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep
Date: Fri, 26 Jun 2015 11:16:24 +0800	[thread overview]
Message-ID: <CANMq1KA2C1ZYUd_s_R5DWn-Hy_XZZQ_aaR47VLpPsNvcvxEQ9w@mail.gmail.com> (raw)
In-Reply-To: <20150625160817.GT14071@sirena.org.uk>

On Fri, Jun 26, 2015 at 12:08 AM, Mark Brown <broonie@kernel.org> wrote:
[...]
>> >As far as I can tell we're likely to end up needing a key per regmap or
>> >something similar.
>
>> Since the number of lockdep classes itself is also limited we should avoid
>> creating extra lockdep classes when we can. I think the approach which
>> having the option of specifying a lockdep class in the regmap config will be
>> ok. The only case it can't handle if we nest instances with the same config,
>> but I don't really see valid use scases for that at the moment.
>
> Oh, ffs.  This just keeps getting better.  I hadn't been aware of that
> limitation.  We still have the problem that this needs to be something
> users can understand rather than something that's just "define something
> here in one of your drivers if you're running into problems with
> spurious warnings" here.  That's always been the biggest problem here
> (once we got past the "what is this supposed to do in the first place?"
> issues).

I found that V4L2 uses separate lockdep classes for each of their
v4l2_ctrl. This was introduced in 6cd247ef22e "[media] v4l2-ctrls:
eliminate lockdep false alarms for struct v4l2_ctrl_handler.lock"
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6cd247ef22e),
so we could possibly take that approach.

On my system, I have:
# cat /proc/lockdep_stats
 lock-classes:                         1241 [max: 8191]
 direct dependencies:                  7364 [max: 32768]
 indirect dependencies:               27686
 all direct dependencies:            158097
 dependency chains:                   10011 [max: 65536]
 dependency chain hlocks:             38887 [max: 327680]
 in-hardirq chains:                      92
 in-softirq chains:                     372
 in-process chains:                    9547
 stack-trace entries:                107703 [max: 524288]

So, at least on that platform, there is some room to grow...

I'm just afraid that implementing this may require creating a bunch of
macros to wrap all regmap_init_[i2c/spi/...] functions, as the lockdep
classes need to be statically allocated... Unless we find a different
solution than what V4L2 does.

  reply	other threads:[~2015-06-26  3:16 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-25  9:35 [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep Nicolas Boichat
2015-06-25  9:35 ` [RFC PATCH 2/2] ASoC: rt5677: Add lockdep class to silence lockdep warnings Nicolas Boichat
2015-06-25 13:21 ` [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep Arjan van de Ven
2015-06-25 14:29   ` Mark Brown
2015-06-25 15:03   ` Lars-Peter Clausen
2015-06-25 15:03     ` Lars-Peter Clausen
2015-06-25 15:33     ` Mark Brown
2015-06-25 15:47       ` Lars-Peter Clausen
2015-06-25 15:47         ` Lars-Peter Clausen
2015-06-25 16:08         ` Mark Brown
2015-06-26  3:16           ` Nicolas Boichat [this message]
2015-06-26  3:16             ` Nicolas Boichat
2015-06-29 12:51             ` Nicolas Boichat
2015-06-29 12:59               ` Lars-Peter Clausen
2015-06-29 12:59                 ` Lars-Peter Clausen
2015-06-29 14:03                 ` Nicolas Boichat
2015-06-29 14:18                   ` Lars-Peter Clausen
2015-06-29 15:34                     ` Mark Brown
2015-06-30  4:56                       ` Nicolas Boichat
2015-06-30  4:56                         ` Nicolas Boichat
2015-06-30 11:02                         ` [alsa-devel] " Lars-Peter Clausen
2015-06-29 14:22                   ` Mark Brown
2015-06-29 14:35                     ` Arjan van de Ven
2015-06-29 15:32                       ` Mark Brown
2015-06-29 15:36                         ` Arjan van de Ven
2015-06-29 16:02                           ` Mark Brown
2015-06-29 14:19                 ` Mark Brown
2015-06-25 14:49 ` Mark Brown
2015-06-25 15:59 ` Lars-Peter Clausen
2015-06-26  2:34   ` Nicolas Boichat
2015-06-26  2:34     ` Nicolas Boichat
2015-06-26  8:31     ` Lars-Peter Clausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANMq1KA2C1ZYUd_s_R5DWn-Hy_XZZQ_aaR47VLpPsNvcvxEQ9w@mail.gmail.com \
    --to=drinkcat@chromium.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=anatol.pomozov@gmail.com \
    --cc=arjan@linux.intel.com \
    --cc=bardliao@realtek.com \
    --cc=broonie@kernel.org \
    --cc=crope@iki.fi \
    --cc=gregkh@linuxfoundation.org \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@osg.samsung.com \
    --cc=mingo@redhat.com \
    --cc=oder_chiou@realtek.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.