linux-kernel - Re: [PATCH] media: cedrus: Remove global IRQ spin lock from the driver

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Nov 2018 11:47:22 +0100
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Cc:     linux-media@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        linux-sunxi@...glegroups.com, Hans Verkuil <hverkuil@...all.nl>,
        Ezequiel Garcia <ezequiel@...labora.com>,
        Tomasz Figa <tfiga@...omium.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>
Subject: Re: [PATCH] media: cedrus: Remove global IRQ spin lock from the
 driver

On Thu, Nov 15, 2018 at 03:39:55PM +0100, Paul Kocialkowski wrote:
> We initially introduced a spin lock to ensure that the VPU registers
> are not accessed concurrently between our setup function and IRQ
> handler. Because the V4L2 M2M API only allows one job to run at a time
> and our jobs are completed following the IRQ handler, there is actually
> no chance of an interrupt happening while programming the VPU registers.

That's not entirely true. There's no chance that the interrupt
signaling the end of the frame decoding can happen.

However, spurious interrupts can happen at any point in time as soon
as you have the interrupts enabled.

> In addition, holding a spin lock is problematic when doing more than
> simply configuring the registers in the setup operation. H.265 support
> currently involves a CMA allocation in that step, which is not
> compatible with an atomic context.

That's not really true either. Any allocation can be done with
GFP_ATOMIC or GFP_NOWAIT, and be compatible with an atomic
context. Whether it's something we want is a different story :)

And since the h265 code isn't upstream, I'm not really sure it's
relevant to mention it here.

> As a result, remove the global IRQ spin lock.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>

Acked-by: Maxime Ripard <maxime.ripard@...tlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)