From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753199AbdBCKAR (ORCPT ); Fri, 3 Feb 2017 05:00:17 -0500 Received: from mail-by2nam03on0068.outbound.protection.outlook.com ([104.47.42.68]:37463 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752999AbdBCKAN (ORCPT ); Fri, 3 Feb 2017 05:00:13 -0500 From: "Jorgen S. Hansen" To: Christoph Hellwig CC: Adit Ranadive , "gregkh@linuxfoundation.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] vmw_vmci: switch to pci_irq_alloc_vectors Thread-Topic: [PATCH] vmw_vmci: switch to pci_irq_alloc_vectors Thread-Index: AQHSfJl/B6Ukt2xprk2ln84/3ugorKFXD2QA Date: Fri, 3 Feb 2017 10:00:07 +0000 Message-ID: References: <1485960177-22703-1-git-send-email-hch@lst.de> In-Reply-To: <1485960177-22703-1-git-send-email-hch@lst.de> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=jhansen@vmware.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [80.197.119.203] x-microsoft-exchange-diagnostics: 1;DM2PR05MB286;7:CqSBYlZCRrkNbEX0n4huCSqWp31vgurPOOPV/+Rv7+68bkwKKX1IbNyTjfTl0W/il0w2xP7QIWCt1ZIwixRxSlO4Fc48uWGVdU/Kjf0Blw4Zzfj7GjBn/QNBSWKp1UVhi11wIJUKadOyqIn4si8o3AFqspJgq6tn/OYsMHoKTm7nT0l3d/Gv3Lxuh2w4zYghs2HR7r+1z91Kdf0PwngkaYs6ftqAbz+f86uhPZG3MKtO07XhtUOe2a7amwXEkVCyzEGx6zMd3hF5keGE2WrrSNcAmT84bEFYXtl08di/DQAUDBgBExG+YA5D6IsdPinfM0EHZDNgGgD5qEsqwder2AdWH1I14AtcSmseUaVIqu1YWlhTkyQYoHAmcNRUHV2l3sgsJsFPwZT18Sk8nTvJ8r6INSPqZ5KvgLchY15B6cM16XMyEwVVe87P5KNRUg2du03Z/dLiUegtuRYe7kBdCoievKau34z3ODE71sC0KBwxhxAMRo8M9XkZosZDTlvyTl6EFuvuiURQooMF/kq1vhBeDOKOOBc5m5S2HwjmiWPxSVyYofWFKzBpB+lBntRB;20:adtkjTqQrIdHfCojzRM2GBOD8QI5+ObeX99bachhSqxokN/T4P+9mDpy3Q3rpZMWXdIvckfuy2c4037GS7RyYSWacJMkX4uJ08YbHtDB7QW8AUiQonhqS32J3ReHEvrlsyaNTnlYVqV2g9ewT2JyRi2eqYihsf33BdxtQtIr5tQ= x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39450400003)(199003)(189002)(377454003)(24454002)(51914003)(38730400001)(6436002)(4326007)(92566002)(8936002)(110136003)(3660700001)(54356999)(6506006)(36756003)(6486002)(6512007)(229853002)(50986999)(5890100001)(77096006)(101416001)(102836003)(5660300001)(54906002)(2900100001)(99286003)(106116001)(53936002)(6246003)(3846002)(76176999)(3280700002)(106356001)(68736007)(6116002)(25786008)(122556002)(66066001)(33656002)(6916009)(2906002)(81166006)(83716003)(105586002)(7736002)(86362001)(81156014)(189998001)(305945005)(8676002)(97736004)(2950100002)(82746002)(104396002);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR05MB286;H:BLUPR0501MB2052.namprd05.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; x-ms-office365-filtering-correlation-id: 594a390b-d3eb-49a7-2b31-08d44c1b6f37 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DM2PR05MB286; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(61668805478150); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6041248)(20161123564025)(20161123562025)(20161123560025)(20161123555025)(20161123558025)(6072148);SRVR:DM2PR05MB286;BCL:0;PCL:0;RULEID:;SRVR:DM2PR05MB286; x-forefront-prvs: 02070414A1 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <5409D9C557C42045967471B2434BC549@namprd05.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2017 10:00:07.7725 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR05MB286 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v13A0g61018437 > On Feb 1, 2017, at 3:42 PM, Christoph Hellwig wrote: > > Cleans up the IRQ management code a lot, including removing a lot of > state from the per-device structure. > > Signed-off-by: Christoph Hellwig > --- > drivers/misc/vmw_vmci/vmci_guest.c | 75 ++++++++++---------------------------- > include/linux/vmw_vmci_defs.h | 7 ---- > 2 files changed, 20 insertions(+), 62 deletions(-) > > diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c > index 189b325..9d65954 100644 > --- a/drivers/misc/vmw_vmci/vmci_guest.c > +++ b/drivers/misc/vmw_vmci/vmci_guest.c > @@ -54,10 +54,7 @@ struct vmci_guest_device { > struct device *dev; /* PCI device we are attached to */ > void __iomem *iobase; > > - unsigned int irq; > - unsigned int intr_type; > bool exclusive_vectors; > - struct msix_entry msix_entries[VMCI_MAX_INTRS]; > > struct tasklet_struct datagram_tasklet; > struct tasklet_struct bm_tasklet; > @@ -369,30 +366,6 @@ static void vmci_process_bitmap(unsigned long data) > } > > /* > - * Enable MSI-X. Try exclusive vectors first, then shared vectors. > - */ > -static int vmci_enable_msix(struct pci_dev *pdev, > - struct vmci_guest_device *vmci_dev) > -{ > - int i; > - int result; > - > - for (i = 0; i < VMCI_MAX_INTRS; ++i) { > - vmci_dev->msix_entries[i].entry = i; > - vmci_dev->msix_entries[i].vector = i; > - } > - > - result = pci_enable_msix_exact(pdev, > - vmci_dev->msix_entries, VMCI_MAX_INTRS); > - if (result == 0) > - vmci_dev->exclusive_vectors = true; > - else if (result == -ENOSPC) > - result = pci_enable_msix_exact(pdev, vmci_dev->msix_entries, 1); > - > - return result; > -} > - > -/* > * Interrupt handler for legacy or MSI interrupt, or for first MSI-X > * interrupt (vector VMCI_INTR_DATAGRAM). > */ > @@ -406,7 +379,7 @@ static irqreturn_t vmci_interrupt(int irq, void *_dev) > * Otherwise we must read the ICR to determine what to do. > */ > > - if (dev->intr_type == VMCI_INTR_TYPE_MSIX && dev->exclusive_vectors) { > + if (dev->exclusive_vectors) { > tasklet_schedule(&dev->datagram_tasklet); > } else { > unsigned int icr; > @@ -491,7 +464,6 @@ static int vmci_guest_probe_device(struct pci_dev *pdev, > } > > vmci_dev->dev = &pdev->dev; > - vmci_dev->intr_type = VMCI_INTR_TYPE_INTX; > vmci_dev->exclusive_vectors = false; > vmci_dev->iobase = iobase; > > @@ -592,26 +564,26 @@ static int vmci_guest_probe_device(struct pci_dev *pdev, > * Enable interrupts. Try MSI-X first, then MSI, and then fallback on > * legacy interrupts. > */ > - if (!vmci_disable_msix && !vmci_enable_msix(pdev, vmci_dev)) { > - vmci_dev->intr_type = VMCI_INTR_TYPE_MSIX; > - vmci_dev->irq = vmci_dev->msix_entries[0].vector; > - } else if (!vmci_disable_msi && !pci_enable_msi(pdev)) { > - vmci_dev->intr_type = VMCI_INTR_TYPE_MSI; > - vmci_dev->irq = pdev->irq; > + error = pci_alloc_irq_vectors(pdev, VMCI_MAX_INTRS, VMCI_MAX_INTRS, > + PCI_IRQ_MSIX); > + if (error) { > + error = pci_alloc_irq_vectors(pdev, 1, 1, > + PCI_IRQ_MSIX | PCI_IRQ_MSI | PCI_IRQ_LEGACY); > + if (error) > + goto err_remove_bitmap; > } else { > - vmci_dev->intr_type = VMCI_INTR_TYPE_INTX; > - vmci_dev->irq = pdev->irq; > + vmci_dev->exclusive_vectors = true; > } > > /* > * Request IRQ for legacy or MSI interrupts, or for first > * MSI-X vector. > */ > - error = request_irq(vmci_dev->irq, vmci_interrupt, IRQF_SHARED, > - KBUILD_MODNAME, vmci_dev); > + error = request_irq(pci_irq_vector(pdev, 0), vmci_interrupt, > + IRQF_SHARED, KBUILD_MODNAME, vmci_dev); > if (error) { > dev_err(&pdev->dev, "Irq %u in use: %d\n", > - vmci_dev->irq, error); > + pci_irq_vector(pdev, 0), error); > goto err_disable_msi; > } > > @@ -622,13 +594,13 @@ static int vmci_guest_probe_device(struct pci_dev *pdev, > * between the vectors. > */ > if (vmci_dev->exclusive_vectors) { > - error = request_irq(vmci_dev->msix_entries[1].vector, > + error = request_irq(pci_irq_vector(pdev, 1), > vmci_interrupt_bm, 0, KBUILD_MODNAME, > vmci_dev); > if (error) { > dev_err(&pdev->dev, > "Failed to allocate irq %u: %d\n", > - vmci_dev->msix_entries[1].vector, error); > + pci_irq_vector(pdev, 1), error); > goto err_free_irq; > } > } > @@ -651,15 +623,12 @@ static int vmci_guest_probe_device(struct pci_dev *pdev, > return 0; > > err_free_irq: > - free_irq(vmci_dev->irq, vmci_dev); > + free_irq(pci_irq_vector(pdev, 0), vmci_dev); > tasklet_kill(&vmci_dev->datagram_tasklet); > tasklet_kill(&vmci_dev->bm_tasklet); > > err_disable_msi: > - if (vmci_dev->intr_type == VMCI_INTR_TYPE_MSIX) > - pci_disable_msix(pdev); > - else if (vmci_dev->intr_type == VMCI_INTR_TYPE_MSI) > - pci_disable_msi(pdev); > + pci_free_irq_vectors(pdev); > > vmci_err = vmci_event_unsubscribe(ctx_update_sub_id); > if (vmci_err < VMCI_SUCCESS) > @@ -719,14 +688,10 @@ static void vmci_guest_remove_device(struct pci_dev *pdev) > * MSI-X, we might have multiple vectors, each with their own > * IRQ, which we must free too. > */ > - free_irq(vmci_dev->irq, vmci_dev); > - if (vmci_dev->intr_type == VMCI_INTR_TYPE_MSIX) { > - if (vmci_dev->exclusive_vectors) > - free_irq(vmci_dev->msix_entries[1].vector, vmci_dev); > - pci_disable_msix(pdev); > - } else if (vmci_dev->intr_type == VMCI_INTR_TYPE_MSI) { > - pci_disable_msi(pdev); > - } > + if (vmci_dev->exclusive_vectors) > + free_irq(pci_irq_vector(pdev, 1), vmci_dev); > + free_irq(pci_irq_vector(pdev, 0), vmci_dev); > + pci_free_irq_vectors(pdev); > > tasklet_kill(&vmci_dev->datagram_tasklet); > tasklet_kill(&vmci_dev->bm_tasklet); > diff --git a/include/linux/vmw_vmci_defs.h b/include/linux/vmw_vmci_defs.h > index 1bd31a3..b724ef7 100644 > --- a/include/linux/vmw_vmci_defs.h > +++ b/include/linux/vmw_vmci_defs.h > @@ -54,13 +54,6 @@ > #define VMCI_IMR_DATAGRAM 0x1 > #define VMCI_IMR_NOTIFICATION 0x2 > > -/* Interrupt type. */ > -enum { > - VMCI_INTR_TYPE_INTX = 0, > - VMCI_INTR_TYPE_MSI = 1, > - VMCI_INTR_TYPE_MSIX = 2, > -}; > - > /* Maximum MSI/MSI-X interrupt vectors in the device. */ > #define VMCI_MAX_INTRS 2 > > -- > 2.1.4 > Thanks for the cleanup! Looks great. Reviewed-by: Jorgen Hansen