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

Message-ID: <BYAPR21MB1688718645F8B19682E93552D79B9@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Wed, 12 Apr 2023 17:30:37 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     "hch@....de" <hch@....de>
CC:     Petr Tesařík <petr@...arici.cz>,
        "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: hch@....de <hch@....de> Sent: Monday, April 10, 2023 8:42 PM
> 
> On Fri, Apr 07, 2023 at 10:05:26PM +0000, Michael Kelley (LINUX) wrote:
> > > Yes, this works fine now, but why are total_used and used_hiwater
> > > global variables? If you make them fields in struct io_tlb_mem
> > > (possibly guarded with #ifdef CONFIG_DEBUG_FS), you can get rid of the
> > > check. Of course, in instances other than io_tlb_default_mem these
> > > fields would not be exported to userspace through debugfs, but if really
> > > needed, I can at least find them in a crash dump (or read them through
> > > /proc/kcore).
> > >
> >
> > Got it.
> >
> > Your previously comments mentioned making them fields in struct io_tlb_mem,
> > and I missed your point.  :-(   I got focused on fixing the accounting for
> > DEBUG_FS so it didn't include the non-default pools, and didn't pick up on the
> > idea of doing the accounting for the non-default pools even though the values
> > aren't exposed in /sys.   I'll fix this in the next version.
> 
> FYI, I agree that per-instance accounting is probably the better way,
> too.

It turns out that restricted pool swiotlb information *is* available in /sys
when CONFIG_DEBUG_FS=y.  There's a call to swiotlb_create_debugfs_files()
in rmem_swiotlb_device_init().  But the /sys info is broken because
io_tlb_used_get() is hardwired to the default pool.

But this is easily fixable. I'll do a two-patch series that fixes this problem,
and then makes the other changes for the high water mark that we've
been discussing.

Michael