Re: [PATCH] regset: use vmalloc() for regset_get_alloc()

[Doug Anderson <dianders@chromium.org>]

Hi,

On Thu, Feb 1, 2024 at 5:24 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Feb 01, 2024 at 05:12:03PM -0800, Douglas Anderson wrote:
> > While browsing through ChromeOS crash reports, I found one with an
> > allocation failure that looked like this:
> >
> >   chrome: page allocation failure: order:7,
> >           mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO),
> >         nodemask=(null),cpuset=urgent,mems_allowed=0
>
> That does seem bad ...
>
> > @@ -16,14 +17,14 @@ static int __regset_get(struct task_struct *target,
> >       if (size > regset->n * regset->size)
> >               size = regset->n * regset->size;
> >       if (!p) {
> > -             to_free = p = kzalloc(size, GFP_KERNEL);
> > +             to_free = p = vmalloc(size);
>
> It's my impression that sometimes this size might be relatively small?
> Perhaps we should make this kvmalloc so that we can satisfy it from the
> slab allocator if it is small?

Right. Sometimes it's small. It feels sad to me that somehow vmalloc()
of small sizes would be much less efficient than kvmalloc() of small
sizes, but I can change it to that if you want. It feels like we
should use kmalloc() if we need it to be contiguous, kvmalloc() if we
know that there will be big efficiency gains with things being
contiguous but we can get by with non-contiguous, and vmalloc() if we
just don't care. ;-)

...anyway, I'll spin v2 with kvmalloc().


> Also, I assume that we don't rely on this memory being physically
> contiguous; we don't, for example, do I/O on it?

As far as I can tell we don't. I had never looked at or thought about
this code before today and so all I have is ~an hour of code analysis
behind me, so if someone tells me I'm wrong then I'll believe them.

-Doug