* [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 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: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 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: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: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).