Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 Aug 2022 10:12:33 +0200
From:   Vincent Whitchurch <vincent.whitchurch@axis.com>
To:     Lars-Peter Clausen <lars@metafoo.de>
CC:     Jonathan Cameron <jic23@kernel.org>, kernel <kernel@axis.com>,
        "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Peter Rosin <peda@axentia.se>
Subject: Re: [PATCH] iio: buffer: Silence lock nesting splat
Message-ID: <YwSL8VD78u0ea0Qb@axis.com>
References: <20220816080828.1218667-1-vincent.whitchurch@axis.com>
 <20220820120640.6d1b928d@jic23-huawei>
 <ff2bc13c-f66f-03f3-fc01-c4f962f7b694@metafoo.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ff2bc13c-f66f-03f3-fc01-c4f962f7b694@metafoo.de>
Precedence: bulk

On Sat, Aug 20, 2022 at 01:08:28PM +0200, Lars-Peter Clausen wrote:
> There are two different approaches for this kind of nested locking. One 
> is to use mutex_lock_nested(). This works if there is a strict 
> hierarchy. The I2C framework for example has a function to determine the 
> position of a I2C mux in the hierarchy and uses that for locking. See 
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L1151.
> 
> I'm not sure this directly translates to IIO since the 
> consumers/producers don't have to be a in strict hierarchy.? And if it 
> is a complex graph it can be difficult to figure out the right level for 
> mutex_lock_nested().
> 
> The other method is to mark each mutex as its own class. lockdep does 
> the lock checking based on the lock class and by default the same mutex 
> of different instances is considered the same class to keep the resource 
> requirements for the checker lower.
> 
> Regmap for example does this. See 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap.c#L795.
> 
> This could be a solution for IIO with the downside how the additional 
> work for the checker. But as long as there are only a few IIO devices 
> per system that should be OK. We could also only set the per device lock 
> class if in kernel consumers are enabled.

The second method certainly sounds like a better fix, since it also
still warns if one actually takes the same iio_dev mutex twice.  I'll
respin the patch.  Thanks.