At 10/17/2012 04:59 PM, KOSAKI Motohiro Wrote:
> On Wed, Oct 17, 2012 at 2:48 AM, Wen Congyang <wency@xxxxxxxxxxxxxx> wrote:
>> At 10/13/2012 03:10 AM, KOSAKI Motohiro Wrote:
>>>>>> -static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
>>>>>> +static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
>>>>>> {
>>>>>> int result;
>>>>>> struct acpi_memory_info *info, *n;
>>>>>>
>>>>>> + list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>>>>>
>>>>> Which lock protect this loop?
>>>>
>>>> There is no any lock to protect it now...
>>>
>>> When iterate an item removal list, you should use lock for protecting from
>>> memory corruption.
>>>
>>>
>>>
>>>
>>>>>> +static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
>>>>>> +{
>>>>>> + int result;
>>>>>>
>>>>>> /*
>>>>>> * Ask the VM to offline this memory range.
>>>>>> * Note: Assume that this function returns zero on success
>>>>>> */
>>>>>
>>>>> Write function comment instead of this silly comment.
>>>>>
>>>>>> - list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>>>>>> - if (info->enabled) {
>>>>>> - result = remove_memory(info->start_addr, info->length);
>>>>>> - if (result)
>>>>>> - return result;
>>>>>> - }
>>>>>> - kfree(info);
>>>>>> - }
>>>>>> + result = acpi_memory_remove_memory(mem_device);
>>>>>> + if (result)
>>>>>> + return result;
>>>>>>
>>>>>> /* Power-off and eject the device */
>>>>>> result = acpi_memory_powerdown_device(mem_device);
>>>>>
>>>>> This patch move acpi_memory_powerdown_device() from ACPI_NOTIFY_EJECT_REQUEST
>>>>> to release callback, but don't explain why.
>>>>
>>>> Hmm, it doesn't move the code. It just reuse the code in acpi_memory_powerdown_device().
>>>
>>> Even if reuse or not reuse, you changed the behavior. If any changes
>>> has no good rational, you cannot get an ack.
>>
>> I don't understand this? IIRC, the behavior isn't changed.
>
> Heh, please explain why do you think so.
>
We just introduce a function, and move codes from acpi_memory_disable_device() to the new
function. We call the new function in acpi_memory_disable_device(), so the function
acpi_memory_disable_device()'s behavior isn't changed.
Maybe I don't understand what do you want to say.
Thanks
Wen Congyang
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
