linux-kernel - Re: [PATCH 00/10 v2] iommu/amd: lock splitting & GFP

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:   Sat, 17 Mar 2018 16:43:39 -0500
From:   Scott Wood <swood@...hat.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     Joerg Roedel <joro@...tes.org>, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH 00/10 v2] iommu/amd: lock splitting & GFP_KERNEL
 allocation

On Sat, 2018-03-17 at 22:10 +0100, Sebastian Andrzej Siewior wrote:
> On 2018-03-17 14:49:54 [-0500], Scott Wood wrote:
> > On Fri, 2018-03-16 at 21:18 +0100, Sebastian Andrzej Siewior wrote:
> > > The goal here is to make the memory allocation in get_irq_table()
> > > not
> > > with disabled interrupts and having as little raw_spin_lock as
> > > possible
> > > while having them if the caller is also holding one (like desc-
> > > >lock
> > > during IRQ-affinity changes).
> > > I reverted one patch one patch in the iommu while rebasing since
> > > it
> > > make job easier.
> > 
> > If the goal is to have "as little raw_spin_lock as possible" -- and
> > presumably also to avoid unnecessary complexity -- wouldn't it be
> > better to leave my patch in, and drop patches 4 and 9?
> 
> 9 gives me GFP_KERNEL instead atomic so no.
> 4 is needed I think but I could double check on Monday. 

If that's worth the lock dropping then fine (though why does only one
of the two allocations use GFP_KERNEL?), but it doesn't need to be a
raw lock if the non-allocating users are separated.  Keeping them
separate will also preserve the WARNs if we somehow end up in an atomic
context with no table (versus relying on atomic sleep debugging that
may or may not be enabled), and make the code easier to understand by
being explicit about which functions can be used from RT-atomic
context.

-Scott