* [PATCH] fs: use kmalloc() to allocate fdmem if possible
[not found] <Alexander Viro <viro@zeniv.linux.org.uk>
@ 2010-05-02 16:46 ` Changli Gao
2010-05-02 17:31 ` Avi Kivity
0 siblings, 1 reply; 18+ messages in thread
From: Changli Gao @ 2010-05-02 16:46 UTC (permalink / raw)
Cc: Jiri Slaby, Andrew Morton, Paul E. McKenney, Alexey Dobriyan,
Ingo Molnar, Peter Zijlstra, linux-fsdevel, linux-kernel,
Changli Gao
use kmalloc() to allocate fdmem if possible.
vmalloc() is used as a fallback solution for fdmem allocation. Two members are
added into the hole of the structure fdtable to indicate if vmalloc() is used
or not.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
fs/file.c | 33 ++++++++++++++++++++-------------
include/linux/fdtable.h | 2 ++
2 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index 34bb7f7..f71dd85 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -39,28 +39,34 @@ 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, unsigned short *use_vmalloc)
{
- if (size <= PAGE_SIZE)
- return kmalloc(size, GFP_KERNEL);
- else
- return vmalloc(size);
+ void *data;
+
+ data = kmalloc(size, GFP_KERNEL);
+ if (data != NULL) {
+ *use_vmalloc = 0;
+ return data;
+ }
+ *use_vmalloc = 1;
+
+ return vmalloc(size);
}
static inline void free_fdarr(struct fdtable *fdt)
{
- if (fdt->max_fds <= (PAGE_SIZE / sizeof(struct file *)))
- kfree(fdt->fd);
- else
+ if (fdt->fdarr_vmalloc)
vfree(fdt->fd);
+ else
+ kfree(fdt->fd);
}
static inline void free_fdset(struct fdtable *fdt)
{
- if (fdt->max_fds <= (PAGE_SIZE * BITS_PER_BYTE / 2))
- kfree(fdt->open_fds);
- else
+ if (fdt->fdset_vmalloc)
vfree(fdt->open_fds);
+ else
+ kfree(fdt->open_fds);
}
static void free_fdtable_work(struct work_struct *work)
@@ -167,12 +173,13 @@ static struct fdtable * alloc_fdtable(unsigned int nr)
if (!fdt)
goto out;
fdt->max_fds = nr;
- data = alloc_fdmem(nr * sizeof(struct file *));
+ data = alloc_fdmem(nr * sizeof(struct file *), &fdt->fdarr_vmalloc);
if (!data)
goto out_fdt;
fdt->fd = (struct file **)data;
data = alloc_fdmem(max_t(unsigned int,
- 2 * nr / BITS_PER_BYTE, L1_CACHE_BYTES));
+ 2 * nr / BITS_PER_BYTE, L1_CACHE_BYTES),
+ &fdt->fdset_vmalloc);
if (!data)
goto out_arr;
fdt->open_fds = (fd_set *)data;
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 013dc52..1e730bc 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -30,6 +30,8 @@ struct embedded_fd_set {
struct fdtable {
unsigned int max_fds;
+ unsigned short fdarr_vmalloc;
+ unsigned short fdset_vmalloc;
struct file ** fd; /* current fd array */
fd_set *close_on_exec;
fd_set *open_fds;
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-02 16:46 ` [PATCH] fs: use kmalloc() to allocate fdmem if possible Changli Gao
@ 2010-05-02 17:31 ` Avi Kivity
2010-05-03 0:15 ` Tetsuo Handa
0 siblings, 1 reply; 18+ messages in thread
From: Avi Kivity @ 2010-05-02 17:31 UTC (permalink / raw)
To: Changli Gao
Cc: Jiri Slaby, Andrew Morton, Paul E. McKenney, Alexey Dobriyan,
Ingo Molnar, Peter Zijlstra, linux-fsdevel, linux-kernel
On 05/02/2010 07:46 PM, Changli Gao wrote:
> use kmalloc() to allocate fdmem if possible.
>
> vmalloc() is used as a fallback solution for fdmem allocation. Two members are
> added into the hole of the structure fdtable to indicate if vmalloc() is used
> or not.
>
>
> diff --git a/fs/file.c b/fs/file.c
> index 34bb7f7..f71dd85 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -39,28 +39,34 @@ 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, unsigned short *use_vmalloc)
> {
> - if (size<= PAGE_SIZE)
> - return kmalloc(size, GFP_KERNEL);
> - else
> - return vmalloc(size);
> + void *data;
> +
> + data = kmalloc(size, GFP_KERNEL);
> + if (data != NULL) {
> + *use_vmalloc = 0;
> + return data;
> + }
> + *use_vmalloc = 1;
> +
> + return vmalloc(size);
> }
>
Perhaps vmalloc() should do this by itself? vfree() can examine the
pointer and call kfree() if it isn't within the vmalloc address range.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-02 17:31 ` Avi Kivity
@ 2010-05-03 0:15 ` Tetsuo Handa
2010-05-03 6:06 ` Avi Kivity
0 siblings, 1 reply; 18+ messages in thread
From: Tetsuo Handa @ 2010-05-03 0:15 UTC (permalink / raw)
To: avi, xiaosuo
Cc: jslaby, akpm, paulmck, adobriyan, mingo, peterz, linux-fsdevel,
linux-kernel
Avi Kivity wrote:
> On 05/02/2010 07:46 PM, Changli Gao wrote:
> > use kmalloc() to allocate fdmem if possible.
> >
> > vmalloc() is used as a fallback solution for fdmem allocation. Two members are
> > added into the hole of the structure fdtable to indicate if vmalloc() is used
> > or not.
> >
> >
> > diff --git a/fs/file.c b/fs/file.c
> > index 34bb7f7..f71dd85 100644
> > --- a/fs/file.c
> > +++ b/fs/file.c
> > @@ -39,28 +39,34 @@ 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, unsigned short *use_vmalloc)
> > {
> > - if (size<= PAGE_SIZE)
> > - return kmalloc(size, GFP_KERNEL);
> > - else
> > - return vmalloc(size);
> > + void *data;
> > +
> > + data = kmalloc(size, GFP_KERNEL);
> > + if (data != NULL) {
> > + *use_vmalloc = 0;
> > + return data;
> > + }
> > + *use_vmalloc = 1;
> > +
> > + return vmalloc(size);
> > }
> >
>
> Perhaps vmalloc() should do this by itself? vfree() can examine the
> pointer and call kfree() if it isn't within the vmalloc address range.
You can use is_vmalloc_addr().
if (is_vmalloc_addr(p))
vfree(p);
else
kfree(p);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 0:15 ` Tetsuo Handa
@ 2010-05-03 6:06 ` Avi Kivity
2010-05-03 7:05 ` Changli Gao
0 siblings, 1 reply; 18+ messages in thread
From: Avi Kivity @ 2010-05-03 6:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: xiaosuo, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
On 05/03/2010 03:15 AM, Tetsuo Handa wrote:
> Avi Kivity wrote:
>
>> On 05/02/2010 07:46 PM, Changli Gao wrote:
>>
>>> use kmalloc() to allocate fdmem if possible.
>>>
>>> vmalloc() is used as a fallback solution for fdmem allocation. Two members are
>>> added into the hole of the structure fdtable to indicate if vmalloc() is used
>>> or not.
>>>
>>>
>>> diff --git a/fs/file.c b/fs/file.c
>>> index 34bb7f7..f71dd85 100644
>>> --- a/fs/file.c
>>> +++ b/fs/file.c
>>> @@ -39,28 +39,34 @@ 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, unsigned short *use_vmalloc)
>>> {
>>> - if (size<= PAGE_SIZE)
>>> - return kmalloc(size, GFP_KERNEL);
>>> - else
>>> - return vmalloc(size);
>>> + void *data;
>>> +
>>> + data = kmalloc(size, GFP_KERNEL);
>>> + if (data != NULL) {
>>> + *use_vmalloc = 0;
>>> + return data;
>>> + }
>>> + *use_vmalloc = 1;
>>> +
>>> + return vmalloc(size);
>>> }
>>>
>>>
>> Perhaps vmalloc() should do this by itself? vfree() can examine the
>> pointer and call kfree() if it isn't within the vmalloc address range.
>>
> You can use is_vmalloc_addr().
>
> if (is_vmalloc_addr(p))
> vfree(p);
> else
> kfree(p);
>
My point is, vmalloc() and vfree should do this, not their callers:
vmalloc(size):
if (size <= PAGE_SIZE)
return kmalloc(size, GFP_KERNEL);
...
vfree(p):
if (!is_vmalloc_addr(p) {
kfree(p);
return;
}
...
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 6:06 ` Avi Kivity
@ 2010-05-03 7:05 ` Changli Gao
2010-05-03 7:42 ` Avi Kivity
0 siblings, 1 reply; 18+ messages in thread
From: Changli Gao @ 2010-05-03 7:05 UTC (permalink / raw)
To: Avi Kivity
Cc: Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
On Mon, May 3, 2010 at 2:06 PM, Avi Kivity <avi@redhat.com> wrote:
>
> My point is, vmalloc() and vfree should do this, not their callers:
>
> vmalloc(size):
> if (size <= PAGE_SIZE)
> return kmalloc(size, GFP_KERNEL);
> ...
>
> vfree(p):
> if (!is_vmalloc_addr(p) {
> kfree(p);
> return;
> }
> ...
I think we should not change vmalloc/vfree, and you can invent new
memory APIs, such as malloc()/free().
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
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] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 7:05 ` Changli Gao
@ 2010-05-03 7:42 ` Avi Kivity
2010-05-03 7:52 ` Changli Gao
0 siblings, 1 reply; 18+ messages in thread
From: Avi Kivity @ 2010-05-03 7:42 UTC (permalink / raw)
To: Changli Gao
Cc: Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
On 05/03/2010 10:05 AM, Changli Gao wrote:
> On Mon, May 3, 2010 at 2:06 PM, Avi Kivity<avi@redhat.com> wrote:
>
>> My point is, vmalloc() and vfree should do this, not their callers:
>>
>> vmalloc(size):
>> if (size<= PAGE_SIZE)
>> return kmalloc(size, GFP_KERNEL);
>> ...
>>
>> vfree(p):
>> if (!is_vmalloc_addr(p) {
>> kfree(p);
>> return;
>> }
>> ...
>>
> I think we should not change vmalloc/vfree, and you can invent new
> memory APIs, such as malloc()/free().
>
Why?
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 7:42 ` Avi Kivity
@ 2010-05-03 7:52 ` Changli Gao
2010-05-03 8:49 ` Avi Kivity
0 siblings, 1 reply; 18+ messages in thread
From: Changli Gao @ 2010-05-03 7:52 UTC (permalink / raw)
To: Avi Kivity
Cc: Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
On Mon, May 3, 2010 at 3:42 PM, Avi Kivity <avi@redhat.com> wrote:
> On 05/03/2010 10:05 AM, Changli Gao wrote:
>>
>> On Mon, May 3, 2010 at 2:06 PM, Avi Kivity<avi@redhat.com> wrote:
>>
>>>
>>> My point is, vmalloc() and vfree should do this, not their callers:
>>>
>>> vmalloc(size):
>>> if (size<= PAGE_SIZE)
>>> return kmalloc(size, GFP_KERNEL);
>>> ...
>>>
>>> vfree(p):
>>> if (!is_vmalloc_addr(p) {
>>> kfree(p);
>>> return;
>>> }
>>> ...
>>>
>>
>> I think we should not change vmalloc/vfree, and you can invent new
>> memory APIs, such as malloc()/free().
>>
>
> Why?
>
Because vmalloc is used to allocate virtually contiguous memory. v in
vmalloc means virtually.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
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] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 7:52 ` Changli Gao
@ 2010-05-03 8:49 ` Avi Kivity
2010-05-03 9:03 ` Jiri Slaby
2010-05-03 9:13 ` Eric Dumazet
0 siblings, 2 replies; 18+ messages in thread
From: Avi Kivity @ 2010-05-03 8:49 UTC (permalink / raw)
To: Changli Gao
Cc: Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
On 05/03/2010 10:52 AM, Changli Gao wrote:
> On Mon, May 3, 2010 at 3:42 PM, Avi Kivity<avi@redhat.com> wrote:
>
>> On 05/03/2010 10:05 AM, Changli Gao wrote:
>>
>>> On Mon, May 3, 2010 at 2:06 PM, Avi Kivity<avi@redhat.com> wrote:
>>>
>>>
>>>> My point is, vmalloc() and vfree should do this, not their callers:
>>>>
>>>> vmalloc(size):
>>>> if (size<= PAGE_SIZE)
>>>> return kmalloc(size, GFP_KERNEL);
>>>> ...
>>>>
>>>> vfree(p):
>>>> if (!is_vmalloc_addr(p) {
>>>> kfree(p);
>>>> return;
>>>> }
>>>> ...
>>>>
>>>>
>>> I think we should not change vmalloc/vfree, and you can invent new
>>> memory APIs, such as malloc()/free().
>>>
>>>
>> Why?
>>
>>
> Because vmalloc is used to allocate virtually contiguous memory. v in
> vmalloc means virtually.
>
>
A kmalloc()ed page is virtually contiguous, satisfying your requirement.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 8:49 ` Avi Kivity
@ 2010-05-03 9:03 ` Jiri Slaby
2010-05-03 9:18 ` Avi Kivity
2010-05-03 9:13 ` Eric Dumazet
1 sibling, 1 reply; 18+ messages in thread
From: Jiri Slaby @ 2010-05-03 9:03 UTC (permalink / raw)
To: Avi Kivity
Cc: Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
On 05/03/2010 10:49 AM, Avi Kivity wrote:
> On 05/03/2010 10:52 AM, Changli Gao wrote:
>> On Mon, May 3, 2010 at 3:42 PM, Avi Kivity<avi@redhat.com> wrote:
>>
>>> On 05/03/2010 10:05 AM, Changli Gao wrote:
>>>
>>>> On Mon, May 3, 2010 at 2:06 PM, Avi Kivity<avi@redhat.com> wrote:
>>>>
>>>>
>>>>> My point is, vmalloc() and vfree should do this, not their callers:
>>>>>
>>>>> vmalloc(size):
>>>>> if (size<= PAGE_SIZE)
>>>>> return kmalloc(size, GFP_KERNEL);
>>>>> ...
>>>>>
>>>>> vfree(p):
>>>>> if (!is_vmalloc_addr(p) {
>>>>> kfree(p);
>>>>> return;
>>>>> }
>>>>> ...
>>>>>
>>>>>
>>>> I think we should not change vmalloc/vfree, and you can invent new
>>>> memory APIs, such as malloc()/free().
>>>>
>>>>
>>> Why?
>>>
>>>
>> Because vmalloc is used to allocate virtually contiguous memory. v in
>> vmalloc means virtually.
>>
>>
>
> A kmalloc()ed page is virtually contiguous, satisfying your requirement.
But it won't work well for vmalloc_to_{page,pfn} and similar. Some code
may expect vmalloc result to be in the vmalloc area and page-aligned
(both in position and size).
Not that it won't be possible to inspect the callers, but in my eyes it
would definitely be better to introduce kmalloc_or_vmalloc-alike where
the caller explicitly doesn't care about the resulting position and size.
regards,
--
js
suse labs
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 8:49 ` Avi Kivity
2010-05-03 9:03 ` Jiri Slaby
@ 2010-05-03 9:13 ` Eric Dumazet
2010-05-03 9:19 ` Jiri Slaby
2010-05-04 4:20 ` Changli Gao
1 sibling, 2 replies; 18+ messages in thread
From: Eric Dumazet @ 2010-05-03 9:13 UTC (permalink / raw)
To: Avi Kivity, Changli Gao
Cc: Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo, peterz,
linux-fsdevel, linux-kernel
Le lundi 03 mai 2010 à 11:49 +0300, Avi Kivity a écrit :
> A kmalloc()ed page is virtually contiguous, satisfying your requirement.
>
Still, a different function pair is needed, at least for documentation
reasons.
Changli, _many_ patches like yours were suggested and refused in the
past for the reason it should be generic. Maybe its time...
So first introduce a pair of funtions, then we can use them.
At free time we can use is_vmalloc_addr() so that no extra bit is needed
to tell if a zone was vmalloced() or kmalloced()
One remark :
+ data = kmalloc(size, GFP_KERNEL);
If allocation fails (because of fragmentation), we would have a kernel
log.
Still, vmalloc() might be OK.
So you should add a __GFP_NOWARN flag.
/*
* Warning: if size is not a power of two, kmalloc() might
* waste memory because of its requirements.
*/
void *kvmalloc(size_t size)
{
void *ptr = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
if (ptr)
return ptr;
return vmalloc(size);
}
void kvfree(void *ptr)
{
BUG_ON(in_interrupt());
if (is_vmalloc_addr(ptr))
vfree(ptr);
else
kfree(ptr);
}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:03 ` Jiri Slaby
@ 2010-05-03 9:18 ` Avi Kivity
2010-05-03 9:22 ` Jiri Slaby
0 siblings, 1 reply; 18+ messages in thread
From: Avi Kivity @ 2010-05-03 9:18 UTC (permalink / raw)
To: Jiri Slaby
Cc: Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
On 05/03/2010 12:03 PM, Jiri Slaby wrote:
>
>
>>> Because vmalloc is used to allocate virtually contiguous memory. v in
>>> vmalloc means virtually.
>>>
>>>
>>>
>> A kmalloc()ed page is virtually contiguous, satisfying your requirement.
>>
> But it won't work well for vmalloc_to_{page,pfn} and similar.
Modify vmalloc_to_{page,pfn} accordingly.
> Some code
> may expect vmalloc result to be in the vmalloc area and page-aligned
> (both in position and size).
>
Both would be a bug IMO. vmalloc() follows kmalloc() and malloc() which
only guarantee natural alignment.
> Not that it won't be possible to inspect the callers, but in my eyes it
> would definitely be better to introduce kmalloc_or_vmalloc-alike where
> the caller explicitly doesn't care about the resulting position and size.
>
You're forcing every user to make a choice about what's essentially a
micro optimization.
If it's really needed to have a vmalloc-in-vmalloc-space, that should be
a special function, while the ordinary vmalloc should make the optimization.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:13 ` Eric Dumazet
@ 2010-05-03 9:19 ` Jiri Slaby
2010-05-03 9:29 ` Eric Dumazet
2010-05-04 4:20 ` Changli Gao
1 sibling, 1 reply; 18+ messages in thread
From: Jiri Slaby @ 2010-05-03 9:19 UTC (permalink / raw)
To: Eric Dumazet
Cc: Avi Kivity, Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan,
mingo, peterz, linux-fsdevel, linux-kernel
On 05/03/2010 11:13 AM, Eric Dumazet wrote:
> /*
> * Warning: if size is not a power of two, kmalloc() might
> * waste memory because of its requirements.
> */
> void *kvmalloc(size_t size)
> {
> void *ptr = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
This is kind of waste of high-order pages, isn't it? Wouldn't it be
better to have a watermark where we skip kmalloc completely?
>
> if (ptr)
> return ptr;
> return vmalloc(size);
> }
--
js
suse labs
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:18 ` Avi Kivity
@ 2010-05-03 9:22 ` Jiri Slaby
2010-05-03 9:25 ` Jiri Slaby
2010-05-03 9:28 ` Avi Kivity
0 siblings, 2 replies; 18+ messages in thread
From: Jiri Slaby @ 2010-05-03 9:22 UTC (permalink / raw)
To: Avi Kivity
Cc: Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
On 05/03/2010 11:18 AM, Avi Kivity wrote:
> On 05/03/2010 12:03 PM, Jiri Slaby wrote:
>>>> Because vmalloc is used to allocate virtually contiguous memory. v in
>>>> vmalloc means virtually.
>>>>
>>>>
>>>>
>>> A kmalloc()ed page is virtually contiguous, satisfying your requirement.
>>>
>> But it won't work well for vmalloc_to_{page,pfn} and similar.
>
> Modify vmalloc_to_{page,pfn} accordingly.
When you get a slub in the middle of page, how?
>> Some code
>> may expect vmalloc result to be in the vmalloc area and page-aligned
>> (both in position and size).
>>
>
> Both would be a bug IMO. vmalloc() follows kmalloc() and malloc() which
> only guarantee natural alignment.
Nope, from what I understand, vmalloc aligns (uses page allocator). Even
for purposes of vmalloc_to_*.
regards,
--
js
suse labs
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:22 ` Jiri Slaby
@ 2010-05-03 9:25 ` Jiri Slaby
2010-05-03 9:28 ` Avi Kivity
1 sibling, 0 replies; 18+ messages in thread
From: Jiri Slaby @ 2010-05-03 9:25 UTC (permalink / raw)
To: Jiri Slaby
Cc: Avi Kivity, Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan,
mingo, peterz, linux-fsdevel, linux-kernel
On 05/03/2010 11:22 AM, Jiri Slaby wrote:
> When you get a slub in the middle of page
What I was thinking about? I meant when you get a piece of memory from
slab allocator which is in the middle of a page.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:22 ` Jiri Slaby
2010-05-03 9:25 ` Jiri Slaby
@ 2010-05-03 9:28 ` Avi Kivity
1 sibling, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2010-05-03 9:28 UTC (permalink / raw)
To: Jiri Slaby
Cc: Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
On 05/03/2010 12:22 PM, Jiri Slaby wrote:
> On 05/03/2010 11:18 AM, Avi Kivity wrote:
>
>> On 05/03/2010 12:03 PM, Jiri Slaby wrote:
>>
>>>>> Because vmalloc is used to allocate virtually contiguous memory. v in
>>>>> vmalloc means virtually.
>>>>>
>>>>>
>>>>>
>>>>>
>>>> A kmalloc()ed page is virtually contiguous, satisfying your requirement.
>>>>
>>>>
>>> But it won't work well for vmalloc_to_{page,pfn} and similar.
>>>
>> Modify vmalloc_to_{page,pfn} accordingly.
>>
> When you get a slub in the middle of page, how?
>
Er, right. So a separate API is needed for users who are interested in
memory, not pages.
>>> Some code
>>> may expect vmalloc result to be in the vmalloc area and page-aligned
>>> (both in position and size).
>>>
>>>
>> Both would be a bug IMO. vmalloc() follows kmalloc() and malloc() which
>> only guarantee natural alignment.
>>
> Nope, from what I understand, vmalloc aligns (uses page allocator). Even
> for purposes of vmalloc_to_*.
>
Correct, my mistake.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:19 ` Jiri Slaby
@ 2010-05-03 9:29 ` Eric Dumazet
0 siblings, 0 replies; 18+ messages in thread
From: Eric Dumazet @ 2010-05-03 9:29 UTC (permalink / raw)
To: Jiri Slaby
Cc: Avi Kivity, Changli Gao, Tetsuo Handa, akpm, paulmck, adobriyan,
mingo, peterz, linux-fsdevel, linux-kernel
Le lundi 03 mai 2010 à 11:19 +0200, Jiri Slaby a écrit :
> On 05/03/2010 11:13 AM, Eric Dumazet wrote:
> > /*
> > * Warning: if size is not a power of two, kmalloc() might
> > * waste memory because of its requirements.
> > */
> > void *kvmalloc(size_t size)
> > {
> > void *ptr = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
>
> This is kind of waste of high-order pages, isn't it? Wouldn't it be
> better to have a watermark where we skip kmalloc completely?
>
>
There are many drawbacks using kmalloc() instead of vmalloc().
(NUMA comes to mind, if you want a zone spreaded on all available nodes)
So this kvmalloc() is not meant to be a generic replacement of all
vmalloc() users, only selected ones.
In some spots, allocating 8192 bytes in a driver setup for example, it
should be fine. Using a high order page for this is perfect, as long as
we can have a fallback to vmalloc()
grep bnx2_alloc_mem /proc/vmallocinfo
0xf8690000-0xf8692000 8192 bnx2_alloc_mem+0x111/0x6d0 pages=1 vmalloc
--
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] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-03 9:13 ` Eric Dumazet
2010-05-03 9:19 ` Jiri Slaby
@ 2010-05-04 4:20 ` Changli Gao
2010-05-04 5:33 ` Eric Dumazet
1 sibling, 1 reply; 18+ messages in thread
From: Changli Gao @ 2010-05-04 4:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: Avi Kivity, Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
On Mon, May 3, 2010 at 5:13 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> /*
> * Warning: if size is not a power of two, kmalloc() might
> * waste memory because of its requirements.
> */
> void *kvmalloc(size_t size)
> {
> void *ptr = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
>
> if (ptr)
> return ptr;
> return vmalloc(size);
> }
we can use alloc_pages_exact()/free_pages_exact() when size is larger
than PAGE_SIZE, then size isn't needed to be a power of two.
void *kvmalloc(size_t size)
{
void *ptr;
if (size < PAGE_SIZE)
return kmalloc(size, GFP_KERNEL);
ptr = alloc_pages_exact(size, GFP_KERNEL | __GFP_NOWARN);
if (ptr)
return ptr;
return vmalloc(size);
}
void *kvfree(void *ptr, size_t size)
{
BUG_ON(in_interrupt());
if (size < PAGE_SIZE)
kfree(ptr);
else if (is_vmalloc_addr(ptr))
vfree(ptr);
else
free_pages_exact(ptr, size);
}
>
> void kvfree(void *ptr)
> {
> BUG_ON(in_interrupt());
> if (is_vmalloc_addr(ptr))
> vfree(ptr);
> else
> kfree(ptr);
> }
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible
2010-05-04 4:20 ` Changli Gao
@ 2010-05-04 5:33 ` Eric Dumazet
0 siblings, 0 replies; 18+ messages in thread
From: Eric Dumazet @ 2010-05-04 5:33 UTC (permalink / raw)
To: Changli Gao
Cc: Avi Kivity, Tetsuo Handa, jslaby, akpm, paulmck, adobriyan, mingo,
peterz, linux-fsdevel, linux-kernel
Le mardi 04 mai 2010 à 12:20 +0800, Changli Gao a écrit :
> we can use alloc_pages_exact()/free_pages_exact() when size is larger
> than PAGE_SIZE, then size isn't needed to be a power of two.
>
> void *kvmalloc(size_t size)
> {
> void *ptr;
>
> if (size < PAGE_SIZE)
> return kmalloc(size, GFP_KERNEL);
> ptr = alloc_pages_exact(size, GFP_KERNEL | __GFP_NOWARN);
> if (ptr)
> return ptr;
> return vmalloc(size);
> }
>
> void *kvfree(void *ptr, size_t size)
> {
> BUG_ON(in_interrupt());
> if (size < PAGE_SIZE)
> kfree(ptr);
> else if (is_vmalloc_addr(ptr))
> vfree(ptr);
> else
> free_pages_exact(ptr, size);
> }
Yes, this has the drawback to need 'size' in kvfree(), but it should not
be a problem for users of this code.
alloc_pages_exact() also fragments high order page, but I guess it's not
worse than kmalloc() in this context.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2010-05-04 5:33 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <Alexander Viro <viro@zeniv.linux.org.uk>
2010-05-02 16:46 ` [PATCH] fs: use kmalloc() to allocate fdmem if possible Changli Gao
2010-05-02 17:31 ` Avi Kivity
2010-05-03 0:15 ` Tetsuo Handa
2010-05-03 6:06 ` Avi Kivity
2010-05-03 7:05 ` Changli Gao
2010-05-03 7:42 ` Avi Kivity
2010-05-03 7:52 ` Changli Gao
2010-05-03 8:49 ` Avi Kivity
2010-05-03 9:03 ` Jiri Slaby
2010-05-03 9:18 ` Avi Kivity
2010-05-03 9:22 ` Jiri Slaby
2010-05-03 9:25 ` Jiri Slaby
2010-05-03 9:28 ` Avi Kivity
2010-05-03 9:13 ` Eric Dumazet
2010-05-03 9:19 ` Jiri Slaby
2010-05-03 9:29 ` Eric Dumazet
2010-05-04 4:20 ` Changli Gao
2010-05-04 5:33 ` Eric Dumazet
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).