Page MenuHomeFreeBSD

Linux KPI request_irq() doesn't set interrupt description.
Needs ReviewPublic

Authored by anish on Sep 27 2019, 11:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 15 2024, 9:07 AM
Unknown Object (File)
Jan 10 2024, 1:56 AM
Unknown Object (File)
Dec 23 2023, 1:40 PM
Unknown Object (File)
Sep 9 2023, 12:19 PM
Unknown Object (File)
Aug 15 2023, 12:37 PM
Unknown Object (File)
Jul 11 2023, 3:44 AM
Unknown Object (File)
Jun 30 2023, 4:46 AM
Unknown Object (File)
Jun 29 2023, 3:58 AM
Subscribers

Details

Reviewers
hselasky
jeff
Summary

request_irq() takes 'name' as argument for interrupt
description but is not used. Use the name to set the IRQ
description.

Test Plan

Run "vmstat -i" to display the interrupt description.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 26904
Build 25215: arc lint + arc unit

Event Timeline

Updating D21828: Linux KPI request_irq() doesn't set interrupt description.

We intentionally don't use the "name" passed to request_irq() because for the mlx5en driver it will overflow the very-short description field !

Is this patch required for anything?

sys/compat/linuxkpi/common/include/linux/interrupt.h
111

Note that some drivers in FreeBSD may pass a NULL for name.

We intentionally don't use the "name" passed to request_irq() because for the mlx5en driver it will overflow the very-short description field !

I think intr_event_describe_handler() handles the long name by not using it. I was bitten by it and then I had to shrink device name to get around it by using fewer characters.

int
intr_event_describe_handler(struct intr_event *ie, void *cookie,
    const char *descr)
{

..
        /*
         * See if there is enough remaining room in the string for the
         * description + ":".  The "- 1" leaves room for the trailing
         * '\0'.  The "+ 1" accounts for the colon.
         */
        space = sizeof(ih->ih_name) - (start - ih->ih_name) - 1;
        if (strlen(descr) + 1 > space) {
                mtx_unlock(&ie->ie_lock);
                return (ENOSPC);
        }

For example if I use our NIC device queue name as "rxtx" it works fine

irq290: ion0:rxtx0               2571467          9
irq291: ion0:rxtx1               1583873          6
irq292: ion0:rxtx2                     0          0

But if I changed it to "rxtxque", it doesn't set name.

sys/compat/linuxkpi/common/include/linux/interrupt.h
111

Ah! thanks I will fix it.

Fix as per code review comment, use name only if its not NULL.

Can you set the argument for this function to NULL for all current in-kernel consumers of this function?

--HPS

Don't set interrupt description for all Mellanox drivers as part of request_irq() as per review request.

You need to rebase your patch on top of the latest -current.