Linux Driver Verification

Nested interface is described in Documentation/lockdep-design.txt.

The purpose of _nested functions is to give tips to the linux kernel Validator which searches for deadlocks. The validator distinguishes the locks by its class (type of the structure). Like our rerouting does! The validator uses the nested tip in case if it can not tell two different objects with the same class (in addition the correct lock order is specified)

http://falcon.intra.ispras.ru:8999/stats/errortrace/profilename/default/name/results/user/user/host/localhost/password/passwd/page/Error+trace/value/29555/Task+id/5/Environment+version/artificial_env_1/Rule+name/32_7/Module/fc_lport.ko/Entry+point/ldv_main0_plain_sorted_withcheck

Thanks, trace is really required, because file drivers/scsi/libfc/fc_lport.c does not use mutex_lock_nested directly, there is no call to it.
From the name fc_lport.ko the other files can hardly be determined.

The nested calls were found in the source np_piv.c:

158 list_for_each_entry(vn_port, &n_port->vports, list) { 
159 mutex_lock_nested(&vn_port->lp_mutex, LPORT_MUTEX_VN_PORT); 
160 __fc_vport_setlink(n_port, vn_port); 
161 mutex_unlock(&vn_port->lp_mutex); 
162 }

But the BLAST trace does not contain nested calls, because
we set CONFIG_DEBUG_LOCK_ALLOC=false and all nested are replaced by normal calls

mutex.h:

extern void mutex_lock(struct mutex *lock);
extern int __must_check mutex_lock_interruptible(struct mutex *lock);
extern int __must_check mutex_lock_killable(struct mutex *lock);

# define mutex_lock_nested(lock, subclass) mutex_lock(lock)
# define mutex_lock_interruptible_nested(lock, subclass) mutex_lock_interruptible(lock)
# define mutex_lock_killable_nested(lock, subclass) mutex_lock_killable(lock)

The trace is incorrect due to bug #680, the goto removed

LDV_ERROR: goto LDV_ERROR;

But If it were not cut we have a false unsafe
lock2: different objects with the same field name

mutex_lock(&lport->lp_mutex); 
mutex_lock_nested(&vn_port->lp_mutex, LPORT_MUTEX_VN_PORT);

But as you can see, the driver developer give us a tip using special nested function with a parameter LPORT_MUTEX_VN_PORT saying that it is the different lock, not the same as &lport->lp_mutex. We definitely should use this tip!

A quick fix that comes to my mind is just to reroute nested calls separately from the "usual" locks. Rerouter should be fixed, I guess, as it supports only unary functions (if I recall correctly)

The problems with using the tips provided by nested functions:
1. The second parameter subclass may not be a constant. (for now it is not a problem because all 25 drivers use constants)
2. The function mutex_unlock does not have nested version and has no subclass parameter.

A proposed solution:¶

state
add previous lock subclass state variable: prev_subclass_TEMPLATE = 0

mutex_lock_nested
1. increment ldv_lock counter if subclass does not equal to prev_subclass, otherwise fail
2. check that subclass is greater then prev_subclass (they should be acquired in the subclass order)
3. update prev_subclass

mutex_unlock
1. decrement ldv_lock counter if possible, otherwise fail

characteristics of the solution:
+ remove false unsafes when nested functions are used
- do not check the order of unlocks, may miss true unsafes

Вадим Мутилин wrote:

A proposed solution:¶

state
add previous lock subclass state variable: prev_subclass_TEMPLATE = 0

add previous lock subclass state variable: prev_subclass_TEMPLATE = -1

mutex_lock_nested
1. increment ldv_lock counter if subclass does not equal to prev_subclass, otherwise fail
2. check that subclass is greater then prev_subclass (they should be acquired in the subclass order)
3. update prev_subclass

mutex_lock = mutex_lock_nested(lock,0)

mutex_unlock
1. decrement ldv_lock counter if possible, otherwise fail

2. update prev_subclass to counter of nested locks-1

characteristics of the solution:
+ remove false unsafes when nested functions are used
- do not check the order of unlocks, may miss true unsafes