From: Gavin Shan <shangw@linux.vnet.ibm.com>
To: Michal Hocko <mhocko@suse.cz>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>,
linux-mm@kvack.org, dave@linux.vnet.ibm.com, rientjes@google.com,
akpm@linux-foundation.org
Subject: Re: [PATCH 3/3] mm/sparse: remove index_init_lock
Date: Thu, 12 Jul 2012 13:48:00 +0800 [thread overview]
Message-ID: <20120712054800.GA2526@shangw> (raw)
In-Reply-To: <20120710155940.GG19223@tiehlicka.suse.cz>
On Tue, Jul 10, 2012 at 05:59:41PM +0200, Michal Hocko wrote:
>On Mon 09-07-12 19:59:35, Gavin Shan wrote:
>[...]
>> Michal, How about the following changelog?
>>
>> ---
>>
>> sparse_index_init() is designed to be safe if two copies of it race. It
>> uses "index_init_lock" to ensure that, even in the case of a race, only
>> one CPU will manage to do:
>>
>> mem_section[root] = section;
>>
>> On the other hand, sparse_index_init() is possiblly called during system
>> boot stage and hotplug path as follows. We need't lock during system boot
>> stage to protect "mem_section[root]" and the function has been protected by
>> hotplug mutex "mem_hotplug_mutex" as well in hotplug case. So we needn't the
>> spinklock in the function.
>
>The changelog is still hard to read but it's getting there slowly ;)
>What about the following?
Thanks, Michal. I will resend the patch with your changelog :-)
Thanks for your time.
Gavin
>---
>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.
>---
>
>--
>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>
prev parent reply other threads:[~2012-07-12 5:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-06 3:09 [PATCH v4 1/3] mm/sparse: optimize sparse_index_alloc Gavin Shan
2012-07-06 3:09 ` [PATCH v4 2/3] mm/sparse: more check on mem_section number Gavin Shan
2012-07-06 3:09 ` [PATCH 3/3] mm/sparse: remove index_init_lock Gavin Shan
2012-07-09 11:13 ` Michal Hocko
2012-07-09 11:59 ` Gavin Shan
2012-07-10 15:59 ` Michal Hocko
2012-07-12 5:48 ` Gavin Shan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120712054800.GA2526@shangw \
--to=shangw@linux.vnet.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=dave@linux.vnet.ibm.com \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.cz \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.