Re: [PATCH RFC] v4l2-core: Use kvmalloc() for potentially big allocations

[Sakari Ailus]

Hi Tomasz,

On Wed, May 31, 2017 at 09:46:05PM +0900, Tomasz Figa wrote:
> On Wed, May 31, 2017 at 9:09 PM, Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
> > Hi Tomasz,
> >
> >
> > On 2017-05-31 08:58, Tomasz Figa wrote:
> >>
> >> There are multiple places where arrays or otherwise variable sized
> >> buffer are allocated through V4L2 core code, including things like
> >> controls, memory pages, staging buffers for ioctls and so on. Such
> >> allocations can potentially require an order > 0 allocation from the
> >> page allocator, which is not guaranteed to be fulfilled and is likely to
> >> fail on a system with severe memory fragmentation (e.g. a system with
> >> very long uptime).
> >>
> >> Since the memory being allocated is intended to be used by the CPU
> >> exclusively, we can consider using vmalloc() as a fallback and this is
> >> exactly what the recently merged kvmalloc() helpers do. A kmalloc() call
> >> is still attempted, even for order > 0 allocations, but it is done
> >> with __GFP_NORETRY and __GFP_NOWARN, with expectation of failing if
> >> requested memory is not available instantly. Only then the vmalloc()
> >> fallback is used. This should give us fast and more reliable allocations
> >> even on systems with higher memory pressure and/or more fragmentation,
> >> while still retaining the same performance level on systems not
> >> suffering from such conditions.
> >>
> >> While at it, replace explicit array size calculations on changed
> >> allocations with kvmalloc_array().
> >>
> >> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> >> ---
> >>   drivers/media/v4l2-core/v4l2-async.c       |  4 ++--
> >>   drivers/media/v4l2-core/v4l2-ctrls.c       | 25
> >> +++++++++++++------------
> >>   drivers/media/v4l2-core/v4l2-event.c       |  8 +++++---
> >>   drivers/media/v4l2-core/v4l2-ioctl.c       |  6 +++---
> >>   drivers/media/v4l2-core/v4l2-subdev.c      |  7 ++++---
> >>   drivers/media/v4l2-core/videobuf2-dma-sg.c |  8 ++++----
> >
> >
> > For vb2:
> > Acked-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> 
> Thanks!
> 
> >
> > There are also a few vmalloc calls in old videobuf (v1) framework, which
> > might be converted to kvmalloc if you have a few spare minutes to take
> > a look.
> 
> I was intending to convert those as well, but on the other hand I
> concluded that it's some very old code, which might be difficult to
> test and likely to introduce some long undiscovered regressions. If
> it's desired to update those as well, I can include those changes in
> the non-RFC version.

I think it's better to leave videobuf1 as-is. I'd rather like to see it
removed instead.

Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx