linux-kernel - RE: [patch V2 07/33] genirq/msi: Provide msi_create/free_device_irq

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:   Wed, 23 Nov 2022 12:38:08 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     "x86@...nel.org" <x86@...nel.org>, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Marc Zyngier <maz@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jason Gunthorpe <jgg@...lanox.com>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        Logan Gunthorpe <logang@...tatee.com>,
        "Raj, Ashok" <ashok.raj@...el.com>, Jon Mason <jdmason@...zu.us>,
        Allen Hubbe <allenbh@...il.com>
Subject: RE: [patch V2 07/33] genirq/msi: Provide
 msi_create/free_device_irq_domain()

On Wed, Nov 23 2022 at 08:02, Kevin Tian wrote:
>> From: Thomas Gleixner <tglx@...utronix.de>
>> Sent: Monday, November 21, 2022 10:38 PM
>> 
>> 
>> +static inline void msi_remove_device_irqdomains(struct device *dev, struct
>> msi_device_data *md)
>> +{
>
> 'md' is unused

Duh, yes.

>> + *
>> + * Return: True on success, false otherwise
>
> Can you elaborate why boolean type is selected instead of returning the
> actual error codes? the outmost callers are all new functions:
>
> 	pci_setup_msi[x]_device_domain()
> 	pci_create_ims_domain()
>
> I didn't find out any legacy convention forcing this way...

What's the value of error codes? 99% of all callsites do:

       ret = foo();
       if (ret)
          goto fail;

Nothing evaluates the error codes, unless there is real useful
information like EAGAIN or ENOSPC which can tell the caller to retry
eventually or with different parameters. But for the above, the error
code is just useless.

>> +	bundle = kmemdup(template, sizeof(*bundle), GFP_KERNEL);
>> +	if (!bundle)
>> +		return false;
>> +
>> +	bundle->info.hwsize = hwsize ? hwsize : MSI_MAX_INDEX;
>
> patch04 marks that hwsize being 0 means unknown or unlimited in the
> header file.
>
> but here info.hwsize always gets a value i.e. the meaning of 0 only exists
> in this function. What about removing the trailing words about 0 in
> patch04?
>
> - + * @hwsize:		The hardware table size (0 if unknown/unlimited)
> + + * @hwsize:		The hardware table size

Fair enough, though I rather make that:

 * @hwsize:		The hardware table size or the software defined
                        index limit