On 2023/02/05 10:23, Alan Stern wrote:
> I suppose we could create separate lockdep classes for every bus_type
> and device_type combination, as well as for the different sorts of
> devices -- treat things like class devices separately from normal
> devices, and so on. But even then there would be trouble.
Sorry, since I'm not familiar with devices, I can't interpret what you
are talking about in this response. But why don't you try test5() approach
in an example module shown below (i.e. treat all dev->mutex instances
independent to each other) ?
Sharing mutex_init() (like test2() approach) causes false positives,
but allocating a key on each dev->mutex (like test5() approach) should
avoid false positives.
----------
#include <linux/module.h>
#include <linux/slab.h>
static struct mutex *alloc_mutex(void)
{
return kzalloc(sizeof(struct mutex), GFP_KERNEL | __GFP_NOFAIL);
}
static void free_mutex(struct mutex *m)
{
kfree(m);
}
static void test1(void)
{
struct mutex *parent_mutex;
struct mutex *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex();
mutex_init(parent_mutex);
peer_mutex[0] = alloc_mutex();
mutex_init(peer_mutex[0]);
peer_mutex[1] = alloc_mutex();
mutex_init(peer_mutex[1]);
peer_mutex[2] = alloc_mutex();
mutex_init(peer_mutex[2]);
peer_mutex[3] = alloc_mutex();
mutex_init(peer_mutex[3]);
mutex_lock(parent_mutex);
for (i = 0; i < 4; i++)
mutex_lock(peer_mutex[i]);
for (i = 0; i < 4; i++)
mutex_unlock(peer_mutex[i]);
mutex_unlock(parent_mutex);
for (i = 0; i < 4; i++)
free_mutex(peer_mutex[i]);
free_mutex(parent_mutex);
}
static void test2(void)
{
struct mutex *parent_mutex;
struct mutex *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex();
mutex_init(parent_mutex);
for (i = 0; i < 4; i++) {
peer_mutex[i] = alloc_mutex();
mutex_init(peer_mutex[i]);
}
mutex_lock(parent_mutex);
for (i = 0; i < 4; i++)
mutex_lock(peer_mutex[i]);
for (i = 0; i < 4; i++)
mutex_unlock(peer_mutex[i]);
mutex_unlock(parent_mutex);
for (i = 0; i < 4; i++)
free_mutex(peer_mutex[i]);
free_mutex(parent_mutex);
}
static void test3(void)
{
struct mutex *parent_mutex;
struct mutex *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex();
mutex_init(parent_mutex);
for (i = 0; i < 4; i++) {
peer_mutex[i] = alloc_mutex();
switch (i) {
case 0:
mutex_init(peer_mutex[i]);
break;
case 1:
mutex_init(peer_mutex[i]);
break;
case 2:
mutex_init(peer_mutex[i]);
break;
case 3:
mutex_init(peer_mutex[i]);
break;
}
}
mutex_lock(parent_mutex);
for (i = 0; i < 4; i++)
mutex_lock(peer_mutex[i]);
for (i = 0; i < 4; i++)
mutex_unlock(peer_mutex[i]);
mutex_unlock(parent_mutex);
for (i = 0; i < 4; i++)
free_mutex(peer_mutex[i]);
free_mutex(parent_mutex);
}
static void test4(void)
{
struct mutex *parent_mutex;
struct mutex *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex();
mutex_init(parent_mutex);
for (i = 0; i < 4; i++) {
peer_mutex[i] = alloc_mutex();
switch (i) {
case 0:
mutex_init(peer_mutex[i]);
break;
case 1:
mutex_init(peer_mutex[i]);
break;
case 2:
mutex_init(peer_mutex[i]);
break;
case 3:
mutex_init(peer_mutex[i]);
break;
}
}
mutex_lock(parent_mutex);
for (i = 0; i < 4; i++)
mutex_lock(peer_mutex[i]);
for (i = 0; i < 4; i++)
mutex_unlock(peer_mutex[i]);
for (i = 3; i >= 0; i--)
mutex_lock(peer_mutex[i]);
for (i = 3; i >= 0; i--)
mutex_unlock(peer_mutex[i]);
mutex_unlock(parent_mutex);
for (i = 0; i < 4; i++)
free_mutex(peer_mutex[i]);
free_mutex(parent_mutex);
}
struct mutex2 {
struct mutex mutex;
struct lock_class_key key;
};
static struct mutex2 *alloc_mutex2(void)
{
struct mutex2 *m = kzalloc(sizeof(struct mutex2), GFP_KERNEL | __GFP_NOFAIL);
lockdep_register_key(&m->key);
mutex_init(&m->mutex);
lockdep_set_class(&m->mutex, &m->key);
return m;
}
static void free_mutex2(struct mutex2 *m)
{
mutex_destroy(&m->mutex);
lockdep_unregister_key(&m->key);
kfree(m);
}
static void test5(void)
{
struct mutex2 *parent_mutex;
struct mutex2 *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex2();
for (i = 0; i < 4; i++)
peer_mutex[i] = alloc_mutex2();
mutex_lock(&parent_mutex->mutex);
for (i = 0; i < 4; i++)
mutex_lock(&peer_mutex[i]->mutex);
for (i = 0; i < 4; i++)
mutex_unlock(&peer_mutex[i]->mutex);
mutex_unlock(&parent_mutex->mutex);
for (i = 0; i < 4; i++)
free_mutex2(peer_mutex[i]);
free_mutex2(parent_mutex);
}
static void test6(void)
{
struct mutex2 *parent_mutex;
struct mutex2 *peer_mutex[4];
int i;
pr_info("Running %s\n", __func__);
parent_mutex = alloc_mutex2();
for (i = 0; i < 4; i++)
peer_mutex[i] = alloc_mutex2();
mutex_lock(&parent_mutex->mutex);
for (i = 0; i < 4; i++)
mutex_lock(&peer_mutex[i]->mutex);
for (i = 0; i < 4; i++)
mutex_unlock(&peer_mutex[i]->mutex);
for (i = 3; i >= 0; i--)
mutex_lock(&peer_mutex[i]->mutex);
for (i = 3; i >= 0; i--)
mutex_unlock(&peer_mutex[i]->mutex);
mutex_unlock(&parent_mutex->mutex);
for (i = 0; i < 4; i++)
free_mutex2(peer_mutex[i]);
free_mutex2(parent_mutex);
}
static int __init lockdep_test_init(void)
{
if (1)
test1(); // safe and lockdep does not complain
if (0)
test2(); // safe but lockdep complains
if (1)
test3(); // safe and lockdep does not complain
if (0)
test4(); // unsafe and lockdep complains
if (1)
test5(); // safe and lockdep does not complain
if (0)
test6(); // unsafe and lockdep complains
return -EINVAL;
}
module_init(lockdep_test_init);
MODULE_LICENSE("GPL");
----------
