* [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list
@ 2015-07-24 3:11 Pan Xinhui
0 siblings, 0 replies; 4+ messages in thread
From: Pan Xinhui @ 2015-07-24 3:11 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Gleixner, Borislav Petkov, mingo, hpa, x86, bp, Kani,
Toshimitsu, jgross, mcgrof, mnipxh, yanmin_zhang, Elliott,
Robert (Server Storage)
From: Pan Xinhui <xinhuix.pan@intel.com>
There are many nodes in the PAT memtype rb-tree. When we dump this tree
we call kzalloc every time to copy nodes. Actually these kzalloc are not
necessary. Lets do a optimization now.
Let seq_file core create an *entry* when open and free it when release.
*entry* is stored as seq->private, so we can use it in seq_operations
callback who traverse/output the PAT memtype rb-tree only. Lots of
unnecessary kzalloc could be avoided now.
Signed-off-by: Pan Xinhui <xinhuix.pan@intel.com>
---
v1 -> v2:
Use seq_open/release_private. let seq_file core handle memory alloc/free.
seq_operations callbacks now only traverse/output the PAT rb-tree.
---
arch/x86/mm/pat.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 268b2c8..53920be 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -1001,41 +1001,33 @@ EXPORT_SYMBOL_GPL(pgprot_writethrough);
#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_X86_PAT)
-static struct memtype *memtype_get_idx(loff_t pos)
+static struct memtype *memtype_get_idx(struct memtype *entry, loff_t pos)
{
- struct memtype *print_entry;
int ret;
- print_entry = kzalloc(sizeof(struct memtype), GFP_KERNEL);
- if (!print_entry)
- return NULL;
-
spin_lock(&memtype_lock);
- ret = rbt_memtype_copy_nth_element(print_entry, pos);
+ ret = rbt_memtype_copy_nth_element(entry, pos);
spin_unlock(&memtype_lock);
- if (!ret) {
- return print_entry;
- } else {
- kfree(print_entry);
- return NULL;
- }
+ return ret ? NULL : entry;
}
static void *memtype_seq_start(struct seq_file *seq, loff_t *pos)
{
+ struct memtype *entry = seq->private;
+
if (*pos == 0) {
++*pos;
seq_puts(seq, "PAT memtype list:\n");
}
- return memtype_get_idx(*pos);
+ return memtype_get_idx(entry, *pos);
}
static void *memtype_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
++*pos;
- return memtype_get_idx(*pos);
+ return memtype_get_idx((struct memtype *)v, *pos);
}
static void memtype_seq_stop(struct seq_file *seq, void *v)
@@ -1048,7 +1040,6 @@ static int memtype_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "%s @ 0x%Lx-0x%Lx\n", cattr_name(print_entry->type),
print_entry->start, print_entry->end);
- kfree(print_entry);
return 0;
}
@@ -1062,14 +1053,15 @@ static const struct seq_operations memtype_seq_ops = {
static int memtype_seq_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &memtype_seq_ops);
+ return seq_open_private(file, &memtype_seq_ops,
+ sizeof(struct memtype));
}
static const struct file_operations memtype_fops = {
.open = memtype_seq_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = seq_release_private,
};
static int __init pat_memtype_list_init(void)
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list
2015-07-23 14:53 ` Elliott, Robert (Server Storage)
@ 2015-07-24 2:40 ` Pan Xinhui
0 siblings, 0 replies; 4+ messages in thread
From: Pan Xinhui @ 2015-07-24 2:40 UTC (permalink / raw)
To: Elliott, Robert (Server Storage), linux-kernel
Cc: Thomas Gleixner, Borislav Petkov, mingo, hpa, x86, bp, Kani,
Toshimitsu, jgross, mcgrof, mnipxh, yanmin_zhang
hi, Elliott
thanks for your reply. :)
On 2015年07月23日 22:53, Elliott, Robert (Server Storage) wrote:
>> -----Original Message-----
>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>> owner@vger.kernel.org] On Behalf Of Pan Xinhui
>> Sent: Thursday, July 23, 2015 4:54 AM
>> To: linux-kernel@vger.kernel.org
>> Subject: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype
>> list
> ...
>> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
>> index 268b2c8..6302119 100644
>> --- a/arch/x86/mm/pat.c
>> +++ b/arch/x86/mm/pat.c
>> @@ -1001,45 +1001,42 @@ EXPORT_SYMBOL_GPL(pgprot_writethrough);
>>
>> #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_X86_PAT)
>>
>> -static struct memtype *memtype_get_idx(loff_t pos)
>> +static struct memtype *memtype_get_idx(struct memtype *entry, loff_t pos)
>> {
>> - struct memtype *print_entry;
>> int ret;
>>
>> - print_entry = kzalloc(sizeof(struct memtype), GFP_KERNEL);
>> - if (!print_entry)
>> - return NULL;
>> -
>> spin_lock(&memtype_lock);
>> - ret = rbt_memtype_copy_nth_element(print_entry, pos);
>> + ret = rbt_memtype_copy_nth_element(entry, pos);
>> spin_unlock(&memtype_lock);
>>
>> - if (!ret) {
>> - return print_entry;
>> - } else {
>> - kfree(print_entry);
>> - return NULL;
>> - }
>> + return ret ? NULL : entry;
>> }
>>
> ...
>> static void memtype_seq_stop(struct seq_file *seq, void *v)
>> {
>> + kfree(seq->private);
>> }
>>
>
> Consider adding
> seq->private = NULL;
> so the stale pointer isn't left around. There's probably not
> much risk of accessing it, but NULL is safer in case it is.
>
I know exactly what are you worrying about.
start and stop callback is still not the best place to alloc/free the *entry*. you are worrying about that some codes might touch it.
seq_file.c has offered seq_open_private and seq_release_private. we can make use of them. :)
I will work out patch v2, this time I will Cc you. I am very happy if you could review it at any free time. :)
thanks
xinhui
> ---
> Robert Elliott, HP Server Storage
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list
2015-07-23 9:54 Pan Xinhui
@ 2015-07-23 14:53 ` Elliott, Robert (Server Storage)
2015-07-24 2:40 ` Pan Xinhui
0 siblings, 1 reply; 4+ messages in thread
From: Elliott, Robert (Server Storage) @ 2015-07-23 14:53 UTC (permalink / raw)
To: Pan Xinhui, linux-kernel
Cc: Thomas Gleixner, Borislav Petkov, mingo, hpa, x86, bp, Kani,
Toshimitsu, jgross, mcgrof, mnipxh, yanmin_zhang
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1651 bytes --]
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Pan Xinhui
> Sent: Thursday, July 23, 2015 4:54 AM
> To: linux-kernel@vger.kernel.org
> Subject: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype
> list
...
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index 268b2c8..6302119 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -1001,45 +1001,42 @@ EXPORT_SYMBOL_GPL(pgprot_writethrough);
>
> #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_X86_PAT)
>
> -static struct memtype *memtype_get_idx(loff_t pos)
> +static struct memtype *memtype_get_idx(struct memtype *entry, loff_t pos)
> {
> - struct memtype *print_entry;
> int ret;
>
> - print_entry = kzalloc(sizeof(struct memtype), GFP_KERNEL);
> - if (!print_entry)
> - return NULL;
> -
> spin_lock(&memtype_lock);
> - ret = rbt_memtype_copy_nth_element(print_entry, pos);
> + ret = rbt_memtype_copy_nth_element(entry, pos);
> spin_unlock(&memtype_lock);
>
> - if (!ret) {
> - return print_entry;
> - } else {
> - kfree(print_entry);
> - return NULL;
> - }
> + return ret ? NULL : entry;
> }
>
...
> static void memtype_seq_stop(struct seq_file *seq, void *v)
> {
> + kfree(seq->private);
> }
>
Consider adding
seq->private = NULL;
so the stale pointer isn't left around. There's probably not
much risk of accessing it, but NULL is safer in case it is.
---
Robert Elliott, HP Server Storage
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list
@ 2015-07-23 9:54 Pan Xinhui
2015-07-23 14:53 ` Elliott, Robert (Server Storage)
0 siblings, 1 reply; 4+ messages in thread
From: Pan Xinhui @ 2015-07-23 9:54 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Gleixner, Borislav Petkov, mingo, hpa, x86, bp,
toshi.kani, jgross, mcgrof, mnipxh, yanmin_zhang
From: Pan Xinhui <xinhuix.pan@intel.com>
There are many nodes in the PAT memtype rb-tree. When we dump this tree
we call kzalloc every time to copy nodes. Actually these kzalloc are not
necessary. Lets do a optimization now.
Create an *entry* in memtype_seq_start(), and free it in
memtype_seq_stop(). These two callback functions are alwasys called in
pair. Also because memtype_seq_show() is usually used only for
outputing. Seems memtype_seq_show is not the best place to free the
*entry*
And reuse the *entry* in memtype_seq_next() to enhance the performance.
Signed-off-by: Pan Xinhui <xinhuix.pan@intel.com>
---
arch/x86/mm/pat.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 268b2c8..6302119 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -1001,45 +1001,42 @@ EXPORT_SYMBOL_GPL(pgprot_writethrough);
#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_X86_PAT)
-static struct memtype *memtype_get_idx(loff_t pos)
+static struct memtype *memtype_get_idx(struct memtype *entry, loff_t pos)
{
- struct memtype *print_entry;
int ret;
- print_entry = kzalloc(sizeof(struct memtype), GFP_KERNEL);
- if (!print_entry)
- return NULL;
-
spin_lock(&memtype_lock);
- ret = rbt_memtype_copy_nth_element(print_entry, pos);
+ ret = rbt_memtype_copy_nth_element(entry, pos);
spin_unlock(&memtype_lock);
- if (!ret) {
- return print_entry;
- } else {
- kfree(print_entry);
- return NULL;
- }
+ return ret ? NULL : entry;
}
static void *memtype_seq_start(struct seq_file *seq, loff_t *pos)
{
+ struct memtype *entry;
+
if (*pos == 0) {
++*pos;
seq_puts(seq, "PAT memtype list:\n");
}
- return memtype_get_idx(*pos);
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ seq->private = entry;
+ if (!entry)
+ return NULL;
+ return memtype_get_idx(entry, *pos);
}
static void *memtype_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
++*pos;
- return memtype_get_idx(*pos);
+ return memtype_get_idx((struct memtype *)v, *pos);
}
static void memtype_seq_stop(struct seq_file *seq, void *v)
{
+ kfree(seq->private);
}
static int memtype_seq_show(struct seq_file *seq, void *v)
@@ -1048,7 +1045,6 @@ static int memtype_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "%s @ 0x%Lx-0x%Lx\n", cattr_name(print_entry->type),
print_entry->start, print_entry->end);
- kfree(print_entry);
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-24 3:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 3:11 [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list Pan Xinhui
-- strict thread matches above, loose matches on Subject: below --
2015-07-23 9:54 Pan Xinhui
2015-07-23 14:53 ` Elliott, Robert (Server Storage)
2015-07-24 2:40 ` Pan Xinhui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).