Re: [PATCH v2 3/3] mm/sparse: remove index_init_lock

[Gavin Shan <shangw@linux.vnet.ibm.com>]

On Fri, Jul 13, 2012 at 10:41:27AM +0200, Michal Hocko wrote:
>On Fri 13-07-12 10:01:22, Gavin Shan wrote:
>> sparse_index_init uses index_init_lock spinlock to protect root
>> mem_section assignment. The lock is not necessary anymore because the
>> function is called only during the boot (during paging init which
>> is executed only from a single CPU) and from the hotplug code (by
>> add_memory via arch_add_memory) which uses mem_hotplug_mutex.
>> 
>> The lock has been introduced by 28ae55c9 (sparsemem extreme: hotplug
>> preparation) and sparse_index_init was used only during boot at that
>> time.
>> 
>> Later when the hotplug code (and add_memory) was introduced there was
>> no synchronization so it was possible to online more sections from
>> the same root probably (though I am not 100% sure about that).
>> The first synchronization has been added by 6ad696d2 (mm: allow memory
>> hotplug and hibernation in the same kernel) which has been later
>> replaced by the mem_hotplug_mutex - 20d6c96b (mem-hotplug: introduce
>> {un}lock_memory_hotplug()).
>> 
>> Let's remove the lock as it is not needed and it makes the code more
>> confusing.
>> 
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>
>Reviewed-by: Michal Hocko <mhocko@suse.cz>
>

Thanks, Michal :-)

>> ---
>>  mm/sparse.c |   14 +-------------
>>  1 files changed, 1 insertions(+), 13 deletions(-)
>> 
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index 51950de..40b1100 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -77,7 +77,6 @@ static struct mem_section noinline __init_refok *sparse_index_alloc(int nid)
>>  
>>  static int __meminit sparse_index_init(unsigned long section_nr, int nid)
>>  {
>> -	static DEFINE_SPINLOCK(index_init_lock);
>>  	unsigned long root = SECTION_NR_TO_ROOT(section_nr);
>>  	struct mem_section *section;
>>  	int ret = 0;
>> @@ -88,20 +87,9 @@ static int __meminit sparse_index_init(unsigned long section_nr, int nid)
>>  	section = sparse_index_alloc(nid);
>>  	if (!section)
>>  		return -ENOMEM;
>> -	/*
>> -	 * This lock keeps two different sections from
>> -	 * reallocating for the same index
>> -	 */
>> -	spin_lock(&index_init_lock);
>> -
>> -	if (mem_section[root]) {
>> -		ret = -EEXIST;
>> -		goto out;
>> -	}
>>  
>>  	mem_section[root] = section;
>> -out:
>> -	spin_unlock(&index_init_lock);
>> +
>>  	return ret;
>>  }
>>  #else /* !SPARSEMEM_EXTREME */
>> -- 
>> 1.7.5.4
>> 
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
>-- 
>Michal Hocko
>SUSE Labs
>SUSE LINUX s.r.o.
>Lihovarska 1060/12
>190 00 Praha 9    
>Czech Republic
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>