2013-08-14 09:38:34

by Tang Chen

[permalink] [raw]
Subject: [PATCH 0/3] Preparation for arranging hotplug memory in ZONE_MOVABLE.

These three trivial patches are from patch-set "Arrange hotpluggable memory as ZONE_MOVABLE."
And they have been acked and reviewed by many people.

Hope they can be merged first. And the rest patches are coming soon.


Tang Chen (3):
acpi: Print Hot-Pluggable Field in SRAT.
earlycpio.c: Fix the confusing comment of find_cpio_data().
acpi: Kill macro INVALID_TABLE().

arch/x86/mm/srat.c | 11 +++++++----
drivers/acpi/osl.c | 36 ++++++++++++++++++++----------------
lib/earlycpio.c | 27 ++++++++++++++-------------
3 files changed, 41 insertions(+), 33 deletions(-)


2013-08-14 09:38:37

by Tang Chen

[permalink] [raw]
Subject: [PATCH 3/3] acpi: Kill macro INVALID_TABLE().

The macro INVALID_TABLE() is defined like this:

#define INVALID_TABLE(x, path, name) \
{ pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); continue; }

And it is used like this:

for (...) {
...
if (...)
INVALID_TABLE()
...
}

The "continue" in the macro makes the code hard to understand.

And also, this macro is only used several times in a single file.
As suggested by Joe Perches <[email protected]>, we can remote it and
use pr_err directly.

So after this patch, this macro is removed, and pr_err() is used
like this:

for (...) {
...
if (...) {
pr_err("ACPI OVERRIDE: ......");
continue;
}
...
}

Signed-off-by: Tang Chen <[email protected]>
Suggested-by: Joe Perches <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Acked-by: Toshi Kani <[email protected]>
Reviewed-by: Zhang Yanfei <[email protected]>
---
drivers/acpi/osl.c | 36 ++++++++++++++++++++----------------
1 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 6ab2c35..e7effc1 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -563,10 +563,6 @@ static const char * const table_sigs[] = {
ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };

-/* Non-fatal errors: Affected tables/files are ignored */
-#define INVALID_TABLE(x, path, name) \
- { pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); continue; }
-
#define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)

/* Must not increase 10 or needs code modification below */
@@ -593,9 +589,11 @@ void __init acpi_initrd_override(void *data, size_t size)
data += offset;
size -= offset;

- if (file.size < sizeof(struct acpi_table_header))
- INVALID_TABLE("Table smaller than ACPI header",
- cpio_path, file.name);
+ if (file.size < sizeof(struct acpi_table_header)) {
+ pr_err("ACPI OVERRIDE: Table smaller than ACPI header [%s%s]\n",
+ cpio_path, file.name);
+ continue;
+ }

table = file.data;

@@ -603,15 +601,21 @@ void __init acpi_initrd_override(void *data, size_t size)
if (!memcmp(table->signature, table_sigs[sig], 4))
break;

- if (!table_sigs[sig])
- INVALID_TABLE("Unknown signature",
- cpio_path, file.name);
- if (file.size != table->length)
- INVALID_TABLE("File length does not match table length",
- cpio_path, file.name);
- if (acpi_table_checksum(file.data, table->length))
- INVALID_TABLE("Bad table checksum",
- cpio_path, file.name);
+ if (!table_sigs[sig]) {
+ pr_err("ACPI OVERRIDE: Unknown signature [%s%s]\n",
+ cpio_path, file.name);
+ continue;
+ }
+ if (file.size != table->length) {
+ pr_err("ACPI OVERRIDE: File length does not match table length [%s%s]\n",
+ cpio_path, file.name);
+ continue;
+ }
+ if (acpi_table_checksum(file.data, table->length)) {
+ pr_err("ACPI OVERRIDE: Bad table checksum [%s%s]\n",
+ cpio_path, file.name);
+ continue;
+ }

pr_info("%4.4s ACPI table found in initrd [%s%s][0x%x]\n",
table->signature, cpio_path, file.name, table->length);
--
1.7.1

2013-08-14 09:38:58

by Tang Chen

[permalink] [raw]
Subject: [PATCH 2/3] earlycpio.c: Fix the confusing comment of find_cpio_data().

The comments of find_cpio_data() says:

* @offset: When a matching file is found, this is the offset to the
* beginning of the cpio. ......

But according to the code,

dptr = PTR_ALIGN(p + ch[C_NAMESIZE], 4);
nptr = PTR_ALIGN(dptr + ch[C_FILESIZE], 4);
....
*offset = (long)nptr - (long)data; /* data is the cpio file */

@offset is the offset of the next file, not the matching file itself.
This is confused and may cause unnecessary waste of time to debug.
So fix it.

v1 -> v2:
As tj suggested, rename @offset to @nextoff which is more clear to
users. And also adjust the new comments.

Signed-off-by: Tang Chen <[email protected]>
Reviewed-by: Zhang Yanfei <[email protected]>
Reviewed-by: Tejun Heo <[email protected]>
---
lib/earlycpio.c | 27 ++++++++++++++-------------
1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/earlycpio.c b/lib/earlycpio.c
index 7aa7ce2..c7ae5ed 100644
--- a/lib/earlycpio.c
+++ b/lib/earlycpio.c
@@ -49,22 +49,23 @@ enum cpio_fields {

/**
* cpio_data find_cpio_data - Search for files in an uncompressed cpio
- * @path: The directory to search for, including a slash at the end
- * @data: Pointer to the the cpio archive or a header inside
- * @len: Remaining length of the cpio based on data pointer
- * @offset: When a matching file is found, this is the offset to the
- * beginning of the cpio. It can be used to iterate through
- * the cpio to find all files inside of a directory path
+ * @path: The directory to search for, including a slash at the end
+ * @data: Pointer to the the cpio archive or a header inside
+ * @len: Remaining length of the cpio based on data pointer
+ * @nextoff: When a matching file is found, this is the offset from the
+ * beginning of the cpio to the beginning of the next file, not the
+ * matching file itself. It can be used to iterate through the cpio
+ * to find all files inside of a directory path
*
- * @return: struct cpio_data containing the address, length and
- * filename (with the directory path cut off) of the found file.
- * If you search for a filename and not for files in a directory,
- * pass the absolute path of the filename in the cpio and make sure
- * the match returned an empty filename string.
+ * @return: struct cpio_data containing the address, length and
+ * filename (with the directory path cut off) of the found file.
+ * If you search for a filename and not for files in a directory,
+ * pass the absolute path of the filename in the cpio and make sure
+ * the match returned an empty filename string.
*/

struct cpio_data find_cpio_data(const char *path, void *data,
- size_t len, long *offset)
+ size_t len, long *nextoff)
{
const size_t cpio_header_len = 8*C_NFIELDS - 2;
struct cpio_data cd = { NULL, 0, "" };
@@ -124,7 +125,7 @@ struct cpio_data find_cpio_data(const char *path, void *data,
if ((ch[C_MODE] & 0170000) == 0100000 &&
ch[C_NAMESIZE] >= mypathsize &&
!memcmp(p, path, mypathsize)) {
- *offset = (long)nptr - (long)data;
+ *nextoff = (long)nptr - (long)data;
if (ch[C_NAMESIZE] - mypathsize >= MAX_CPIO_FILE_NAME) {
pr_warn(
"File %s exceeding MAX_CPIO_FILE_NAME [%d]\n",
--
1.7.1

2013-08-14 09:39:21

by Tang Chen

[permalink] [raw]
Subject: [PATCH 1/3] acpi: Print Hot-Pluggable Field in SRAT.

The Hot-Pluggable field in SRAT suggests if the memory could be
hotplugged while the system is running. Print it as well when
parsing SRAT will help users to know which memory is hotpluggable.

Signed-off-by: Tang Chen <[email protected]>
Reviewed-by: Wanpeng Li <[email protected]>
Reviewed-by: Zhang Yanfei <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
arch/x86/mm/srat.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index cdd0da9..266ca91 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -146,6 +146,7 @@ int __init
acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
{
u64 start, end;
+ u32 hotpluggable;
int node, pxm;

if (srat_disabled())
@@ -154,7 +155,8 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
goto out_err_bad_srat;
if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
goto out_err;
- if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
+ hotpluggable = ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE;
+ if (hotpluggable && !save_add_info())
goto out_err;

start = ma->base_address;
@@ -174,9 +176,10 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)

node_set(node, numa_nodes_parsed);

- printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
- node, pxm,
- (unsigned long long) start, (unsigned long long) end - 1);
+ pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]%s\n",
+ node, pxm,
+ (unsigned long long) start, (unsigned long long) end - 1,
+ hotpluggable ? " hotplug" : "");

return 0;
out_err_bad_srat:
--
1.7.1

2013-08-19 23:29:46

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/3] Preparation for arranging hotplug memory in ZONE_MOVABLE.

On Wednesday, August 14, 2013 05:37:05 PM Tang Chen wrote:
> These three trivial patches are from patch-set "Arrange hotpluggable memory as ZONE_MOVABLE."
> And they have been acked and reviewed by many people.
>
> Hope they can be merged first. And the rest patches are coming soon.
>
>
> Tang Chen (3):
> acpi: Print Hot-Pluggable Field in SRAT.
> earlycpio.c: Fix the confusing comment of find_cpio_data().
> acpi: Kill macro INVALID_TABLE().
>
> arch/x86/mm/srat.c | 11 +++++++----
> drivers/acpi/osl.c | 36 ++++++++++++++++++++----------------
> lib/earlycpio.c | 27 ++++++++++++++-------------
> 3 files changed, 41 insertions(+), 33 deletions(-)

I've queued up this series for 3.12.

Thanks,
Rafael

2013-08-20 01:09:22

by Tang Chen

[permalink] [raw]
Subject: Re: [PATCH 0/3] Preparation for arranging hotplug memory in ZONE_MOVABLE.

On 08/20/2013 07:40 AM, Rafael J. Wysocki wrote:
> On Wednesday, August 14, 2013 05:37:05 PM Tang Chen wrote:
>> These three trivial patches are from patch-set "Arrange hotpluggable memory as ZONE_MOVABLE."
>> And they have been acked and reviewed by many people.
>>
>> Hope they can be merged first. And the rest patches are coming soon.
>>
>>
>> Tang Chen (3):
>> acpi: Print Hot-Pluggable Field in SRAT.
>> earlycpio.c: Fix the confusing comment of find_cpio_data().
>> acpi: Kill macro INVALID_TABLE().
>>
>> arch/x86/mm/srat.c | 11 +++++++----
>> drivers/acpi/osl.c | 36 ++++++++++++++++++++----------------
>> lib/earlycpio.c | 27 ++++++++++++++-------------
>> 3 files changed, 41 insertions(+), 33 deletions(-)
>
> I've queued up this series for 3.12.

Hi Rafael,

Thanks a lot. :)