Messages in this thread | | | Date | Thu, 29 Mar 2012 16:35:56 +1100 | From | Dave Chinner <> | Subject | Re: suppress page allocation failure warnings from sys_listxattr |
| |
On Wed, Mar 28, 2012 at 06:46:02PM -0700, Andrew Morton wrote: > On Wed, 28 Mar 2012 18:28:43 -0700 Joe Perches <joe@perches.com> wrote: > > > On Wed, 2012-03-28 at 18:10 -0700, Andrew Morton wrote: > > > On Wed, 28 Mar 2012 20:54:42 -0400 Dave Jones <davej@redhat.com> wrote: > > > > > Yup. How does the below look? > > > > Don't see anything immediately wrong with it. > > > > Any thoughts on what to do about the similar problem in setxattr ? (memdup_user) > > [] > > > diff -puN fs/xattr.c~fs-xattrc-setxattr-improve-handling-of-allocation-failures fs/xattr.c > > [] > > > @@ -334,13 +335,25 @@ setxattr(struct dentry *d, const char __ > > [] > > > + kvalue = kmalloc(size, GFP_KERNEL | __GFP_NOWARN); > > > + if (!kvalue) { > > > + vvalue = vmalloc(size); > > [] > > > + if (vvalue) > > > + vfree(vvalue); > > > + else > > > + kfree(kvalue); > > > return error; > > > > These patterns are pretty common, maybe create a standard helper? > > Could. There was some discussion last year and implementations were > tossed around. > > I'm a bit apprehensive - kernel code is supposed to be robust, and > large allocations are not robust and vmalloc() is crappy. Formalising > these things in an API probably won't make anything worse, but will > deprive us of opportunities for ritualistic humiliation and > knuckle-rapping.
I did a sweep of this recently, considering helpers for exactly such an allocation and replacing the existing per-filesystem wrappers for it. IIRC, there are wrappper functions in ext4, gfs2, and ntfs, XFS now open codes it in a couple of places, there's alloc_fdmem(), cgroup pidlists and the network code does it in several places, etc. Even some drivers are doing this. It's a widespread pattern.
The easiest way to find the trivial wrappers is to grep for is_vmalloc_addr, because all the wrapper functions use this code to determine what to do:
if (is_vmalloc_addr(p)) vfree(p) else kfree(p)
Cheers,
Dave. -- Dave Chinner david@fromorbit.com
| |