On 07/20/2010 02:18 PM, Dave Hansen wrote:
> On Mon, 2010-07-19 at 22:55 -0500, Nathan Fontenot wrote:
>> +static int add_memory_section(int nid, struct mem_section *section,
>> + unsigned long state, enum mem_add_context context)
>> +{
>> + struct memory_block *mem;
>> + int ret = 0;
>> +
>> + mem = find_memory_block(section);
>> + if (mem) {
>> + atomic_inc(&mem->section_count);
>> + kobject_put(&mem->sysdev.kobj);
>> + } else
>> + ret = init_memory_block(&mem, section, state);
>> +
>> if (!ret) {
>> - if (context == HOTPLUG)
>> + if (context == HOTPLUG &&
>> + atomic_read(&mem->section_count) == sections_per_block)
>> ret = register_mem_sect_under_node(mem, nid);
>> }
>
> I think the atomic_inc() can race with the atomic_dec_and_test() in
> remove_memory_block().
>
> Thread 1 does:
>
> mem = find_memory_block(section);
>
> Thread 2 does
>
> atomic_dec_and_test(&mem->section_count);
>
> and destroys the memory block, Thread 1 runs again:
>
> if (mem) {
> atomic_inc(&mem->section_count);
> kobject_put(&mem->sysdev.kobj);
> } else
>
> but now mem got destroyed by Thread 2. You probably need to change
> find_memory_block() to itself take a reference, and to use
> atomic_inc_unless().
>
You're right but I think the fix you suggested will narrow the window for the
race condition, not eliminate it. We could still take a time splice in
find_memory_block prior to the container_of() calls to get the memory
block pointer and end up de-referencing a invalid kobject o sysdev pointer.
I think if we want to eliminate this we may need to have lock that protects
access to any of the memory_block structures. This would need to be taken
any time find_memory_block is called and released when use of the memory_block
returned is finished. If we're going to fix this we should eliminate the
window completely instead of just closing it further.
If we add a lock should I submit it as part of this patchset? or submit it
as a follow-on?
-Nathan
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>