linux-kernel - Re: [PATCH v2 01/11] PCI: Prepare removing devres from pci

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: <2f94dd0f0bfef8d51f1ced78a9b5db8e2ce48429.camel@redhat.com>
Date: Wed, 13 Nov 2024 17:11:29 +0100
From: Philipp Stanner <pstanner@...hat.com>
To: Thomas Gleixner <tglx@...utronix.de>, Damien Le Moal
 <dlemoal@...nel.org>,  Niklas Cassel <cassel@...nel.org>, Basavaraj Natikar
 <basavaraj.natikar@....com>, Jiri Kosina <jikos@...nel.org>,  Benjamin
 Tissoires <bentiss@...nel.org>, Arnd Bergmann <arnd@...db.de>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Alex Dubov <oakad@...oo.com>,
 Sudarsana Kalluru <skalluru@...vell.com>, Manish Chopra
 <manishc@...vell.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rasesh Mody
 <rmody@...vell.com>,  GR-Linux-NIC-Dev@...vell.com, Igor Mitsyanko
 <imitsyanko@...ntenna.com>,  Sergey Matyukevich <geomatsi@...il.com>, Kalle
 Valo <kvalo@...nel.org>, Sanjay R Mehta <sanju.mehta@....com>, Shyam Sundar
 S K <Shyam-sundar.S-k@....com>, Jon Mason <jdmason@...zu.us>, Dave Jiang
 <dave.jiang@...el.com>, Allen Hubbe <allenbh@...il.com>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Alex Williamson <alex.williamson@...hat.com>,
 Juergen Gross <jgross@...e.com>, Stefano Stabellini
 <sstabellini@...nel.org>, Oleksandr Tyshchenko
 <oleksandr_tyshchenko@...m.com>, Mario Limonciello
 <mario.limonciello@....com>, Chen Ni <nichen@...as.ac.cn>, Ricky Wu
 <ricky_wu@...ltek.com>,  Al Viro <viro@...iv.linux.org.uk>, Breno Leitao
 <leitao@...ian.org>, Kevin Tian <kevin.tian@...el.com>, Mostafa Saleh
 <smostafa@...gle.com>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Jason Gunthorpe <jgg@...pe.ca>, Yi Liu <yi.l.liu@...el.com>, Kunwu Chan
 <chentao@...inos.cn>, Ankit Agrawal <ankita@...dia.com>, Christian Brauner
 <brauner@...nel.org>, Reinette Chatre <reinette.chatre@...el.com>, Eric
 Auger <eric.auger@...hat.com>, Ye Bin <yebin10@...wei.com>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-input@...r.kernel.org, netdev@...r.kernel.org, 
 linux-wireless@...r.kernel.org, ntb@...ts.linux.dev,
 linux-pci@...r.kernel.org,  kvm@...r.kernel.org,
 xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v2 01/11] PCI: Prepare removing devres from pci_intx()

On Wed, 2024-11-13 at 17:04 +0100, Thomas Gleixner wrote:
> On Wed, Nov 13 2024 at 13:41, Philipp Stanner wrote:
> > +/**
> > + * pci_intx_unmanaged - enables/disables PCI INTx for device dev,
> > + * unmanaged version
> > + * @pdev: the PCI device to operate on
> > + * @enable: boolean: whether to enable or disable PCI INTx
> 
> Except that the argument is of type int, which really should be type
> bool.

True, but this is a *temporary* copy of pci_intx(), a ~16 year old
function. Older C programmers had the habit of for some reason using
32-bit integers for a true/false boolean all the time.

We _could_ think of changing pci_intx()'s parameter to a boolean, but I
think it wouldn't really improve things very much

see also below

> 
> > + * Enables/disables PCI INTx for device @pdev
> > + *
> > + * This function behavios identically to pci_intx(), but is never
> > managed with
> > + * devres.
> 
> behavios?

-> behaves. Will fix.

> 
> > + */
> > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable)
> 
> I find this function name mildy confusing. This _unmanaged suffix is
> not
> really telling me anything. And the reference that this behaves
> identically to pci_intx() makes it even worse.
> 
> This function is about controlling the PCI INTX_DISABLE bit in the
> PCI_COMMAND config word, right?
> 
> So naming it pci_intx_control() would make it entirely clear what
> this
> is about, no?

We had this conversation last week. I answered on that already, maybe
you have overlooked it:

https://lore.kernel.org/all/a8d9f32f60f55c58d79943c4409b8b94535ff853.camel@redhat.com/

Please also take a look at patch 11, then you'll see the full picture

Danke,
Philipp

> 
> Thanks,
> 
>         tglx
>