2013-03-13 06:55:31

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: [Bug fix PATCH] ACPI,acpi_memhotplug: enable acpi_memory_info->enabled of memory device presented at boot-time

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in
acpi_memhotplug.c. This change seems to require that
acpi_memory_enable_device() calls add_memory() to add all memory ranges
represented by memory device objects at boot-time, and keep the results
be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at
boot-time since all memory ranges should have been added from EFI memory
table (or e820) already. This results all memory ranges be marked as !
enabled & !failed. I think this means that we cannot hot-delete any
memory ranges presented at boot-time since acpi_memory_remove_memory()
only calls remove_memory() when the enabled flag is set. Is that
correct?"

Above mention is correct. Thus even if memory device supports hotplug,
memory presented at boot-time cannot be hot removed since the memory
device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory
device presented at boot-time for hot removing the memory device.

Signed-off-by: Yasuaki Ishimatsu <[email protected]>

---
drivers/acpi/acpi_memhotplug.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index da1f82b..88fd46a 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
continue;
}

- if (!result)
- info->enabled = 1;
+ info->enabled = 1;
+
/*
* Add num_enable even if add_memory() returns -EEXIST, so the
* device is bound to this driver.


2013-03-13 15:01:23

by Toshi Kani

[permalink] [raw]
Subject: Re: [Bug fix PATCH] ACPI,acpi_memhotplug: enable acpi_memory_info->enabled of memory device presented at boot-time

On Wed, 2013-03-13 at 15:54 +0900, Yasuaki Ishimatsu wrote:
> At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
> Toshi Kani mentioned as follows:
>
> "I have a question about the change you made in commit 65479472 in
> acpi_memhotplug.c. This change seems to require that
> acpi_memory_enable_device() calls add_memory() to add all memory ranges
> represented by memory device objects at boot-time, and keep the results
> be used for hot-remove.
>
> If I understand it right, this add_memory() call fails with EEXIST at
> boot-time since all memory ranges should have been added from EFI memory
> table (or e820) already. This results all memory ranges be marked as !
> enabled & !failed. I think this means that we cannot hot-delete any
> memory ranges presented at boot-time since acpi_memory_remove_memory()
> only calls remove_memory() when the enabled flag is set. Is that
> correct?"
>
> Above mention is correct. Thus even if memory device supports hotplug,
> memory presented at boot-time cannot be hot removed since the memory
> device's acpi_memory_info->enabled is always 0.
>
> This patch changes to set 1 to "acpi_memory_info->enabled" of memory
> device presented at boot-time for hot removing the memory device.
>
> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
>
> ---
> drivers/acpi/acpi_memhotplug.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index da1f82b..88fd46a 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> continue;
> }
>
> - if (!result)
> - info->enabled = 1;
> + info->enabled = 1;

Do we still need to keep the enable bit? I think !failed means enabled
with this change.

Thanks,
-Toshi

2013-03-18 02:20:44

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: [PATCH] Remove acpi_memory_info->failed bit

Hi Toshi,

Sorry for the late reply

2013/03/13 23:50, Toshi Kani wrote:
> On Wed, 2013-03-13 at 15:54 +0900, Yasuaki Ishimatsu wrote:
>> At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
>> Toshi Kani mentioned as follows:
>>
>> "I have a question about the change you made in commit 65479472 in
>> acpi_memhotplug.c. This change seems to require that
>> acpi_memory_enable_device() calls add_memory() to add all memory ranges
>> represented by memory device objects at boot-time, and keep the results
>> be used for hot-remove.
>>
>> If I understand it right, this add_memory() call fails with EEXIST at
>> boot-time since all memory ranges should have been added from EFI memory
>> table (or e820) already. This results all memory ranges be marked as !
>> enabled & !failed. I think this means that we cannot hot-delete any
>> memory ranges presented at boot-time since acpi_memory_remove_memory()
>> only calls remove_memory() when the enabled flag is set. Is that
>> correct?"
>>
>> Above mention is correct. Thus even if memory device supports hotplug,
>> memory presented at boot-time cannot be hot removed since the memory
>> device's acpi_memory_info->enabled is always 0.
>>
>> This patch changes to set 1 to "acpi_memory_info->enabled" of memory
>> device presented at boot-time for hot removing the memory device.
>>
>> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
>>
>> ---
>> drivers/acpi/acpi_memhotplug.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>> index da1f82b..88fd46a 100644
>> --- a/drivers/acpi/acpi_memhotplug.c
>> +++ b/drivers/acpi/acpi_memhotplug.c
>> @@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>> continue;
>> }
>>
>> - if (!result)
>> - info->enabled = 1;
>> + info->enabled = 1;
>
> Do we still need to keep the enable bit? I think !failed means enabled
> with this change.

For controlling memory hotplug, we need either failed bit or enabled bit.
So I want to remove failed bit as follows:

---

acpi_memory_info has enabled bit and failed bit for controlling memory
hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by: Yasuaki Ishimatsu <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 88fd46a..cf4e1cf 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -79,7 +79,6 @@ struct acpi_memory_info {
unsigned short caching; /* memory cache attribute */
unsigned short write_protect; /* memory read/write attribute */
unsigned int enabled:1;
- unsigned int failed:1;
};

struct acpi_memory_device {
@@ -249,10 +248,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
* returns -EEXIST. If add_memory() returns the other error, it
* means that this memory block is not used by the kernel.
*/
- if (result && result != -EEXIST) {
- info->failed = 1;
+ if (result && result != -EEXIST)
continue;
- }

info->enabled = 1;

@@ -286,16 +283,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
nid = acpi_get_node(mem_device->device->handle);

list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
- if (info->failed)
- /* The kernel does not use this memory block */
- continue;
-
if (!info->enabled)
- /*
- * The kernel uses this memory block, but it may be not
- * managed by us.
- */
- return -EBUSY;
+ continue;

if (nid < 0)
nid = memory_add_physaddr_to_nid(info->start_addr);

2013-03-19 15:15:14

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] Remove acpi_memory_info->failed bit

On Mon, 2013-03-18 at 11:19 +0900, Yasuaki Ishimatsu wrote:
> Hi Toshi,
>
> Sorry for the late reply
>
> 2013/03/13 23:50, Toshi Kani wrote:
> > On Wed, 2013-03-13 at 15:54 +0900, Yasuaki Ishimatsu wrote:
> >> At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
> >> Toshi Kani mentioned as follows:
> >>
> >> "I have a question about the change you made in commit 65479472 in
> >> acpi_memhotplug.c. This change seems to require that
> >> acpi_memory_enable_device() calls add_memory() to add all memory ranges
> >> represented by memory device objects at boot-time, and keep the results
> >> be used for hot-remove.
> >>
> >> If I understand it right, this add_memory() call fails with EEXIST at
> >> boot-time since all memory ranges should have been added from EFI memory
> >> table (or e820) already. This results all memory ranges be marked as !
> >> enabled & !failed. I think this means that we cannot hot-delete any
> >> memory ranges presented at boot-time since acpi_memory_remove_memory()
> >> only calls remove_memory() when the enabled flag is set. Is that
> >> correct?"
> >>
> >> Above mention is correct. Thus even if memory device supports hotplug,
> >> memory presented at boot-time cannot be hot removed since the memory
> >> device's acpi_memory_info->enabled is always 0.
> >>
> >> This patch changes to set 1 to "acpi_memory_info->enabled" of memory
> >> device presented at boot-time for hot removing the memory device.
> >>
> >> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
> >>
> >> ---
> >> drivers/acpi/acpi_memhotplug.c | 4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> >> index da1f82b..88fd46a 100644
> >> --- a/drivers/acpi/acpi_memhotplug.c
> >> +++ b/drivers/acpi/acpi_memhotplug.c
> >> @@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> >> continue;
> >> }
> >>
> >> - if (!result)
> >> - info->enabled = 1;
> >> + info->enabled = 1;
> >
> > Do we still need to keep the enable bit? I think !failed means enabled
> > with this change.
>
> For controlling memory hotplug, we need either failed bit or enabled bit.
> So I want to remove failed bit as follows:
>
> ---
>
> acpi_memory_info has enabled bit and failed bit for controlling memory
> hotplug. But we don't need to keep both bits.
>
> The patch removes acpi_memory_info->failed bit.

Thanks for the update. Which branch does this patch apply to? I tried
several, but could not apply.
-Toshi

2013-03-20 23:57:27

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] Remove acpi_memory_info->failed bit

Hi Toshi,

2013/03/20 0:03, Toshi Kani wrote:
> On Mon, 2013-03-18 at 11:19 +0900, Yasuaki Ishimatsu wrote:
>> Hi Toshi,
>>
>> Sorry for the late reply
>>
>> 2013/03/13 23:50, Toshi Kani wrote:
>>> On Wed, 2013-03-13 at 15:54 +0900, Yasuaki Ishimatsu wrote:
>>>> At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
>>>> Toshi Kani mentioned as follows:
>>>>
>>>> "I have a question about the change you made in commit 65479472 in
>>>> acpi_memhotplug.c. This change seems to require that
>>>> acpi_memory_enable_device() calls add_memory() to add all memory ranges
>>>> represented by memory device objects at boot-time, and keep the results
>>>> be used for hot-remove.
>>>>
>>>> If I understand it right, this add_memory() call fails with EEXIST at
>>>> boot-time since all memory ranges should have been added from EFI memory
>>>> table (or e820) already. This results all memory ranges be marked as !
>>>> enabled & !failed. I think this means that we cannot hot-delete any
>>>> memory ranges presented at boot-time since acpi_memory_remove_memory()
>>>> only calls remove_memory() when the enabled flag is set. Is that
>>>> correct?"
>>>>
>>>> Above mention is correct. Thus even if memory device supports hotplug,
>>>> memory presented at boot-time cannot be hot removed since the memory
>>>> device's acpi_memory_info->enabled is always 0.
>>>>
>>>> This patch changes to set 1 to "acpi_memory_info->enabled" of memory
>>>> device presented at boot-time for hot removing the memory device.
>>>>
>>>> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
>>>>
>>>> ---
>>>> drivers/acpi/acpi_memhotplug.c | 4 ++--
>>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>>>> index da1f82b..88fd46a 100644
>>>> --- a/drivers/acpi/acpi_memhotplug.c
>>>> +++ b/drivers/acpi/acpi_memhotplug.c
>>>> @@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>>>> continue;
>>>> }
>>>>
>>>> - if (!result)
>>>> - info->enabled = 1;
>>>> + info->enabled = 1;
>>>
>>> Do we still need to keep the enable bit? I think !failed means enabled
>>> with this change.
>>
>> For controlling memory hotplug, we need either failed bit or enabled bit.
>> So I want to remove failed bit as follows:
>>
>> ---
>>
>> acpi_memory_info has enabled bit and failed bit for controlling memory
>> hotplug. But we don't need to keep both bits.
>>
>> The patch removes acpi_memory_info->failed bit.
>
> Thanks for the update. Which branch does this patch apply to? I tried
> several, but could not apply.

These patches are based on linux-3.9-rc3.
O.K. I'll make them based on linux-pm.git/bleeding-edge.

Thanks,
Yasuaki Ishimatsu

> -Toshi
>
>

2013-03-21 04:36:50

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: [Bug fix PATCH v2 1/2] ACPI,acpi_memhotplug: enable acpi_memory_info->enabled of memory device presented at boot-time

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in
acpi_memhotplug.c. This change seems to require that
acpi_memory_enable_device() calls add_memory() to add all memory ranges
represented by memory device objects at boot-time, and keep the results
be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at
boot-time since all memory ranges should have been added from EFI memory
table (or e820) already. This results all memory ranges be marked as !
enabled & !failed. I think this means that we cannot hot-delete any
memory ranges presented at boot-time since acpi_memory_remove_memory()
only calls remove_memory() when the enabled flag is set. Is that
correct?"

Above mention is correct. Thus even if memory device supports hotplug,
memory presented at boot-time cannot be hot removed since the memory
device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory
device presented at boot-time for hot removing the memory device.

Signed-off-by: Yasuaki Ishimatsu <[email protected]>
---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
drivers/acpi/acpi_memhotplug.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index d4f2eb8..ea78988 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -206,8 +206,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
continue;
}

- if (!result)
- info->enabled = 1;
+ info->enabled = 1;
+
/*
* Add num_enable even if add_memory() returns -EEXIST, so the
* device is bound to this driver.

2013-03-21 04:39:37

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: [PATCH 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

acpi_memory_info has enabled bit and failed bit for controlling memory
hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by: yasuaki ishimatsu <[email protected]>
---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
drivers/acpi/acpi_memhotplug.c | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index ea78988..597cd65 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -73,7 +73,6 @@ struct acpi_memory_info {
unsigned short caching; /* memory cache attribute */
unsigned short write_protect; /* memory read/write attribute */
unsigned int enabled:1;
- unsigned int failed:1;
};

struct acpi_memory_device {
@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
* returns -EEXIST. If add_memory() returns the other error, it
* means that this memory block is not used by the kernel.
*/
- if (result && result != -EEXIST) {
- info->failed = 1;
+ if (result && result != -EEXIST)
continue;
- }

info->enabled = 1;

@@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
nid = acpi_get_node(mem_device->device->handle);

list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
- if (info->failed)
- /* The kernel does not use this memory block */
- continue;
-
if (!info->enabled)
- /*
- * The kernel uses this memory block, but it may be not
- * managed by us.
- */
return -EBUSY;

if (nid < 0)

2013-03-21 04:45:08

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [Bug fix PATCH v2 1/2] ACPI,acpi_memhotplug: enable acpi_memory_info->enabled of memory device presented at boot-time

The mail's tab become spaces. So I attached origianl patch.

Thanks,
Yasuaki Ishimatsu

2013/03/21 13:36, Yasuaki Ishimatsu wrote:
> At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread,
> Toshi Kani mentioned as follows:
>
> "I have a question about the change you made in commit 65479472 in
> acpi_memhotplug.c. This change seems to require that
> acpi_memory_enable_device() calls add_memory() to add all memory ranges
> represented by memory device objects at boot-time, and keep the results
> be used for hot-remove.
>
> If I understand it right, this add_memory() call fails with EEXIST at
> boot-time since all memory ranges should have been added from EFI memory
> table (or e820) already. This results all memory ranges be marked as !
> enabled & !failed. I think this means that we cannot hot-delete any
> memory ranges presented at boot-time since acpi_memory_remove_memory()
> only calls remove_memory() when the enabled flag is set. Is that
> correct?"
>
> Above mention is correct. Thus even if memory device supports hotplug,
> memory presented at boot-time cannot be hot removed since the memory
> device's acpi_memory_info->enabled is always 0.
>
> This patch changes to set 1 to "acpi_memory_info->enabled" of memory
> device presented at boot-time for hot removing the memory device.
>
> Signed-off-by: Yasuaki Ishimatsu <[email protected]>
> ---
>
> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>
> ---
> drivers/acpi/acpi_memhotplug.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index d4f2eb8..ea78988 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -206,8 +206,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> continue;
> }
>
> - if (!result)
> - info->enabled = 1;
> + info->enabled = 1;
> +
> /*
> * Add num_enable even if add_memory() returns -EEXIST, so the
> * device is bound to this driver.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
0001-ACPI-acpi_memhotplug-enable-acpi_memory_info-enabled.patch (1.83 kB)

2013-03-21 04:45:37

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

The mail's tab become spaces. So I attached origianl patch.

Thanks,
Yasuaki Ishimatsu

2013/03/21 13:39, Yasuaki Ishimatsu wrote:
> acpi_memory_info has enabled bit and failed bit for controlling memory
> hotplug. But we don't need to keep both bits.
>
> The patch removes acpi_memory_info->failed bit.
>
> Signed-off-by: yasuaki ishimatsu <[email protected]>
> ---
>
> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>
> ---
> drivers/acpi/acpi_memhotplug.c | 13 +------------
> 1 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index ea78988..597cd65 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -73,7 +73,6 @@ struct acpi_memory_info {
> unsigned short caching; /* memory cache attribute */
> unsigned short write_protect; /* memory read/write attribute */
> unsigned int enabled:1;
> - unsigned int failed:1;
> };
>
> struct acpi_memory_device {
> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> * returns -EEXIST. If add_memory() returns the other error, it
> * means that this memory block is not used by the kernel.
> */
> - if (result && result != -EEXIST) {
> - info->failed = 1;
> + if (result && result != -EEXIST)
> continue;
> - }
>
> info->enabled = 1;
>
> @@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> nid = acpi_get_node(mem_device->device->handle);
>
> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> - if (info->failed)
> - /* The kernel does not use this memory block */
> - continue;
> -
> if (!info->enabled)
> - /*
> - * The kernel uses this memory block, but it may be not
> - * managed by us.
> - */
> return -EBUSY;
>
> if (nid < 0)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
0002-Remove-acpi_memory_info-failed-bit.patch (1.67 kB)

2013-03-22 00:40:50

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

On Thu, 2013-03-21 at 13:39 +0900, Yasuaki Ishimatsu wrote:
> acpi_memory_info has enabled bit and failed bit for controlling memory
> hotplug. But we don't need to keep both bits.
>
> The patch removes acpi_memory_info->failed bit.
>
> Signed-off-by: yasuaki ishimatsu <[email protected]>
> ---
>
> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>
> ---
> drivers/acpi/acpi_memhotplug.c | 13 +------------
> 1 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index ea78988..597cd65 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -73,7 +73,6 @@ struct acpi_memory_info {
> unsigned short caching; /* memory cache attribute */
> unsigned short write_protect; /* memory read/write attribute */
> unsigned int enabled:1;
> - unsigned int failed:1;
> };
>
> struct acpi_memory_device {
> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> * returns -EEXIST. If add_memory() returns the other error, it
> * means that this memory block is not used by the kernel.
> */
> - if (result && result != -EEXIST) {
> - info->failed = 1;
> + if (result && result != -EEXIST)
> continue;
> - }
>
> info->enabled = 1;
>
> @@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> nid = acpi_get_node(mem_device->device->handle);
>
> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> - if (info->failed)
> - /* The kernel does not use this memory block */
> - continue;
> -
> if (!info->enabled)
> - /*
> - * The kernel uses this memory block, but it may be not
> - * managed by us.
> - */
> return -EBUSY;

Shouldn't this case (!info->enabled) continue since it is the same as
info->failed before? -EBUSY was previously used for the -EEXIST case,
which is no longer a failure case with this patchset.

Thanks,
-Toshi

2013-03-22 01:36:12

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

Hi Toshi,

2013/03/22 9:29, Toshi Kani wrote:
> On Thu, 2013-03-21 at 13:39 +0900, Yasuaki Ishimatsu wrote:
>> acpi_memory_info has enabled bit and failed bit for controlling memory
>> hotplug. But we don't need to keep both bits.
>>
>> The patch removes acpi_memory_info->failed bit.
>>
>> Signed-off-by: yasuaki ishimatsu <[email protected]>
>> ---
>>
>> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>>
>> ---
>> drivers/acpi/acpi_memhotplug.c | 13 +------------
>> 1 files changed, 1 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>> index ea78988..597cd65 100644
>> --- a/drivers/acpi/acpi_memhotplug.c
>> +++ b/drivers/acpi/acpi_memhotplug.c
>> @@ -73,7 +73,6 @@ struct acpi_memory_info {
>> unsigned short caching; /* memory cache attribute */
>> unsigned short write_protect; /* memory read/write attribute */
>> unsigned int enabled:1;
>> - unsigned int failed:1;
>> };
>>
>> struct acpi_memory_device {
>> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>> * returns -EEXIST. If add_memory() returns the other error, it
>> * means that this memory block is not used by the kernel.
>> */
>> - if (result && result != -EEXIST) {
>> - info->failed = 1;
>> + if (result && result != -EEXIST)
>> continue;
>> - }
>>
>> info->enabled = 1;
>>
>> @@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
>> nid = acpi_get_node(mem_device->device->handle);
>>
>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>> - if (info->failed)
>> - /* The kernel does not use this memory block */
>> - continue;
>> -
>> if (!info->enabled)
>> - /*
>> - * The kernel uses this memory block, but it may be not
>> - * managed by us.
>> - */
>> return -EBUSY;
>
> Shouldn't this case (!info->enabled) continue since it is the same as
> info->failed before? -EBUSY was previously used for the -EEXIST case,
> which is no longer a failure case with this patchset.

You are right. It is my mitake. We need to continue to hot remove memory.
I'll update soon.

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> -Toshi
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-03-22 01:54:16

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

acpi_memory_info has enabled bit and failed bit for controlling memory
hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by: yasuaki ishimatsu <[email protected]>
---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
drivers/acpi/acpi_memhotplug.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index ea78988..5e6301e 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -73,7 +73,6 @@ struct acpi_memory_info {
unsigned short caching; /* memory cache attribute */
unsigned short write_protect; /* memory read/write attribute */
unsigned int enabled:1;
- unsigned int failed:1;
};

struct acpi_memory_device {
@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
* returns -EEXIST. If add_memory() returns the other error, it
* means that this memory block is not used by the kernel.
*/
- if (result && result != -EEXIST) {
- info->failed = 1;
+ if (result && result != -EEXIST)
continue;
- }

info->enabled = 1;

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
nid = acpi_get_node(mem_device->device->handle);

list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
- if (info->failed)
- /* The kernel does not use this memory block */
- continue;
-
if (!info->enabled)
- /*
- * The kernel uses this memory block, but it may be not
- * managed by us.
- */
- return -EBUSY;
+ continue;

if (nid < 0)
nid = memory_add_physaddr_to_nid(info->start_addr);


Attachments:
0002-Remove-acpi_memory_info-failed-bit.patch (1.80 kB)

2013-03-22 20:36:04

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
> acpi_memory_info has enabled bit and failed bit for controlling memory
> hotplug. But we don't need to keep both bits.
>
> The patch removes acpi_memory_info->failed bit.
>
> Signed-off-by: yasuaki ishimatsu <[email protected]>

Thanks for the update. It looks good. For the series:

Acked-by: Toshi Kani <[email protected]>

-Toshi


> ---
>
> v3 : Continue to memory hot remove when (!info->enabled) case.
> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>
> ---
> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
> 1 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index ea78988..5e6301e 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -73,7 +73,6 @@ struct acpi_memory_info {
> unsigned short caching; /* memory cache attribute */
> unsigned short write_protect; /* memory read/write attribute */
> unsigned int enabled:1;
> - unsigned int failed:1;
> };
>
> struct acpi_memory_device {
> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> * returns -EEXIST. If add_memory() returns the other error, it
> * means that this memory block is not used by the kernel.
> */
> - if (result && result != -EEXIST) {
> - info->failed = 1;
> + if (result && result != -EEXIST)
> continue;
> - }
>
> info->enabled = 1;
>
> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> nid = acpi_get_node(mem_device->device->handle);
>
> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> - if (info->failed)
> - /* The kernel does not use this memory block */
> - continue;
> -
> if (!info->enabled)
> - /*
> - * The kernel uses this memory block, but it may be not
> - * managed by us.
> - */
> - return -EBUSY;
> + continue;
>
> if (nid < 0)
> nid = memory_add_physaddr_to_nid(info->start_addr);
>

2013-03-25 23:57:35

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

2013/03/23 5:24, Toshi Kani wrote:
> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
>> acpi_memory_info has enabled bit and failed bit for controlling memory
>> hotplug. But we don't need to keep both bits.
>>
>> The patch removes acpi_memory_info->failed bit.
>>
>> Signed-off-by: yasuaki ishimatsu <[email protected]>
>
> Thanks for the update. It looks good. For the series:
>
> Acked-by: Toshi Kani <[email protected]>

Thanks!

>
> -Toshi
>
>
>> ---
>>
>> v3 : Continue to memory hot remove when (!info->enabled) case.
>> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>>
>> ---
>> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
>> 1 files changed, 2 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>> index ea78988..5e6301e 100644
>> --- a/drivers/acpi/acpi_memhotplug.c
>> +++ b/drivers/acpi/acpi_memhotplug.c
>> @@ -73,7 +73,6 @@ struct acpi_memory_info {
>> unsigned short caching; /* memory cache attribute */
>> unsigned short write_protect; /* memory read/write attribute */
>> unsigned int enabled:1;
>> - unsigned int failed:1;
>> };
>>
>> struct acpi_memory_device {
>> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>> * returns -EEXIST. If add_memory() returns the other error, it
>> * means that this memory block is not used by the kernel.
>> */
>> - if (result && result != -EEXIST) {
>> - info->failed = 1;
>> + if (result && result != -EEXIST)
>> continue;
>> - }
>>
>> info->enabled = 1;
>>
>> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
>> nid = acpi_get_node(mem_device->device->handle);
>>
>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>> - if (info->failed)
>> - /* The kernel does not use this memory block */
>> - continue;
>> -
>> if (!info->enabled)
>> - /*
>> - * The kernel uses this memory block, but it may be not
>> - * managed by us.
>> - */
>> - return -EBUSY;
>> + continue;
>>
>> if (nid < 0)
>> nid = memory_add_physaddr_to_nid(info->start_addr);
>>
>
>

2013-03-26 13:34:03

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
> 2013/03/23 5:24, Toshi Kani wrote:
> > On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
> >> acpi_memory_info has enabled bit and failed bit for controlling memory
> >> hotplug. But we don't need to keep both bits.
> >>
> >> The patch removes acpi_memory_info->failed bit.
> >>
> >> Signed-off-by: yasuaki ishimatsu <[email protected]>
> >
> > Thanks for the update. It looks good. For the series:
> >
> > Acked-by: Toshi Kani <[email protected]>
>
> Thanks!

Both patches applied to linux-pm.git/linux-next.

Thanks,
Rafael


> >> ---
> >>
> >> v3 : Continue to memory hot remove when (!info->enabled) case.
> >> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
> >>
> >> ---
> >> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
> >> 1 files changed, 2 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> >> index ea78988..5e6301e 100644
> >> --- a/drivers/acpi/acpi_memhotplug.c
> >> +++ b/drivers/acpi/acpi_memhotplug.c
> >> @@ -73,7 +73,6 @@ struct acpi_memory_info {
> >> unsigned short caching; /* memory cache attribute */
> >> unsigned short write_protect; /* memory read/write attribute */
> >> unsigned int enabled:1;
> >> - unsigned int failed:1;
> >> };
> >>
> >> struct acpi_memory_device {
> >> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> >> * returns -EEXIST. If add_memory() returns the other error, it
> >> * means that this memory block is not used by the kernel.
> >> */
> >> - if (result && result != -EEXIST) {
> >> - info->failed = 1;
> >> + if (result && result != -EEXIST)
> >> continue;
> >> - }
> >>
> >> info->enabled = 1;
> >>
> >> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> >> nid = acpi_get_node(mem_device->device->handle);
> >>
> >> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> >> - if (info->failed)
> >> - /* The kernel does not use this memory block */
> >> - continue;
> >> -
> >> if (!info->enabled)
> >> - /*
> >> - * The kernel uses this memory block, but it may be not
> >> - * managed by us.
> >> - */
> >> - return -EBUSY;
> >> + continue;
> >>
> >> if (nid < 0)
> >> nid = memory_add_physaddr_to_nid(info->start_addr);
> >>
> >
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-03-27 23:58:31

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

Hi Rafael,

2013/03/26 22:41, Rafael J. Wysocki wrote:
> On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
>> 2013/03/23 5:24, Toshi Kani wrote:
>>> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
>>>> acpi_memory_info has enabled bit and failed bit for controlling memory
>>>> hotplug. But we don't need to keep both bits.
>>>>
>>>> The patch removes acpi_memory_info->failed bit.
>>>>
>>>> Signed-off-by: yasuaki ishimatsu <[email protected]>
>>>
>>> Thanks for the update. It looks good. For the series:
>>>
>>> Acked-by: Toshi Kani <[email protected]>
>>
>> Thanks!
>
> Both patches applied to linux-pm.git/linux-next.

Thank you for applying both patches.
If you don't mind, I hope these patches are merged into linux-3.9
since this is bug fix.

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Rafael
>
>
>>>> ---
>>>>
>>>> v3 : Continue to memory hot remove when (!info->enabled) case.
>>>> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>>>>
>>>> ---
>>>> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
>>>> 1 files changed, 2 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>>>> index ea78988..5e6301e 100644
>>>> --- a/drivers/acpi/acpi_memhotplug.c
>>>> +++ b/drivers/acpi/acpi_memhotplug.c
>>>> @@ -73,7 +73,6 @@ struct acpi_memory_info {
>>>> unsigned short caching; /* memory cache attribute */
>>>> unsigned short write_protect; /* memory read/write attribute */
>>>> unsigned int enabled:1;
>>>> - unsigned int failed:1;
>>>> };
>>>>
>>>> struct acpi_memory_device {
>>>> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>>>> * returns -EEXIST. If add_memory() returns the other error, it
>>>> * means that this memory block is not used by the kernel.
>>>> */
>>>> - if (result && result != -EEXIST) {
>>>> - info->failed = 1;
>>>> + if (result && result != -EEXIST)
>>>> continue;
>>>> - }
>>>>
>>>> info->enabled = 1;
>>>>
>>>> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
>>>> nid = acpi_get_node(mem_device->device->handle);
>>>>
>>>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>>>> - if (info->failed)
>>>> - /* The kernel does not use this memory block */
>>>> - continue;
>>>> -
>>>> if (!info->enabled)
>>>> - /*
>>>> - * The kernel uses this memory block, but it may be not
>>>> - * managed by us.
>>>> - */
>>>> - return -EBUSY;
>>>> + continue;
>>>>
>>>> if (nid < 0)
>>>> nid = memory_add_physaddr_to_nid(info->start_addr);
>>>>
>>>
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-04-01 23:40:32

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

On Thursday, March 28, 2013 08:58:03 AM Yasuaki Ishimatsu wrote:
> Hi Rafael,
>
> 2013/03/26 22:41, Rafael J. Wysocki wrote:
> > On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
> >> 2013/03/23 5:24, Toshi Kani wrote:
> >>> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
> >>>> acpi_memory_info has enabled bit and failed bit for controlling memory
> >>>> hotplug. But we don't need to keep both bits.
> >>>>
> >>>> The patch removes acpi_memory_info->failed bit.
> >>>>
> >>>> Signed-off-by: yasuaki ishimatsu <[email protected]>
> >>>
> >>> Thanks for the update. It looks good. For the series:
> >>>
> >>> Acked-by: Toshi Kani <[email protected]>
> >>
> >> Thanks!
> >
> > Both patches applied to linux-pm.git/linux-next.
>
> Thank you for applying both patches.
> If you don't mind, I hope these patches are merged into linux-3.9
> since this is bug fix.

I honestly don't think that [2/2] is 3.9 material, but I can push [1/2]
for 3.9 provided that you also regard it as material for 3.8-stable.

Thanks,
Rafael


> >>>> ---
> >>>>
> >>>> v3 : Continue to memory hot remove when (!info->enabled) case.
> >>>> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
> >>>>
> >>>> ---
> >>>> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
> >>>> 1 files changed, 2 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> >>>> index ea78988..5e6301e 100644
> >>>> --- a/drivers/acpi/acpi_memhotplug.c
> >>>> +++ b/drivers/acpi/acpi_memhotplug.c
> >>>> @@ -73,7 +73,6 @@ struct acpi_memory_info {
> >>>> unsigned short caching; /* memory cache attribute */
> >>>> unsigned short write_protect; /* memory read/write attribute */
> >>>> unsigned int enabled:1;
> >>>> - unsigned int failed:1;
> >>>> };
> >>>>
> >>>> struct acpi_memory_device {
> >>>> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
> >>>> * returns -EEXIST. If add_memory() returns the other error, it
> >>>> * means that this memory block is not used by the kernel.
> >>>> */
> >>>> - if (result && result != -EEXIST) {
> >>>> - info->failed = 1;
> >>>> + if (result && result != -EEXIST)
> >>>> continue;
> >>>> - }
> >>>>
> >>>> info->enabled = 1;
> >>>>
> >>>> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
> >>>> nid = acpi_get_node(mem_device->device->handle);
> >>>>
> >>>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
> >>>> - if (info->failed)
> >>>> - /* The kernel does not use this memory block */
> >>>> - continue;
> >>>> -
> >>>> if (!info->enabled)
> >>>> - /*
> >>>> - * The kernel uses this memory block, but it may be not
> >>>> - * managed by us.
> >>>> - */
> >>>> - return -EBUSY;
> >>>> + continue;
> >>>>
> >>>> if (nid < 0)
> >>>> nid = memory_add_physaddr_to_nid(info->start_addr);
> >>>>
> >>>
> >>>
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-02 00:13:26

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

Hi Rafael,

2013/04/02 8:48, Rafael J. Wysocki wrote:
> On Thursday, March 28, 2013 08:58:03 AM Yasuaki Ishimatsu wrote:
>> Hi Rafael,
>>
>> 2013/03/26 22:41, Rafael J. Wysocki wrote:
>>> On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
>>>> 2013/03/23 5:24, Toshi Kani wrote:
>>>>> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
>>>>>> acpi_memory_info has enabled bit and failed bit for controlling memory
>>>>>> hotplug. But we don't need to keep both bits.
>>>>>>
>>>>>> The patch removes acpi_memory_info->failed bit.
>>>>>>
>>>>>> Signed-off-by: yasuaki ishimatsu <[email protected]>
>>>>>
>>>>> Thanks for the update. It looks good. For the series:
>>>>>
>>>>> Acked-by: Toshi Kani <[email protected]>
>>>>
>>>> Thanks!
>>>
>>> Both patches applied to linux-pm.git/linux-next.
>>
>> Thank you for applying both patches.
>> If you don't mind, I hope these patches are merged into linux-3.9
>> since this is bug fix.
>
> I honestly don't think that [2/2] is 3.9 material, but I can push [1/2]
> for 3.9 provided that you also regard it as material for 3.8-stable.

linux-3.8 does not support physical memory hot remove. It has been supported
since linux-3.9. So I don't need [1/2] patch to linux-3.8 but need it to
linux-3.9. So please push [1/2] patch to linux-3.9.

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Rafael
>
>
>>>>>> ---
>>>>>>
>>>>>> v3 : Continue to memory hot remove when (!info->enabled) case.
>>>>>> v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.
>>>>>>
>>>>>> ---
>>>>>> drivers/acpi/acpi_memhotplug.c | 15 ++-------------
>>>>>> 1 files changed, 2 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>>>>>> index ea78988..5e6301e 100644
>>>>>> --- a/drivers/acpi/acpi_memhotplug.c
>>>>>> +++ b/drivers/acpi/acpi_memhotplug.c
>>>>>> @@ -73,7 +73,6 @@ struct acpi_memory_info {
>>>>>> unsigned short caching; /* memory cache attribute */
>>>>>> unsigned short write_protect; /* memory read/write attribute */
>>>>>> unsigned int enabled:1;
>>>>>> - unsigned int failed:1;
>>>>>> };
>>>>>>
>>>>>> struct acpi_memory_device {
>>>>>> @@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
>>>>>> * returns -EEXIST. If add_memory() returns the other error, it
>>>>>> * means that this memory block is not used by the kernel.
>>>>>> */
>>>>>> - if (result && result != -EEXIST) {
>>>>>> - info->failed = 1;
>>>>>> + if (result && result != -EEXIST)
>>>>>> continue;
>>>>>> - }
>>>>>>
>>>>>> info->enabled = 1;
>>>>>>
>>>>>> @@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
>>>>>> nid = acpi_get_node(mem_device->device->handle);
>>>>>>
>>>>>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
>>>>>> - if (info->failed)
>>>>>> - /* The kernel does not use this memory block */
>>>>>> - continue;
>>>>>> -
>>>>>> if (!info->enabled)
>>>>>> - /*
>>>>>> - * The kernel uses this memory block, but it may be not
>>>>>> - * managed by us.
>>>>>> - */
>>>>>> - return -EBUSY;
>>>>>> + continue;
>>>>>>
>>>>>> if (nid < 0)
>>>>>> nid = memory_add_physaddr_to_nid(info->start_addr);
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-04-02 12:49:31

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

On Tuesday, April 02, 2013 09:12:44 AM Yasuaki Ishimatsu wrote:
> Hi Rafael,
>
> 2013/04/02 8:48, Rafael J. Wysocki wrote:
> > On Thursday, March 28, 2013 08:58:03 AM Yasuaki Ishimatsu wrote:
> >> Hi Rafael,
> >>
> >> 2013/03/26 22:41, Rafael J. Wysocki wrote:
> >>> On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
> >>>> 2013/03/23 5:24, Toshi Kani wrote:
> >>>>> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
> >>>>>> acpi_memory_info has enabled bit and failed bit for controlling memory
> >>>>>> hotplug. But we don't need to keep both bits.
> >>>>>>
> >>>>>> The patch removes acpi_memory_info->failed bit.
> >>>>>>
> >>>>>> Signed-off-by: yasuaki ishimatsu <[email protected]>
> >>>>>
> >>>>> Thanks for the update. It looks good. For the series:
> >>>>>
> >>>>> Acked-by: Toshi Kani <[email protected]>
> >>>>
> >>>> Thanks!
> >>>
> >>> Both patches applied to linux-pm.git/linux-next.
> >>
> >> Thank you for applying both patches.
> >> If you don't mind, I hope these patches are merged into linux-3.9
> >> since this is bug fix.
> >
> > I honestly don't think that [2/2] is 3.9 material, but I can push [1/2]
> > for 3.9 provided that you also regard it as material for 3.8-stable.
>
> linux-3.8 does not support physical memory hot remove. It has been supported
> since linux-3.9. So I don't need [1/2] patch to linux-3.8 but need it to
> linux-3.9. So please push [1/2] patch to linux-3.9.

Well, I don't think it would be a good idea to push that patch for 3.9 so late
in the cycle. Yes, it would make memory hotplug work in 3.9, but that might
surprise some people and cause problems to happen as a result and we wouldn't
have the time to address them before the 3.9 release.

If that's not a big problem, I'd prefer to defer that patch until the 3.10
merge window is open (which is just about 2 weeks from now) and if need be we
can still push it for 3.9-stable afterward.

Thanks,
Rafael


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-02 23:56:03

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] ACPI,acpi_memhotplug: Remove acpi_memory_info->failed bit

Hi Rafael,

2013/04/02 21:57, Rafael J. Wysocki wrote:
> On Tuesday, April 02, 2013 09:12:44 AM Yasuaki Ishimatsu wrote:
>> Hi Rafael,
>>
>> 2013/04/02 8:48, Rafael J. Wysocki wrote:
>>> On Thursday, March 28, 2013 08:58:03 AM Yasuaki Ishimatsu wrote:
>>>> Hi Rafael,
>>>>
>>>> 2013/03/26 22:41, Rafael J. Wysocki wrote:
>>>>> On Tuesday, March 26, 2013 08:57:05 AM Yasuaki Ishimatsu wrote:
>>>>>> 2013/03/23 5:24, Toshi Kani wrote:
>>>>>>> On Fri, 2013-03-22 at 10:53 +0900, Yasuaki Ishimatsu wrote:
>>>>>>>> acpi_memory_info has enabled bit and failed bit for controlling memory
>>>>>>>> hotplug. But we don't need to keep both bits.
>>>>>>>>
>>>>>>>> The patch removes acpi_memory_info->failed bit.
>>>>>>>>
>>>>>>>> Signed-off-by: yasuaki ishimatsu <[email protected]>
>>>>>>>
>>>>>>> Thanks for the update. It looks good. For the series:
>>>>>>>
>>>>>>> Acked-by: Toshi Kani <[email protected]>
>>>>>>
>>>>>> Thanks!
>>>>>
>>>>> Both patches applied to linux-pm.git/linux-next.
>>>>
>>>> Thank you for applying both patches.
>>>> If you don't mind, I hope these patches are merged into linux-3.9
>>>> since this is bug fix.
>>>
>>> I honestly don't think that [2/2] is 3.9 material, but I can push [1/2]
>>> for 3.9 provided that you also regard it as material for 3.8-stable.
>>
>> linux-3.8 does not support physical memory hot remove. It has been supported
>> since linux-3.9. So I don't need [1/2] patch to linux-3.8 but need it to
>> linux-3.9. So please push [1/2] patch to linux-3.9.
>
> Well, I don't think it would be a good idea to push that patch for 3.9 so late
> in the cycle. Yes, it would make memory hotplug work in 3.9, but that might
> surprise some people and cause problems to happen as a result and we wouldn't
> have the time to address them before the 3.9 release.
>
> If that's not a big problem, I'd prefer to defer that patch until the 3.10
> merge window is open (which is just about 2 weeks from now) and if need be we
> can still push it for 3.9-stable afterward.

Sorry for bothering you by my mention.
I understood your thought. Please merge this patch to 3.10 and push if for
3.9-stable.

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Rafael
>
>