Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752633AbbF2MvS (ORCPT ); Mon, 29 Jun 2015 08:51:18 -0400 Received: from mail-oi0-f44.google.com ([209.85.218.44]:35096 "EHLO mail-oi0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752267AbbF2MvE (ORCPT ); Mon, 29 Jun 2015 08:51:04 -0400 MIME-Version: 1.0 In-Reply-To: References: <1435224904-35517-1-git-send-email-drinkcat@chromium.org> <558C0067.2000401@linux.intel.com> <558C1824.8020204@metafoo.de> <20150625153325.GR14071@sirena.org.uk> <558C229D.4090409@metafoo.de> <20150625160817.GT14071@sirena.org.uk> Date: Mon, 29 Jun 2015 20:51:04 +0800 Message-ID: Subject: Re: [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep From: Nicolas Boichat To: Mark Brown Cc: Lars-Peter Clausen , Arjan van de Ven , Mauro Carvalho Chehab , Antti Palosaari , Ingo Molnar , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Bard Liao , Oder Chiou , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , alsa-devel@alsa-project.org, Anatol Pomozov Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4331 Lines: 87 On Fri, Jun 26, 2015 at 11:16 AM, Nicolas Boichat wrote: > > On Fri, Jun 26, 2015 at 12:08 AM, Mark Brown 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. Following up on this. Lars-Peter's comments also highlights that we have no good way to figure out which regmap requires a separate maps, no clear hierarchy we can know about in advance, so we should put each regmap in its own class. The main issue is that the keys need to be allocated statically. We have 2 options to do this: 1. mutex_init and v4l2_ctrl_handler_init solve this issue by being a preprocessor macro that first allocates a static lock_class_key, then calls the real init function. This is not so practical in the case of regmap, as we have 14 different init functions ([devm_]regmap_init[_bus_type]), that would each require a wrapper. 2. Bus registration takes a different approach (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=be871b7e5): struct bus_type (statically allocated for each bus) has a lock_key member: "struct lock_class_key lock_key;". In the context of regmaps, that would mean adding a "lock_key" member to regmap_config. I did a quick implementation of this idea, and it seems to work, without modification to the rt5677 driver. The only issue with this is that regmap_config cannot be const anymore: we'd need to remove the const specifier in all drivers that use regmaps. Both alternatives would mean that all regmaps created from 1. the same line of code, or 2. the same regmap_config, would share the same class. That may not be an issue, however (do we have an example of different regmaps created from the same line/config that need to call each other?), and the custom mutex workaround is still available.... Any preference between a bunch of macros, and adding a non-const member to regmap_config? Or maybe someone has a better idea? Thanks! -- 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/