 a2f46ee1ba
			
		
	
	
	a2f46ee1ba
	
	
	
		
			
			So in the forward porting of various tipc packages, I was constantly getting this lockdep warning everytime I used tipc-config to set a network address for the protocol: [ INFO: possible circular locking dependency detected ] 2.6.33 #1 tipc-config/1326 is trying to acquire lock: (ref_table_lock){+.-...}, at: [<ffffffffa0315148>] tipc_ref_discard+0x53/0xd4 [tipc] but task is already holding lock: (&(&entry->lock)->rlock#2){+.-...}, at: [<ffffffffa03150d5>] tipc_ref_lock+0x43/0x63 [tipc] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&(&entry->lock)->rlock#2){+.-...}: [<ffffffff8107b508>] __lock_acquire+0xb67/0xd0f [<ffffffff8107b78c>] lock_acquire+0xdc/0x102 [<ffffffff8145471e>] _raw_spin_lock_bh+0x3b/0x6e [<ffffffffa03152b1>] tipc_ref_acquire+0xe8/0x11b [tipc] [<ffffffffa031433f>] tipc_createport_raw+0x78/0x1b9 [tipc] [<ffffffffa031450b>] tipc_createport+0x8b/0x125 [tipc] [<ffffffffa030f221>] tipc_subscr_start+0xce/0x126 [tipc] [<ffffffffa0308fb2>] process_signal_queue+0x47/0x7d [tipc] [<ffffffff81053e0c>] tasklet_action+0x8c/0xf4 [<ffffffff81054bd8>] __do_softirq+0xf8/0x1cd [<ffffffff8100aadc>] call_softirq+0x1c/0x30 [<ffffffff810549f4>] _local_bh_enable_ip+0xb8/0xd7 [<ffffffff81054a21>] local_bh_enable_ip+0xe/0x10 [<ffffffff81454d31>] _raw_spin_unlock_bh+0x34/0x39 [<ffffffffa0308eb8>] spin_unlock_bh.clone.0+0x15/0x17 [tipc] [<ffffffffa0308f47>] tipc_k_signal+0x8d/0xb1 [tipc] [<ffffffffa0308dd9>] tipc_core_start+0x8a/0xad [tipc] [<ffffffffa01b1087>] 0xffffffffa01b1087 [<ffffffff8100207d>] do_one_initcall+0x72/0x18a [<ffffffff810872fb>] sys_init_module+0xd8/0x23a [<ffffffff81009b42>] system_call_fastpath+0x16/0x1b -> #0 (ref_table_lock){+.-...}: [<ffffffff8107b3b2>] __lock_acquire+0xa11/0xd0f [<ffffffff8107b78c>] lock_acquire+0xdc/0x102 [<ffffffff81454836>] _raw_write_lock_bh+0x3b/0x6e [<ffffffffa0315148>] tipc_ref_discard+0x53/0xd4 [tipc] [<ffffffffa03141ee>] tipc_deleteport+0x40/0x119 [tipc] [<ffffffffa0316e35>] release+0xeb/0x137 [tipc] [<ffffffff8139dbf4>] sock_release+0x1f/0x6f [<ffffffff8139dc6b>] sock_close+0x27/0x2b [<ffffffff811116f6>] __fput+0x12a/0x1df [<ffffffff811117c5>] fput+0x1a/0x1c [<ffffffff8110e49b>] filp_close+0x68/0x72 [<ffffffff8110e552>] sys_close+0xad/0xe7 [<ffffffff81009b42>] system_call_fastpath+0x16/0x1b Finally decided I should fix this. Its a straightforward inversion, tipc_ref_acquire takes two locks in this order: ref_table_lock entry->lock while tipc_deleteport takes them in this order: entry->lock (via tipc_port_lock()) ref_table_lock (via tipc_ref_discard()) when the same entry is referenced, we get the above warning. The fix is equally straightforward. Theres no real relation between the entry->lock and the ref_table_lock (they just are needed at the same time), so move the entry->lock aquisition in tipc_ref_acquire down, after we unlock ref_table_lock (this is safe since the ref_table_lock guards changes to the reference table, and we've already claimed a slot there. I've tested the below fix and confirmed that it clears up the lockdep issue Signed-off-by: Neil Horman <nhorman@tuxdriver.com> CC: Allan Stephens <allan.stephens@windriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
		
			
				
	
	
		
			319 lines
		
	
	
	
		
			8.8 KiB
			
		
	
	
	
		
			C
		
	
	
	
	
	
			
		
		
	
	
			319 lines
		
	
	
	
		
			8.8 KiB
			
		
	
	
	
		
			C
		
	
	
	
	
	
| /*
 | |
|  * net/tipc/ref.c: TIPC object registry code
 | |
|  *
 | |
|  * Copyright (c) 1991-2006, Ericsson AB
 | |
|  * Copyright (c) 2004-2007, Wind River Systems
 | |
|  * All rights reserved.
 | |
|  *
 | |
|  * Redistribution and use in source and binary forms, with or without
 | |
|  * modification, are permitted provided that the following conditions are met:
 | |
|  *
 | |
|  * 1. Redistributions of source code must retain the above copyright
 | |
|  *    notice, this list of conditions and the following disclaimer.
 | |
|  * 2. Redistributions in binary form must reproduce the above copyright
 | |
|  *    notice, this list of conditions and the following disclaimer in the
 | |
|  *    documentation and/or other materials provided with the distribution.
 | |
|  * 3. Neither the names of the copyright holders nor the names of its
 | |
|  *    contributors may be used to endorse or promote products derived from
 | |
|  *    this software without specific prior written permission.
 | |
|  *
 | |
|  * Alternatively, this software may be distributed under the terms of the
 | |
|  * GNU General Public License ("GPL") version 2 as published by the Free
 | |
|  * Software Foundation.
 | |
|  *
 | |
|  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
 | |
|  * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 | |
|  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 | |
|  * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
 | |
|  * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 | |
|  * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 | |
|  * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
 | |
|  * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
 | |
|  * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 | |
|  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 | |
|  * POSSIBILITY OF SUCH DAMAGE.
 | |
|  */
 | |
| 
 | |
| #include "core.h"
 | |
| #include "ref.h"
 | |
| 
 | |
| /**
 | |
|  * struct reference - TIPC object reference entry
 | |
|  * @object: pointer to object associated with reference entry
 | |
|  * @lock: spinlock controlling access to object
 | |
|  * @ref: reference value for object (combines instance & array index info)
 | |
|  */
 | |
| 
 | |
| struct reference {
 | |
| 	void *object;
 | |
| 	spinlock_t lock;
 | |
| 	u32 ref;
 | |
| };
 | |
| 
 | |
| /**
 | |
|  * struct tipc_ref_table - table of TIPC object reference entries
 | |
|  * @entries: pointer to array of reference entries
 | |
|  * @capacity: array index of first unusable entry
 | |
|  * @init_point: array index of first uninitialized entry
 | |
|  * @first_free: array index of first unused object reference entry
 | |
|  * @last_free: array index of last unused object reference entry
 | |
|  * @index_mask: bitmask for array index portion of reference values
 | |
|  * @start_mask: initial value for instance value portion of reference values
 | |
|  */
 | |
| 
 | |
| struct ref_table {
 | |
| 	struct reference *entries;
 | |
| 	u32 capacity;
 | |
| 	u32 init_point;
 | |
| 	u32 first_free;
 | |
| 	u32 last_free;
 | |
| 	u32 index_mask;
 | |
| 	u32 start_mask;
 | |
| };
 | |
| 
 | |
| /*
 | |
|  * Object reference table consists of 2**N entries.
 | |
|  *
 | |
|  * State	Object ptr	Reference
 | |
|  * -----        ----------      ---------
 | |
|  * In use        non-NULL       XXXX|own index
 | |
|  *				(XXXX changes each time entry is acquired)
 | |
|  * Free            NULL         YYYY|next free index
 | |
|  *				(YYYY is one more than last used XXXX)
 | |
|  * Uninitialized   NULL         0
 | |
|  *
 | |
|  * Entry 0 is not used; this allows index 0 to denote the end of the free list.
 | |
|  *
 | |
|  * Note that a reference value of 0 does not necessarily indicate that an
 | |
|  * entry is uninitialized, since the last entry in the free list could also
 | |
|  * have a reference value of 0 (although this is unlikely).
 | |
|  */
 | |
| 
 | |
| static struct ref_table tipc_ref_table = { NULL };
 | |
| 
 | |
| static DEFINE_RWLOCK(ref_table_lock);
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_table_init - create reference table for objects
 | |
|  */
 | |
| 
 | |
| int tipc_ref_table_init(u32 requested_size, u32 start)
 | |
| {
 | |
| 	struct reference *table;
 | |
| 	u32 actual_size;
 | |
| 
 | |
| 	/* account for unused entry, then round up size to a power of 2 */
 | |
| 
 | |
| 	requested_size++;
 | |
| 	for (actual_size = 16; actual_size < requested_size; actual_size <<= 1)
 | |
| 		/* do nothing */ ;
 | |
| 
 | |
| 	/* allocate table & mark all entries as uninitialized */
 | |
| 
 | |
| 	table = __vmalloc(actual_size * sizeof(struct reference),
 | |
| 			  GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, PAGE_KERNEL);
 | |
| 	if (table == NULL)
 | |
| 		return -ENOMEM;
 | |
| 
 | |
| 	tipc_ref_table.entries = table;
 | |
| 	tipc_ref_table.capacity = requested_size;
 | |
| 	tipc_ref_table.init_point = 1;
 | |
| 	tipc_ref_table.first_free = 0;
 | |
| 	tipc_ref_table.last_free = 0;
 | |
| 	tipc_ref_table.index_mask = actual_size - 1;
 | |
| 	tipc_ref_table.start_mask = start & ~tipc_ref_table.index_mask;
 | |
| 
 | |
| 	return 0;
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_table_stop - destroy reference table for objects
 | |
|  */
 | |
| 
 | |
| void tipc_ref_table_stop(void)
 | |
| {
 | |
| 	if (!tipc_ref_table.entries)
 | |
| 		return;
 | |
| 
 | |
| 	vfree(tipc_ref_table.entries);
 | |
| 	tipc_ref_table.entries = NULL;
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_acquire - create reference to an object
 | |
|  *
 | |
|  * Register an object pointer in reference table and lock the object.
 | |
|  * Returns a unique reference value that is used from then on to retrieve the
 | |
|  * object pointer, or to determine that the object has been deregistered.
 | |
|  *
 | |
|  * Note: The object is returned in the locked state so that the caller can
 | |
|  * register a partially initialized object, without running the risk that
 | |
|  * the object will be accessed before initialization is complete.
 | |
|  */
 | |
| 
 | |
| u32 tipc_ref_acquire(void *object, spinlock_t **lock)
 | |
| {
 | |
| 	u32 index;
 | |
| 	u32 index_mask;
 | |
| 	u32 next_plus_upper;
 | |
| 	u32 ref;
 | |
| 	struct reference *entry = NULL;
 | |
| 
 | |
| 	if (!object) {
 | |
| 		err("Attempt to acquire reference to non-existent object\n");
 | |
| 		return 0;
 | |
| 	}
 | |
| 	if (!tipc_ref_table.entries) {
 | |
| 		err("Reference table not found during acquisition attempt\n");
 | |
| 		return 0;
 | |
| 	}
 | |
| 
 | |
| 	/* take a free entry, if available; otherwise initialize a new entry */
 | |
| 
 | |
| 	write_lock_bh(&ref_table_lock);
 | |
| 	if (tipc_ref_table.first_free) {
 | |
| 		index = tipc_ref_table.first_free;
 | |
| 		entry = &(tipc_ref_table.entries[index]);
 | |
| 		index_mask = tipc_ref_table.index_mask;
 | |
| 		next_plus_upper = entry->ref;
 | |
| 		tipc_ref_table.first_free = next_plus_upper & index_mask;
 | |
| 		ref = (next_plus_upper & ~index_mask) + index;
 | |
| 	}
 | |
| 	else if (tipc_ref_table.init_point < tipc_ref_table.capacity) {
 | |
| 		index = tipc_ref_table.init_point++;
 | |
| 		entry = &(tipc_ref_table.entries[index]);
 | |
| 		spin_lock_init(&entry->lock);
 | |
| 		ref = tipc_ref_table.start_mask + index;
 | |
| 	}
 | |
| 	else {
 | |
| 		ref = 0;
 | |
| 	}
 | |
| 	write_unlock_bh(&ref_table_lock);
 | |
| 
 | |
| 	/*
 | |
| 	 * Grab the lock so no one else can modify this entry
 | |
| 	 * While we assign its ref value & object pointer
 | |
| 	 */
 | |
| 	if (entry) {
 | |
| 		spin_lock_bh(&entry->lock);
 | |
| 		entry->ref = ref;
 | |
| 		entry->object = object;
 | |
| 		*lock = &entry->lock;
 | |
| 		/*
 | |
| 		 * keep it locked, the caller is responsible
 | |
| 		 * for unlocking this when they're done with it
 | |
| 		 */
 | |
| 	}
 | |
| 
 | |
| 	return ref;
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_discard - invalidate references to an object
 | |
|  *
 | |
|  * Disallow future references to an object and free up the entry for re-use.
 | |
|  * Note: The entry's spin_lock may still be busy after discard
 | |
|  */
 | |
| 
 | |
| void tipc_ref_discard(u32 ref)
 | |
| {
 | |
| 	struct reference *entry;
 | |
| 	u32 index;
 | |
| 	u32 index_mask;
 | |
| 
 | |
| 	if (!tipc_ref_table.entries) {
 | |
| 		err("Reference table not found during discard attempt\n");
 | |
| 		return;
 | |
| 	}
 | |
| 
 | |
| 	index_mask = tipc_ref_table.index_mask;
 | |
| 	index = ref & index_mask;
 | |
| 	entry = &(tipc_ref_table.entries[index]);
 | |
| 
 | |
| 	write_lock_bh(&ref_table_lock);
 | |
| 
 | |
| 	if (!entry->object) {
 | |
| 		err("Attempt to discard reference to non-existent object\n");
 | |
| 		goto exit;
 | |
| 	}
 | |
| 	if (entry->ref != ref) {
 | |
| 		err("Attempt to discard non-existent reference\n");
 | |
| 		goto exit;
 | |
| 	}
 | |
| 
 | |
| 	/*
 | |
| 	 * mark entry as unused; increment instance part of entry's reference
 | |
| 	 * to invalidate any subsequent references
 | |
| 	 */
 | |
| 
 | |
| 	entry->object = NULL;
 | |
| 	entry->ref = (ref & ~index_mask) + (index_mask + 1);
 | |
| 
 | |
| 	/* append entry to free entry list */
 | |
| 
 | |
| 	if (tipc_ref_table.first_free == 0)
 | |
| 		tipc_ref_table.first_free = index;
 | |
| 	else
 | |
| 		tipc_ref_table.entries[tipc_ref_table.last_free].ref |= index;
 | |
| 	tipc_ref_table.last_free = index;
 | |
| 
 | |
| exit:
 | |
| 	write_unlock_bh(&ref_table_lock);
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_lock - lock referenced object and return pointer to it
 | |
|  */
 | |
| 
 | |
| void *tipc_ref_lock(u32 ref)
 | |
| {
 | |
| 	if (likely(tipc_ref_table.entries)) {
 | |
| 		struct reference *entry;
 | |
| 
 | |
| 		entry = &tipc_ref_table.entries[ref &
 | |
| 						tipc_ref_table.index_mask];
 | |
| 		if (likely(entry->ref != 0)) {
 | |
| 			spin_lock_bh(&entry->lock);
 | |
| 			if (likely((entry->ref == ref) && (entry->object)))
 | |
| 				return entry->object;
 | |
| 			spin_unlock_bh(&entry->lock);
 | |
| 		}
 | |
| 	}
 | |
| 	return NULL;
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_unlock - unlock referenced object
 | |
|  */
 | |
| 
 | |
| void tipc_ref_unlock(u32 ref)
 | |
| {
 | |
| 	if (likely(tipc_ref_table.entries)) {
 | |
| 		struct reference *entry;
 | |
| 
 | |
| 		entry = &tipc_ref_table.entries[ref &
 | |
| 						tipc_ref_table.index_mask];
 | |
| 		if (likely((entry->ref == ref) && (entry->object)))
 | |
| 			spin_unlock_bh(&entry->lock);
 | |
| 		else
 | |
| 			err("Attempt to unlock non-existent reference\n");
 | |
| 	}
 | |
| }
 | |
| 
 | |
| /**
 | |
|  * tipc_ref_deref - return pointer referenced object (without locking it)
 | |
|  */
 | |
| 
 | |
| void *tipc_ref_deref(u32 ref)
 | |
| {
 | |
| 	if (likely(tipc_ref_table.entries)) {
 | |
| 		struct reference *entry;
 | |
| 
 | |
| 		entry = &tipc_ref_table.entries[ref &
 | |
| 						tipc_ref_table.index_mask];
 | |
| 		if (likely(entry->ref == ref))
 | |
| 			return entry->object;
 | |
| 	}
 | |
| 	return NULL;
 | |
| }
 | |
| 
 |