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().
>
I'm not sure I like that for a couple of reasons. I think there may still be a
path through the find_memory_block() code that this race condition can occur.
We could take a time sslice after the kobject_get and before getting the
memory_block pointer.
The second reason is that the node sysfs code calls find_memory_block() and it
may be a bit kludgy to have callers of find_memory_block have to reduce the
section_count after using it.
With the way the memory_block structs are kept, retrieved via a kobject_get()
call instead maintained on a local list, there may not be a solution that is
foolproof without changing this.
-Nathan
> -- Dave
>
--
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>