lkml.org 
[lkml]   [2023]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: Converting dev->mutex into dev->spinlock ?
From
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");
----------

\
 
 \ /
  Last update: 2023-03-27 00:11    [W:0.214 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site