linux-kernel - RE: [PATCH v3 1/1] swiotlb: Track and report io_tlb

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]

Message-ID: <BYAPR21MB1688CCCC4CFC597A3C12E370D7969@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Fri, 7 Apr 2023 22:01:13 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Christoph Hellwig <hch@....de>
CC:     "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        Dexuan Cui <decui@...rosoft.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 1/1] swiotlb: Track and report io_tlb_used high water
 mark in debugfs

From: Christoph Hellwig <hch@....de> Sent: Thursday, April 6, 2023 10:50 PM
> 
> On Fri, Mar 31, 2023 at 09:45:00PM -0700, Michael Kelley wrote:
> > Changes in v3:
> > * Do high water mark accounting only when CONFIG_DEBUG_FS=y. As
> >   as a result, add back the mem_used() function for the "swiotlb
> >   buffer is full" error message. [Christoph -- I didn't hear back
> >   whether this approach addresses your concern about one additional
> >   atomic operation when slots are allocated and again when freed. I've
> >   gone ahead with this new version, and we can obviously have further
> >   discussion.]
> 
> Still not too happy, but at least debugfs is an interfact we could
> remove at any time.
> 
> But can you please factor the used_hiwater accounting into two
> separate helpers that are udner CONFIG_DEBUG_FS and otherwise
> stubbed out, instead of adding the logic directly into
> swiotlb_do_find_slots and swiotlb_release_slots?

I coded the way I did to follow the kernel coding style guidance
that prefers converting a Kconfig symbol into a C boolean
expression, and using it in a normal C conditional instead of
using #ifdef.  If CONFIG_DEBUG_FS=n, the compiler will constant
fold the conditional away so there's no runtime overhead.  I
like the way that approached worked out in this case, but if you prefer
separate functions with #ifdef and stubs, I don't feel strongly either way.

Michael