| Message ID | 20210903152430.244937-1-nitesh@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | genirq: Cleanup the abuse of irq_set_affinity_hint() | expand |
On Fri, Sep 3, 2021 at 11:25 AM Nitesh Narayan Lal <nitesh@redhat.com> wrote: > > The drivers currently rely on irq_set_affinity_hint() to either set the > affinity_hint that is consumed by the userspace and/or to enforce a custom > affinity. > > irq_set_affinity_hint() as the name suggests is originally introduced to > only set the affinity_hint to help the userspace in guiding the interrupts > and not the affinity itself. However, since the commit > > e2e64a932556 "genirq: Set initial affinity in irq_set_affinity_hint()" [...] > > Nitesh Narayan Lal (13): > iavf: Use irq_update_affinity_hint > i40e: Use irq_update_affinity_hint > scsi: megaraid_sas: Use irq_set_affinity_and_hint > scsi: mpt3sas: Use irq_set_affinity_and_hint > RDMA/irdma: Use irq_update_affinity_hint > enic: Use irq_update_affinity_hint > be2net: Use irq_update_affinity_hint > ixgbe: Use irq_update_affinity_hint > mailbox: Use irq_update_affinity_hint > scsi: lpfc: Use irq_set_affinity > hinic: Use irq_set_affinity_and_hint > net/mlx5: Use irq_set_affinity_and_hint > net/mlx4: Use irq_update_affinity_hint > > Thomas Gleixner (1): > genirq: Provide new interfaces for affinity hints > Any suggestions on what should be the next steps here? Unfortunately, I haven't been able to get any reviews on the following two patches: be2net: Use irq_update_affinity_hint hinic: Use irq_set_affinity_and_hint One option would be to proceed with the remaining patches and I can try posting these two again when I post patches for the remaining drivers?
Nitesh, On Mon, Sep 13 2021 at 10:34, Nitesh Lal wrote: > On Fri, Sep 3, 2021 at 11:25 AM Nitesh Narayan Lal <nitesh@redhat.com> wrote: >> >> The drivers currently rely on irq_set_affinity_hint() to either set the >> affinity_hint that is consumed by the userspace and/or to enforce a custom >> affinity. >> >> irq_set_affinity_hint() as the name suggests is originally introduced to >> only set the affinity_hint to help the userspace in guiding the interrupts >> and not the affinity itself. However, since the commit >> >> e2e64a932556 "genirq: Set initial affinity in irq_set_affinity_hint()" sorry for ignoring this. It fell through the cracks. >> Thomas Gleixner (1): >> genirq: Provide new interfaces for affinity hints Did I actually write this? > Any suggestions on what should be the next steps here? Unfortunately, I haven't > been able to get any reviews on the following two patches: > be2net: Use irq_update_affinity_hint > hinic: Use irq_set_affinity_and_hint > > One option would be to proceed with the remaining patches and I can try > posting these two again when I post patches for the remaining drivers? The more general question is whether I should queue all the others or whether some subsystem would prefer to pull in a tagged commit on top of rc1. I'm happy to carry them all of course. Thanks, tglx
On Wed, Nov 24, 2021 at 2:30 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > Nitesh, > > On Mon, Sep 13 2021 at 10:34, Nitesh Lal wrote: > > On Fri, Sep 3, 2021 at 11:25 AM Nitesh Narayan Lal <nitesh@redhat.com> wrote: > >> > >> The drivers currently rely on irq_set_affinity_hint() to either set the > >> affinity_hint that is consumed by the userspace and/or to enforce a custom > >> affinity. > >> > >> irq_set_affinity_hint() as the name suggests is originally introduced to > >> only set the affinity_hint to help the userspace in guiding the interrupts > >> and not the affinity itself. However, since the commit > >> > >> e2e64a932556 "genirq: Set initial affinity in irq_set_affinity_hint()" > > sorry for ignoring this. It fell through the cracks. No worries, thank you for reviewing. > > > >> Thomas Gleixner (1): > >> genirq: Provide new interfaces for affinity hints > > Did I actually write this? Yeap, the idea and the initial patch came from you. :) > > > > Any suggestions on what should be the next steps here? Unfortunately, I haven't > > been able to get any reviews on the following two patches: > > be2net: Use irq_update_affinity_hint > > hinic: Use irq_set_affinity_and_hint > > > > One option would be to proceed with the remaining patches and I can try > > posting these two again when I post patches for the remaining drivers? > > The more general question is whether I should queue all the others or > whether some subsystem would prefer to pull in a tagged commit on top of > rc1. I'm happy to carry them all of course. > I am fine either way. In the past, while I was asking for more testing help I was asked if the SCSI changes are part of Martins's scsi-fixes tree as that's something Broadcom folks test to check for regression. So, maybe Martin can pull this up?
Hi Martin, On Wed, Nov 24, 2021 at 5:16 PM Nitesh Lal <nilal@redhat.com> wrote: > > On Wed, Nov 24, 2021 at 2:30 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > > > Nitesh, > > > > On Mon, Sep 13 2021 at 10:34, Nitesh Lal wrote: > > > On Fri, Sep 3, 2021 at 11:25 AM Nitesh Narayan Lal <nitesh@redhat.com> wrote: > > >> > > >> The drivers currently rely on irq_set_affinity_hint() to either set the > > >> affinity_hint that is consumed by the userspace and/or to enforce a custom > > >> affinity. > > >> > > >> irq_set_affinity_hint() as the name suggests is originally introduced to > > >> only set the affinity_hint to help the userspace in guiding the interrupts > > >> and not the affinity itself. However, since the commit > > >> > > >> e2e64a932556 "genirq: Set initial affinity in irq_set_affinity_hint()" > > > > sorry for ignoring this. It fell through the cracks. > > > No worries, thank you for reviewing. > > > > > > > >> Thomas Gleixner (1): > > >> genirq: Provide new interfaces for affinity hints > > > > Did I actually write this? > > > Yeap, the idea and the initial patch came from you. :) > > > > > > > > Any suggestions on what should be the next steps here? Unfortunately, I haven't > > > been able to get any reviews on the following two patches: > > > be2net: Use irq_update_affinity_hint > > > hinic: Use irq_set_affinity_and_hint > > > > > > One option would be to proceed with the remaining patches and I can try > > > posting these two again when I post patches for the remaining drivers? > > > > The more general question is whether I should queue all the others or > > whether some subsystem would prefer to pull in a tagged commit on top of > > rc1. I'm happy to carry them all of course. > > > > I am fine either way. > In the past, while I was asking for more testing help I was asked if the > SCSI changes are part of Martins's scsi-fixes tree as that's something > Broadcom folks test to check for regression. > So, maybe Martin can pull this up? > Gentle ping. Any thoughts on the above query?
On Fri, Dec 10 2021 at 08:51, Nitesh Lal wrote: > On Wed, Nov 24, 2021 at 5:16 PM Nitesh Lal <nilal@redhat.com> wrote: >> > The more general question is whether I should queue all the others or >> > whether some subsystem would prefer to pull in a tagged commit on top of >> > rc1. I'm happy to carry them all of course. >> > >> >> I am fine either way. >> In the past, while I was asking for more testing help I was asked if the >> SCSI changes are part of Martins's scsi-fixes tree as that's something >> Broadcom folks test to check for regression. >> So, maybe Martin can pull this up? >> > > Gentle ping. > Any thoughts on the above query? As nobody cares, I'll pick it up. Thanks, tglx
On Fri, Dec 10, 2021 at 1:44 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Fri, Dec 10 2021 at 08:51, Nitesh Lal wrote: > > On Wed, Nov 24, 2021 at 5:16 PM Nitesh Lal <nilal@redhat.com> wrote: > >> > The more general question is whether I should queue all the others or > >> > whether some subsystem would prefer to pull in a tagged commit on top of > >> > rc1. I'm happy to carry them all of course. > >> > > >> > >> I am fine either way. > >> In the past, while I was asking for more testing help I was asked if the > >> SCSI changes are part of Martins's scsi-fixes tree as that's something > >> Broadcom folks test to check for regression. > >> So, maybe Martin can pull this up? > >> > > > > Gentle ping. > > Any thoughts on the above query? > > As nobody cares, I'll pick it up. > Sounds good to me. Thank you! -- Nitesh
The drivers currently rely on irq_set_affinity_hint() to either set the affinity_hint that is consumed by the userspace and/or to enforce a custom affinity. irq_set_affinity_hint() as the name suggests is originally introduced to only set the affinity_hint to help the userspace in guiding the interrupts and not the affinity itself. However, since the commit e2e64a932556 "genirq: Set initial affinity in irq_set_affinity_hint()" irq_set_affinity_hint() also started applying the provided cpumask (if not NULL) as the affinity for the interrupts. The issue that this commit was trying to solve is to allow the drivers to enforce their affinity mask to distribute the interrupts across the CPUs such that they don't always end up on CPU0. This issue has been resolved within the irq subsystem since the commit a0c9259dc4e1 "irq/matrix: Spread interrupts on allocation" Hence, there is no need for the drivers to overwrite the affinity to spread as it is dynamically performed at the time of allocation. Also, irq_set_affinity_hint() setting affinity unconditionally introduces issues for the drivers that only want to set their affinity_hint and not the affinity itself as for these driver interrupts the default_smp_affinity_mask is completely ignored (for detailed investigation please refer to [1]). Unfortunately reverting the commit e2e64a932556 is not an option at this point for two reasons [2]: - Several drivers for a valid reason (performance) rely on this API to enforce their affinity mask - Until very recently this was the only exported interface that was available To clear this out Thomas has come up with the following interfaces: - irq_set_affinity(): only sets affinity of an IRQ [3] - irq_update_affinity_hint(): Only sets the hint [4] - irq_set_affinity_and_hint(): Sets both affinity and the hint mask [4] The first API is already merged in the linus's tree and the patch that introduces the other two interfaces is included with this patch-set. To move to the stage where we can safely get rid of the irq_set_affinity_hint(), which has been marked deprecated, we have to move all its consumers to these new interfaces. In this patch-set, I have done that for a few drivers and will hopefully try to move the remaining of them in the coming days. The two patches in this series that still needs to be reviewed are: be2net: Use irq_update_affinity_hint hinic: Use irq_set_affinity_and_hint any help there will be much appreciated. Change from v5 [5] ------------------ - Rebased on top of the latest Linus's tree: a9c9a6f741cd "Merge tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi" - Added reviewed-by/acked-by in the patches Change from v4 [6] ------------------ - Fixed kernel-doc warnings (Jakub Kicinski) - Renamed the cover to "Cleanup the abuse of irq_set_affinity_hint()", what Thomas originally used as this series is an extension of the work that he started and proposed [7]. Change from v3 [8] ------------------ - Replaced irq_set_affinity_and_hint with irq_update_affinity_hint in irdma (Leon Romanovsky) - rebased the patches on top of 5.14-rc2 Change from v2 [9] ------------------ - Rebased on top of 5.14-rc1 (Leon Romanovsky) + After discussion with Leon [10], made changes in the mlx5 patch to use irq_set_affinity_and_hint over irq_update_affinity_hint + i40iw is replaced with irdma driver, hence made the respective changes in irdma (also replcaed irq_update_affinity_hint with irq_set_affinity_and_hint). Change from v1 [11] ------------------ - Fixed compilation error by adding the new interface definitions for cases where CONFIG_SMP is not defined - Fixed function usage in megaraid_sas and removed unnecessary variable (Robin Murphy) - Removed unwanted #if/endif from mlx4 (Leon Romanovsky) - Other indentation related fixes [1] https://lore.kernel.org/lkml/1a044a14-0884-eedb-5d30-28b4bec24b23@redhat.com/ [2] https://lore.kernel.org/linux-pci/d1d5e797-49ee-4968-88c6-c07119343492@arm.com/ [3] https://lore.kernel.org/linux-arm-kernel/20210518091725.046774792@linutronix.de/ [4] https://lore.kernel.org/patchwork/patch/1434326/ [5] https://lore.kernel.org/netdev/20210720232624.1493424-10-nitesh@redhat.com/T/ [6] https://lore.kernel.org/lkml/20210719180746.1008665-1-nitesh@redhat.com/ [7] https://lore.kernel.org/linux-arm-kernel/20210518091725.046774792@linutronix.de/ [8] https://lore.kernel.org/linux-scsi/20210713211502.464259-1-nitesh@redhat.com/ [9] https://lore.kernel.org/lkml/20210629152746.2953364-1-nitesh@redhat.com/ [10] https://lore.kernel.org/lkml/YO0eKv2GJcADQTHH@unreal/ [11] https://lore.kernel.org/linux-scsi/20210617182242.8637-1-nitesh@redhat.com/ Nitesh Narayan Lal (13): iavf: Use irq_update_affinity_hint i40e: Use irq_update_affinity_hint scsi: megaraid_sas: Use irq_set_affinity_and_hint scsi: mpt3sas: Use irq_set_affinity_and_hint RDMA/irdma: Use irq_update_affinity_hint enic: Use irq_update_affinity_hint be2net: Use irq_update_affinity_hint ixgbe: Use irq_update_affinity_hint mailbox: Use irq_update_affinity_hint scsi: lpfc: Use irq_set_affinity hinic: Use irq_set_affinity_and_hint net/mlx5: Use irq_set_affinity_and_hint net/mlx4: Use irq_update_affinity_hint Thomas Gleixner (1): genirq: Provide new interfaces for affinity hints drivers/infiniband/hw/irdma/hw.c | 4 +- drivers/mailbox/bcm-flexrm-mailbox.c | 4 +- drivers/net/ethernet/cisco/enic/enic_main.c | 8 +-- drivers/net/ethernet/emulex/benet/be_main.c | 4 +- drivers/net/ethernet/huawei/hinic/hinic_rx.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +-- drivers/net/ethernet/intel/iavf/iavf_main.c | 8 +-- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 10 ++-- drivers/net/ethernet/mellanox/mlx4/eq.c | 8 ++- .../net/ethernet/mellanox/mlx5/core/pci_irq.c | 8 +-- drivers/scsi/lpfc/lpfc_init.c | 4 +- drivers/scsi/megaraid/megaraid_sas_base.c | 27 +++++----- drivers/scsi/mpt3sas/mpt3sas_base.c | 21 ++++---- include/linux/interrupt.h | 53 ++++++++++++++++++- kernel/irq/manage.c | 8 +-- 15 files changed, 114 insertions(+), 65 deletions(-) --