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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170608132609.0a4748ec@vento.lan>
Date:   Thu, 8 Jun 2017 13:26:09 -0300
From:   Mauro Carvalho Chehab <mchehab@...pensource.com>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     Tomasz Figa <tfiga@...omium.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        linux-media@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Pawel Osciak <pawel@...iak.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>
Subject: Re: [PATCH RFC] v4l2-core: Use kvmalloc() for potentially big
 allocations

Em Wed, 31 May 2017 16:07:29 +0300
Sakari Ailus <sakari.ailus@....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@...sung.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@...omium.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@...sung.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