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

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Wed, 31 May 2017 16:07:29 +0300
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> 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@samsung.com> 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@chromium.org>
> > >> ---
> > >>   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@samsung.com>  
> > 
> > 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.

Agreed.

There aren't much VB drivers anymore:

$ git grep -l VIDEOBUF_ |grep Kconfig
drivers/media/common/saa7146/Kconfig
drivers/media/pci/bt8xx/Kconfig
drivers/media/pci/cx18/Kconfig
drivers/media/platform/Kconfig
drivers/media/platform/davinci/Kconfig
drivers/media/platform/omap/Kconfig
drivers/media/usb/cx231xx/Kconfig
drivers/media/usb/tm6000/Kconfig
drivers/media/usb/zr364xx/Kconfig
drivers/media/v4l2-core/Kconfig
drivers/staging/media/atomisp/pci/Kconfig

(at platform/Kconfig, there are two drivers: via-camera and viu)

Not sure how easy/hard would be to convert those remaining ones.


Thanks,
Mauro