* [PATCH v2] fs: use kmalloc() to allocate fdmem if possible
@ 2010-05-02 19:02 Changli Gao
2010-05-04 22:27 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Changli Gao @ 2010-05-02 19:02 UTC (permalink / raw
To: Alexander Viro
Cc: Jiri Slaby, Andrew Morton, Paul E. McKenney, Alexey Dobriyan,
Ingo Molnar, Peter Zijlstra, linux-fsdevel, linux-kernel,
Avi Kivity, Tetsuo Handa, Changli Gao
use kmalloc() to allocate fdmem if possible.
vmalloc() is used as a fallback solution for fdmem allocation. A new helper
function __free_fdtable() is introduced to reduce the lines of code.
A potential bug, vfree() a memory allocated by kmalloc(), is fixed.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
fs/file.c | 55 ++++++++++++++++++++++---------------------------------
1 file changed, 22 insertions(+), 33 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index 34bb7f7..9af31ae 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -39,28 +39,27 @@ int sysctl_nr_open_max = 1024 * 1024; /* raised later */
*/
static DEFINE_PER_CPU(struct fdtable_defer, fdtable_defer_list);
-static inline void * alloc_fdmem(unsigned int size)
+static inline void *alloc_fdmem(unsigned int size)
{
- if (size <= PAGE_SIZE)
- return kmalloc(size, GFP_KERNEL);
- else
- return vmalloc(size);
+ void *data;
+
+ data = kmalloc(size, GFP_KERNEL);
+ if (data != NULL)
+ return data;
+
+ return vmalloc(size);
}
-static inline void free_fdarr(struct fdtable *fdt)
+static inline void free_fdmem(void *ptr)
{
- if (fdt->max_fds <= (PAGE_SIZE / sizeof(struct file *)))
- kfree(fdt->fd);
- else
- vfree(fdt->fd);
+ is_vmalloc_addr(ptr) ? vfree(ptr) : kfree(ptr);
}
-static inline void free_fdset(struct fdtable *fdt)
+static inline void __free_fdtable(struct fdtable *fdt)
{
- if (fdt->max_fds <= (PAGE_SIZE * BITS_PER_BYTE / 2))
- kfree(fdt->open_fds);
- else
- vfree(fdt->open_fds);
+ free_fdmem(fdt->fd);
+ free_fdmem(fdt->open_fds);
+ kfree(fdt);
}
static void free_fdtable_work(struct work_struct *work)
@@ -75,9 +74,8 @@ static void free_fdtable_work(struct work_struct *work)
spin_unlock_bh(&f->lock);
while(fdt) {
struct fdtable *next = fdt->next;
- vfree(fdt->fd);
- free_fdset(fdt);
- kfree(fdt);
+
+ __free_fdtable(fdt);
fdt = next;
}
}
@@ -184,7 +182,7 @@ static struct fdtable * alloc_fdtable(unsigned int nr)
return fdt;
out_arr:
- free_fdarr(fdt);
+ free_fdmem(fdt->fd);
out_fdt:
kfree(fdt);
out:
@@ -214,9 +212,7 @@ static int expand_fdtable(struct files_struct *files, int nr)
* caller and alloc_fdtable(). Cheaper to catch it here...
*/
if (unlikely(new_fdt->max_fds <= nr)) {
- free_fdarr(new_fdt);
- free_fdset(new_fdt);
- kfree(new_fdt);
+ __free_fdtable(new_fdt);
return -EMFILE;
}
/*
@@ -232,9 +228,7 @@ static int expand_fdtable(struct files_struct *files, int nr)
free_fdtable(cur_fdt);
} else {
/* Somebody else expanded, so undo our attempt */
- free_fdarr(new_fdt);
- free_fdset(new_fdt);
- kfree(new_fdt);
+ __free_fdtable(new_fdt);
}
return 1;
}
@@ -325,11 +319,8 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp)
while (unlikely(open_files > new_fdt->max_fds)) {
spin_unlock(&oldf->file_lock);
- if (new_fdt != &newf->fdtab) {
- free_fdarr(new_fdt);
- free_fdset(new_fdt);
- kfree(new_fdt);
- }
+ if (new_fdt != &newf->fdtab)
+ __free_fdtable(new_fdt);
new_fdt = alloc_fdtable(open_files - 1);
if (!new_fdt) {
@@ -339,9 +330,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp)
/* beyond sysctl_nr_open; nothing to do */
if (unlikely(new_fdt->max_fds < open_files)) {
- free_fdarr(new_fdt);
- free_fdset(new_fdt);
- kfree(new_fdt);
+ __free_fdtable(new_fdt);
*errorp = -EMFILE;
goto out_release;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] fs: use kmalloc() to allocate fdmem if possible
2010-05-02 19:02 [PATCH v2] fs: use kmalloc() to allocate fdmem if possible Changli Gao
@ 2010-05-04 22:27 ` Andrew Morton
2010-05-05 7:18 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2010-05-04 22:27 UTC (permalink / raw
To: Changli Gao
Cc: Alexander Viro, Jiri Slaby, Paul E. McKenney, Alexey Dobriyan,
Ingo Molnar, Peter Zijlstra, linux-fsdevel, linux-kernel,
Avi Kivity, Tetsuo Handa
On Mon, 3 May 2010 03:02:00 +0800
Changli Gao <xiaosuo@gmail.com> wrote:
> use kmalloc() to allocate fdmem if possible.
>
> vmalloc() is used as a fallback solution for fdmem allocation. A new helper
> function __free_fdtable() is introduced to reduce the lines of code.
>
> A potential bug, vfree() a memory allocated by kmalloc(), is fixed.
>
Seems a reasonable thing to do. It might also be reasonable to make
vmalloc() try kmalloc() first, but that's a separate exercise.
> ----
> fs/file.c | 55 ++++++++++++++++++++++---------------------------------
> 1 file changed, 22 insertions(+), 33 deletions(-)
> diff --git a/fs/file.c b/fs/file.c
> index 34bb7f7..9af31ae 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -39,28 +39,27 @@ int sysctl_nr_open_max = 1024 * 1024; /* raised later */
> */
> static DEFINE_PER_CPU(struct fdtable_defer, fdtable_defer_list);
>
> -static inline void * alloc_fdmem(unsigned int size)
> +static inline void *alloc_fdmem(unsigned int size)
> {
> - if (size <= PAGE_SIZE)
> - return kmalloc(size, GFP_KERNEL);
> - else
> - return vmalloc(size);
> + void *data;
> +
> + data = kmalloc(size, GFP_KERNEL);
This most definitely should have __GFP_NOWARN.
> + if (data != NULL)
> + return data;
> +
> + return vmalloc(size);
> }
>
> -static inline void free_fdarr(struct fdtable *fdt)
> +static inline void free_fdmem(void *ptr)
> {
> - if (fdt->max_fds <= (PAGE_SIZE / sizeof(struct file *)))
> - kfree(fdt->fd);
> - else
> - vfree(fdt->fd);
> + is_vmalloc_addr(ptr) ? vfree(ptr) : kfree(ptr);
> }
>
> -static inline void free_fdset(struct fdtable *fdt)
> +static inline void __free_fdtable(struct fdtable *fdt)
> {
> - if (fdt->max_fds <= (PAGE_SIZE * BITS_PER_BYTE / 2))
> - kfree(fdt->open_fds);
> - else
> - vfree(fdt->open_fds);
> + free_fdmem(fdt->fd);
> + free_fdmem(fdt->open_fds);
> + kfree(fdt);
> }
And these should be uninlined - they're too large to be inlined.
My version of gcc seems to just uninline them anyway, but forcing them
to be inlined with __always_inline indeed causes 70-80 bytes more text,
and we figure that larger text generally causes a slower kernel due to
cache eviction effects.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] fs: use kmalloc() to allocate fdmem if possible
2010-05-04 22:27 ` Andrew Morton
@ 2010-05-05 7:18 ` Jiri Slaby
2010-05-05 7:33 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2010-05-05 7:18 UTC (permalink / raw
To: Andrew Morton
Cc: Changli Gao, Alexander Viro, Paul E. McKenney, Alexey Dobriyan,
Ingo Molnar, Peter Zijlstra, linux-fsdevel, linux-kernel,
Avi Kivity, Tetsuo Handa, eric.dumazet
On 05/05/2010 12:27 AM, Andrew Morton wrote:
> On Mon, 3 May 2010 03:02:00 +0800
> Changli Gao <xiaosuo@gmail.com> wrote:
>
>> use kmalloc() to allocate fdmem if possible.
>>
>> vmalloc() is used as a fallback solution for fdmem allocation. A new helper
>> function __free_fdtable() is introduced to reduce the lines of code.
>>
>> A potential bug, vfree() a memory allocated by kmalloc(), is fixed.
>>
>
> Seems a reasonable thing to do.
Hi,
So didn't we converge to something like this:
http://lkml.org/lkml/2010/5/3/53
instead of this patch?
This
> It might also be reasonable to make
> vmalloc() try kmalloc() first, but that's a separate exercise.
and this
> This most definitely should have __GFP_NOWARN.
is discussed there too btw.
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] fs: use kmalloc() to allocate fdmem if possible
2010-05-05 7:18 ` Jiri Slaby
@ 2010-05-05 7:33 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2010-05-05 7:33 UTC (permalink / raw
To: Jiri Slaby
Cc: Andrew Morton, Changli Gao, Alexander Viro, Paul E. McKenney,
Alexey Dobriyan, Ingo Molnar, Peter Zijlstra, linux-fsdevel,
linux-kernel, Avi Kivity, Tetsuo Handa
Le mercredi 05 mai 2010 à 09:18 +0200, Jiri Slaby a écrit :
> On 05/05/2010 12:27 AM, Andrew Morton wrote:
> > On Mon, 3 May 2010 03:02:00 +0800
> > Changli Gao <xiaosuo@gmail.com> wrote:
> >
> >> use kmalloc() to allocate fdmem if possible.
> >>
> >> vmalloc() is used as a fallback solution for fdmem allocation. A new helper
> >> function __free_fdtable() is introduced to reduce the lines of code.
> >>
> >> A potential bug, vfree() a memory allocated by kmalloc(), is fixed.
> >>
> >
> > Seems a reasonable thing to do.
>
> Hi,
>
> So didn't we converge to something like this:
> http://lkml.org/lkml/2010/5/3/53
> instead of this patch?
>
> This
>
> > It might also be reasonable to make
> > vmalloc() try kmalloc() first, but that's a separate exercise.
>
> and this
>
> > This most definitely should have __GFP_NOWARN.
>
> is discussed there too btw.
>
> thanks,
Its a start, I guess we also need some NUMA capability as well.
For example, iptables could use such an allocator only if it is NUMA
enabled.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] fs: use kmalloc() to allocate fdmem if possible
@ 2010-05-05 7:33 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2010-05-05 7:33 UTC (permalink / raw
To: Jiri Slaby
Cc: Andrew Morton, Changli Gao, Alexander Viro, Paul E. McKenney,
Alexey Dobriyan, Ingo Molnar, Peter Zijlstra, linux-fsdevel,
linux-kernel, Avi Kivity, Tetsuo Handa
Le mercredi 05 mai 2010 à 09:18 +0200, Jiri Slaby a écrit :
> On 05/05/2010 12:27 AM, Andrew Morton wrote:
> > On Mon, 3 May 2010 03:02:00 +0800
> > Changli Gao <xiaosuo@gmail.com> wrote:
> >
> >> use kmalloc() to allocate fdmem if possible.
> >>
> >> vmalloc() is used as a fallback solution for fdmem allocation. A new helper
> >> function __free_fdtable() is introduced to reduce the lines of code.
> >>
> >> A potential bug, vfree() a memory allocated by kmalloc(), is fixed.
> >>
> >
> > Seems a reasonable thing to do.
>
> Hi,
>
> So didn't we converge to something like this:
> http://lkml.org/lkml/2010/5/3/53
> instead of this patch?
>
> This
>
> > It might also be reasonable to make
> > vmalloc() try kmalloc() first, but that's a separate exercise.
>
> and this
>
> > This most definitely should have __GFP_NOWARN.
>
> is discussed there too btw.
>
> thanks,
Its a start, I guess we also need some NUMA capability as well.
For example, iptables could use such an allocator only if it is NUMA
enabled.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-05 7:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-02 19:02 [PATCH v2] fs: use kmalloc() to allocate fdmem if possible Changli Gao
2010-05-04 22:27 ` Andrew Morton
2010-05-05 7:18 ` Jiri Slaby
2010-05-05 7:33 ` Eric Dumazet
2010-05-05 7:33 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.