Re: [PATCH] fs: use kmalloc() to allocate fdmem if possible

Avi Kivity
2010-05-03T06:07:11Z

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.