x86/mm/pat: Do a small optimization when dump PAT memtype list
PATCH 1/1 x86/mm/pat: Do a small optimization when dump PAT memtype list
From: Pan Xinhui <[email protected]>
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.
--- arch/x86/mm/pat.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)
arch/x86/mm/pat.c +12 -16
@@ -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 ; }
Signature · 1 line
1.9.1
Quoted 7 lines
-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Pan Xinhui
Sent: Thursday, July 23, 2015 4:54 AM
To: [email protected]
Subject: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list
...
Quoted 32 lines
arch/x86/mm/pat.c +3
-13@@ -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; }
...
arch/x86/mm/pat.c EXPORT_SYMBOL_GPL(pgprot_writethrough); · line 1040
Quoted 5 lines
static void
memtype_seq_stop(struct seq_file *seq, void *v){arch/x86/mm/pat.c +1
-0+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�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
hi, Elliott
thanks for your reply. :)
Quoted 47 lines · Robert (Server Storage)
Quoted 7 lines
-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Pan Xinhui
Sent: Thursday, July 23, 2015 4:54 AM
To: [email protected]
Subject: [PATCH] x86/mm/pat: Do a small optimization when dump PAT memtype list...
Quoted 32 lines
arch/x86/mm/pat.c +3
-13@@ -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; }...
Quoted 5 lines
static void
memtype_seq_stop(struct seq_file *seq, void *v){
+kfree(seq->private);
}
Quoted 5 lines
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
Quoted 3 lines
---
Robert Elliott, HP Server Storage