From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 2/9] IB: add a proper completion queue abstraction Date: Tue, 17 Nov 2015 09:52:58 -0800 Message-ID: <564B697A.2020601@sandisk.com> References: <1447422410-20891-1-git-send-email-hch@lst.de> <1447422410-20891-3-git-send-email-hch@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1447422410-20891-3-git-send-email-hch-jcswGhMUV9g@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Christoph Hellwig , linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org, bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org, axboe-b10kYP2dOMg@public.gmane.org, linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-rdma@vger.kernel.org On 11/13/2015 05:46 AM, Christoph Hellwig wrote: > + * context and does not ask from completion interrupts from the HCA. ^^^^ Should this perhaps be changed into "for" ? > + */ > +void ib_process_cq_direct(struct ib_cq *cq) > +{ > + WARN_ON_ONCE(cq->poll_ctx != IB_POLL_DIRECT); > + > + __ib_process_cq(cq, INT_MAX); > +} > +EXPORT_SYMBOL(ib_process_cq_direct); My proposal is to drop this function and to export __ib_process_cq() instead (with or without renaming). That will allow callers of this function to compare the poll budget with the number of completions that have been processed and use that information to decide whether or not to call this function again. > +static void ib_cq_poll_work(struct work_struct *work) > +{ > + struct ib_cq *cq = container_of(work, struct ib_cq, work); > + int completed; > + > + completed = __ib_process_cq(cq, IB_POLL_BUDGET_WORKQUEUE); > + if (completed >= IB_POLL_BUDGET_WORKQUEUE || > + ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0) > + queue_work(ib_comp_wq, &cq->work); > +} > + > +static void ib_cq_completion_workqueue(struct ib_cq *cq, void *private) > +{ > + queue_work(ib_comp_wq, &cq->work); > +} The above code will cause all polling to occur on the context of the CPU that received the completion interrupt. This approach is not powerful enough. For certain workloads throughput is higher if work completions are processed by another CPU core on the same CPU socket. Has it been considered to make the CPU core on which work completions are processed configurable ? > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 62b6cba..3027824 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -457,10 +457,11 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct srp_target_port *target) > static void srp_destroy_qp(struct srp_rdma_ch *ch) > { > static struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR }; > - static struct ib_recv_wr wr = { .wr_id = SRP_LAST_WR_ID }; > + static struct ib_recv_wr wr = { 0 }; > struct ib_recv_wr *bad_wr; > int ret; Since the 'wr' structure is static I don't think it needs to be zero-initialized explicitly. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932145AbbKQRxI (ORCPT ); Tue, 17 Nov 2015 12:53:08 -0500 Received: from mail-bn1bon0096.outbound.protection.outlook.com ([157.56.111.96]:7687 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932069AbbKQRxD (ORCPT ); Tue, 17 Nov 2015 12:53:03 -0500 Authentication-Results: spf=pass (sender IP is 63.163.107.173) smtp.mailfrom=sandisk.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=bestguesspass action=none header.from=sandisk.com; X-AuditID: ac160a69-f79f76d000007db2-c9-564b697bc732 Subject: Re: [PATCH 2/9] IB: add a proper completion queue abstraction To: Christoph Hellwig , References: <1447422410-20891-1-git-send-email-hch@lst.de> <1447422410-20891-3-git-send-email-hch@lst.de> CC: , , , , From: Bart Van Assche Message-ID: <564B697A.2020601@sandisk.com> Date: Tue, 17 Nov 2015 09:52:58 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1447422410-20891-3-git-send-email-hch@lst.de> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpmkeLIzCtJLcpLzFFi42JZI8azSLc60zvM4OFrdov/e46xWaxcfZTJ 4vKuOWwWzw71slh0X9/BZvH6+FMmBzaP6TM2sXpMbH7H7rH7ZgObx+dNcgEsUVw2Kak5mWWp Rfp2CVwZ5zZ+Zy7oFar4tfsJSwPjCb4uRk4OCQETidcPVjNB2GISF+6tZ+ti5OIQEjjBKHH+ yQMoZwejxLE33YwwHYeaLkIlNjFK7FmzlBkkISzgJrFr/gkWEFtEwEFixqeZ7CC2kEC2xPRT m1lBGpgFehgljt7bDzaJTcBI4tv7mWANvAJaEkf6lgA1cHCwCKhKvPjIDxIWFYiQmDihgRWi RFDi5MwnYOWcAtYSzR132UDKmQXsJR5sLQMJMwvIS2x/O4cZZJWEwFVWiXn3F7NB3KAucXLJ fKYJjCKzkIyahdA+C0n7AkbmVYxiuZk5xbnpqQWGRnrFiXkpmcXZesn5uZsYwVHDlbmDccUk 80OMAhyMSjy8Ase9woRYE8uKK3MPMUpwMCuJ8HJaeYcJ8aYkVlalFuXHF5XmpBYfYpTmYFES 57VuUQsTEkhPLEnNTk0tSC2CyTJxcEo1MPZEuDOVXFd5WfJge1Dv55xtj1e3HN/tPfkE67/9 mi/fBVscNS+I51FZK35gH9fh0pQ7izS5iiJftR/s2RA5QfIB60T/aezKgjM2rE88vGwi40s9 gwMOpx9GsJlnquS2PjhTO2nf++3n5p5NkD/jyXnz/ufTzfe1228wX57I9V8obtfsSVa/+kuV WIozEg21mIuKEwGO94eLlgIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmphluLIzCtJLcpLzFFi42Lh2siRoluV6R1m0D7DxOL/nmNsFgd/tjFa rFx9lMni8q45bBbPDvWyWHRf38Fm8fr4UyYHdo/pMzaxekxsfsfusftmA5vHtDXnmTw+b5IL YI3isklJzcksSy3St0vgyji38TtzQa9Qxa/dT1gaGE/wdTFyckgImEgcarrIBmGLSVy4tx7I 5uIQEtjAKLH06xJ2kISwgJvErvknWEBsEQE7ifWvm1hBbCGBbInppzazgjQwCzQxStz7uhNs EpuAkcS39zPBGngFtCSO9IEM4uBgEVCVePGRHyQsKhAhMXFCAytEiaDEyZlPwMo5Bawlmjvu go1hFrCVuDN3NzOELS+x/e0c5gmM/LOQtMxCUjYLSdkCRuZVjGK5mTnFuemZBYaGesWJeSmZ xdl6yfm5mxjB4cwZuYPx6UTzQ4xMHJxSDYxZC74WVqbM+Oj56FPZlSO2Js5/mFfMTefUWRog /JjpgagVV2FE4LP1B+tVG3/8Prp4CivDDY4rPtf+TOYS/XXg2PtfMVHH6za97FAtqZ4ozP3R LvLoblfmLzsv1Nsdr7TnynOQ3vNhXvPtM7p+E700nSbeXnW2ReL88/7O6asuqHwKPcXo3tKj xFKckWioxVxUnAgAsxsqoxcCAAA= X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11FD006;1:M2+3eiKVhxsNRmpXm7nDub6BaugfmXh/9MdmQZx8vvdZ6f+Ba+Wgb77yfqpnw85A/hzUgb/s0OkkSgE0cWFuxi0r5r1hkNXlWnJFn1AxN75L/lj2Nb2BqAoXPDyxX+fiVHop0766j9P3EnGfh/slgEhv3mnVLwMHq5mASXegPZgypKn1gKWL6INvsW3umN54GeUEJQUm7iguGAYu8RG1B3hCPpnTWl+WnE+7I/ataNMNnQEGexk6lbmPpiq5o6uj32A0kQYjYnbwnaLt1ChwvQ+/i+bll0ipDVhXkGVHENWATJeHIHxwJ4MPKHSL4aVbxTGXvjNjmF2XVIC/Ck8u1ouhvZ219ECmSB5zjAIjeiIAKbqSmcFYoOgWTZGbL73JzREjghTETrib923pQGXAAg== X-Forefront-Antispam-Report: CIP:63.163.107.173;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(438002)(377454003)(479174004)(199003)(189002)(24454002)(11100500001)(561944003)(47776003)(59896002)(64126003)(65956001)(5001770100001)(86362001)(36756003)(76176999)(23746002)(81156007)(65816999)(80316001)(54356999)(87266999)(586003)(65806001)(5008740100001)(33656002)(69596002)(77096005)(50466002)(50986999)(83506001)(92566002)(5001960100002)(4001350100001)(230700001)(189998001)(5007970100001)(87936001)(2950100001)(97736004)(106466001)(3076002);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR02MB1388;H:milsmgep12.sandisk.com;FPR:;SPF:Pass;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM2PR02MB1388;2:yq7BHayKNe7SjBf+Q+XPnBlyNxvyaVCiCVY4dRV36pxdbRD7RYyHJ5QM9o9nLzeTtjx6Ot80YQzQC0Cebi4RnDlcCbC0Iod0gDacGObiYHQxW0YLJdb445dNP+lRtcD5vixHvXuUMYmKIXNjetmGd7F7l2wLMoq2j3rz19/nXA8=;3:gygw76wUpDt+agxTJIY7Gx7fUtCBhvCewWHKfGIGMgVqC50Qm6KPYQbIYBePMNqrXMg2nR3pfwOoabOUpWOZA0OhcxJt6G6C7SnA+JWk76mYVqxoWAjgGvhjqwg9QBXHLN+78WkhECp/npgVzv9gUBtdnXbjAntmOSH4HZhYMJusCAL+dit7DrAePVEd+hGMRJpJPlNI8s7srKb+ZiMmzolymtnB7O33uS6SMEVy9kHinqMFJKmZ98Oz743TSDmfwNV+4YKhK2NNcyBKZlUeIA==;25:emRj44+DW2bAHd+e/3hWb5QDI9IBYh1S92Y/FHz7pxTvw6BHU2AuCHW75ldjYAKcJCJsa/fDVp6zj7PCEUQdsO3x98Ek+399OW8r04pfP9RIQD/JG6rJjtroUSRvdSGgBfn6+NINO0Uz87zFm4u8L8fRW8+Pwtzzdmj8gRPncGzzKv0UfGthxjUroni7vbbMnMFUL0TsMqv3KBDFMGwFtwgcI99eEt9lA2UlHDBQ/Yo6Z862fDDF8iaDuOoDD6UjsyBNlge1brLntuxKPXLeAw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(8251501001);SRVR:DM2PR02MB1388; X-Microsoft-Exchange-Diagnostics: 1;DM2PR02MB1388;20:YNh0heV5DDK50Og6jgWQjT4WsLxn4q7HwLpPmKBZgd8Nclr+sXZdaBH1DQY+VYNUAvRdXPpPCxG3BPyIn4RB7HDT0Ap1cl/upLTJO2AmjZsYm9iDJcI34ev7KH7+omypLzJ2XfHRsdO4w3LzYZyQgiTYt6QHY168pcylw9TUws2BIvuRVhjFGmsmChnZSEqhuDGPZwBXNvWAyukbAjyA+wHEi7e+WlPbP1vafdKwbrvxzAaBbgubroj4WKqW0ouW0gMP6dzwDltSbr9vxrVm+4bqghjpI0V9RbE04hphFfFXFaT+mseMI2jfKnOJCB+w5bf6qaz14kjnJVGNHOg5j7H137f0WmghxZg577Ea0qbVcQpYXBpTi3GU0nnNw2nOEewQASNmm5Uub8Fu06xkKc4Dg6Zhghxo0oXtOvI6IhywmBTT8RSDSkQ56za1tpUu5KgrHFVSw+erQKSCW8vfUZwt2o+6JelGnxQsvq5RvuUEp4bUIH2DhxaLNnTCMRy3;4:hrpcOFk0gcSc1YPOkzjaFJum8Cz6bO43/TOl/C2ErNRUKtrhjg4jC/phiLs6dRY/L7PZ6fTGOXGxj/84cyyCKT/t/SRQ6vTBZJqcSBjb4gnOmiSoOzrikuYuuwS9PXaVDCiuNSHXCo8PQ7h1hUQCp1sMLXL94104Xbc00RJGUaBMrV58x0aUEf1TSTlU9DaVg91K+h97i3msFEfThJK2eq869wkeVZydOQAVy+JeGvzgtcDBKrEvfdR0gemJ1Qrw4b363hSaQVcuUiyoiqeJtkA/tpoEH0Qwzwk2VOAPM0Xje3eQeiHnpg2lSC/2PxCH/9dgmTvDoHCdgVWfqeeN+K9wkX2N7ApPivCs1Vzr6zzUF6lIXKdgCD/788S2xjdZ X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(10201501046)(3002001);SRVR:DM2PR02MB1388;BCL:0;PCL:0;RULEID:;SRVR:DM2PR02MB1388; X-Forefront-PRVS: 07630F72AD X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;DM2PR02MB1388;23:BigYWj689gIKqqyE66wLZvJ04iGFdy/1dbv8z?= =?Windows-1252?Q?7DcwI9/PPFSZyvw79lqoGsD3vct2MYa//UDErLnbeaxqJ3gBj480+NmJ?= =?Windows-1252?Q?GL+X0KUkgGH7o2Xll5uA7IpXtxCOTw95e5NJqSL98pGQZCXfpugsKW8F?= =?Windows-1252?Q?GdHd9HvEDxMmBcnKJg0pek3LAOlztpxD40jiZMCoU2QDacE6aoPFJT6s?= =?Windows-1252?Q?JEHF3e1qJJ98oQJUaC5r4BaBxe60cYJDJY03+mapaxV8hhpPVuNGodd5?= =?Windows-1252?Q?rPUde1SDSQgpRlElot9CFJIoTq0MzhSi0Aa/rBgO8CA3Ny1x1OSCVQHH?= =?Windows-1252?Q?jM/e9GFVC+bShhtEZyK+79Aj6L2V8DRGCAKuCvfo9a74f5OENdiewKYJ?= =?Windows-1252?Q?t8+1tqaGhM/Aj2ZDSO1NLEiq9gTfCFcT0VkeQhb37EaGQE6mUxQgDDCM?= =?Windows-1252?Q?8PJMilJ6xiXy4xnxVrZVQmYmA2GULuRi30tUwlckTZQPVo+QwY0eJaY3?= =?Windows-1252?Q?27YIhnAyiMOYOEEEe7ZHRFtd4LovIM8XgDgw8+7Q1RO9ahKBq11OPQg2?= =?Windows-1252?Q?t9WWrjmUCISuWRzOsnX8RfLzNyuBFJ85ZKO+65pFflbZ5dvoy/5TRPXl?= =?Windows-1252?Q?Q5uSVOLHkZQz11O0MWj0gh4WEmrVEyqVarOVNp40DESYkAIJCQBi7To/?= =?Windows-1252?Q?iw/jt53yjj/jSFeuZhOJe1enAUXGrgMqzWOmGUN3rFRhU8HYXNi/sqv2?= =?Windows-1252?Q?iKo/ucqZr/ihjuRlV4oe+NVVk4UEY4YiYwNDJESMjCu52Hd5DHxCeEH1?= =?Windows-1252?Q?idDrTuN/0Z8HilaGVYKDW966NTDIlfla31AR0nVCXyivMdjCma9kACJu?= =?Windows-1252?Q?6zqlLKpczC8EBaEGF0lnUYp30xBdX8cnXiTLjClGOSGlbuWP1G/nd2um?= =?Windows-1252?Q?3TBZfsYnOqwLc/4V4ppvfkEyM52lFclMl0OmX9HKtHAp3x7yDtGK/N23?= =?Windows-1252?Q?CreFKyjqx6H1szK4JB9vkxAhfa6JsyQIv1z+O2Ws3blunXZhamn/6YGN?= =?Windows-1252?Q?9bXj4AKOHkYG1uQabfEQgyEnlvhTaKDXLajRZeZp/Rr9IbQTa5KjgVb3?= =?Windows-1252?Q?tO9m1XWleXoGAx7imM97Oi7ElOSqXudkhHE++9NqR0Don3FFPjN4bLf1?= =?Windows-1252?Q?59s/5gV2CCCux4Zds1D+CPB3t0/0KsCU/J3wbMK8n06k//EW9MBPwLzv?= =?Windows-1252?Q?bxeUuGXylWYAfhpXg=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM2PR02MB1388;5:Cr6UfSku2hBF4bBtVGZSeiR8l9KPkAYviYJEHw78ObTbInfAfzk5H3fZJnS0C0x4mH6u0v9WuTyz+LgBfbWH95GTHV8ndHLw4mQ2kcxv+KfcUhnjYl3gVKH4sh7SMdRJjSW2Z/fGN6XLUG+xbwCwUw==;24:PITZG9Zex6gQMly09TZXt0oOY2VuNSBaC9/tqb8ACSbfsSjYoLlGJatrAIvUeHrq8LDExMp+D9ciUduqi6otGK9igEMXoUFVkG24gWd6rXY=;20:8O7+w754rvtbQ+msxuVZNUSi22LR6nAuEbyF8s7VAy0Cs0t46uOwOeES3nzkuRynyAR9jn2C4oeYm3wjGjpyM+8888S+/D9M4EDnqniMD4i/oDoYI2Iz68VSAEPFQsKkIHcvZ3Ky5A+9lotM/MZejDJGtRRv9qiWxRQKJNVM2VwLAEB0iD//L3OJiGdoKnCqc8iZ4nPzHRS17q20OAMYTXcu8Lk1iKo+vmcwmT1mPXREmNJBd8sQLO0e6CHUGCt1 SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: sandisk.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2015 17:52:59.1992 (UTC) X-MS-Exchange-CrossTenant-Id: fcd9ea9c-ae8c-460c-ab3c-3db42d7ac64d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=fcd9ea9c-ae8c-460c-ab3c-3db42d7ac64d;Ip=[63.163.107.173];Helo=[milsmgep12.sandisk.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR02MB1388 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/13/2015 05:46 AM, Christoph Hellwig wrote: > + * context and does not ask from completion interrupts from the HCA. ^^^^ Should this perhaps be changed into "for" ? > + */ > +void ib_process_cq_direct(struct ib_cq *cq) > +{ > + WARN_ON_ONCE(cq->poll_ctx != IB_POLL_DIRECT); > + > + __ib_process_cq(cq, INT_MAX); > +} > +EXPORT_SYMBOL(ib_process_cq_direct); My proposal is to drop this function and to export __ib_process_cq() instead (with or without renaming). That will allow callers of this function to compare the poll budget with the number of completions that have been processed and use that information to decide whether or not to call this function again. > +static void ib_cq_poll_work(struct work_struct *work) > +{ > + struct ib_cq *cq = container_of(work, struct ib_cq, work); > + int completed; > + > + completed = __ib_process_cq(cq, IB_POLL_BUDGET_WORKQUEUE); > + if (completed >= IB_POLL_BUDGET_WORKQUEUE || > + ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0) > + queue_work(ib_comp_wq, &cq->work); > +} > + > +static void ib_cq_completion_workqueue(struct ib_cq *cq, void *private) > +{ > + queue_work(ib_comp_wq, &cq->work); > +} The above code will cause all polling to occur on the context of the CPU that received the completion interrupt. This approach is not powerful enough. For certain workloads throughput is higher if work completions are processed by another CPU core on the same CPU socket. Has it been considered to make the CPU core on which work completions are processed configurable ? > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 62b6cba..3027824 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -457,10 +457,11 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct srp_target_port *target) > static void srp_destroy_qp(struct srp_rdma_ch *ch) > { > static struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR }; > - static struct ib_recv_wr wr = { .wr_id = SRP_LAST_WR_ID }; > + static struct ib_recv_wr wr = { 0 }; > struct ib_recv_wr *bad_wr; > int ret; Since the 'wr' structure is static I don't think it needs to be zero-initialized explicitly. Bart.