From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 07/13] IB: add a proper completion queue abstraction Date: Thu, 10 Dec 2015 10:42:22 -0800 Message-ID: <5669C78E.6070302@sandisk.com> References: <1449521512-22921-1-git-send-email-hch@lst.de> <1449521512-22921-8-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: <1449521512-22921-8-git-send-email-hch@lst.de> Sender: linux-kernel-owner@vger.kernel.org To: Christoph Hellwig , linux-rdma@vger.kernel.org Cc: sagig@dev.mellanox.co.il, bart.vanassche@sandisk.com, axboe@fb.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-rdma@vger.kernel.org On 12/07/2015 12:51 PM, Christoph Hellwig wrote: > This adds an abstraction that allows ULP to simply pass a completion ^^^ I think this should either be changed into either "an ULP" or "ULPs". > +/** > + * ib_process_direct_cq - process a CQ in caller context > + * @cq: CQ to process > + * @budget: number of CQEs to poll for > + * > + * This function is used to process all outstanding CQ entries on a > + * %IB_POLL_DIRECT CQ. It does not offload CQ processing to a different > + * context and does not ask from completion interrupts from the HCA. ^^^^ Should this perhaps be changed into "for" ? > + * > + * Note: for compatibility reasons -1 can be passed in %budget for unlimited > + * polling. Do not use this feature in new code, it will be remove soon. ^^^^^^ Did you perhaps intend "removed" ? > +struct ib_cq *ib_alloc_cq(struct ib_device *dev, void *private, > + int nr_cqe, int comp_vector, enum ib_poll_context poll_ctx) > +{ > [ ... ] > + cq->wc = kmalloc_array(IB_POLL_BATCH, sizeof(*cq->wc), GFP_KERNEL); Why is the wc array allocated separately instead of being embedded in struct ib_cq ? I think the faster completion queues can be created the better so if it is possible to eliminate the above kmalloc() call I would prefer that. > --- 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; Is explicit initialization to "{ 0 }" really needed for static structures ? Bart. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755209AbbLJSme (ORCPT ); Thu, 10 Dec 2015 13:42:34 -0500 Received: from mail-bn1bon0077.outbound.protection.outlook.com ([157.56.111.77]:40064 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752067AbbLJSm0 (ORCPT ); Thu, 10 Dec 2015 13:42:26 -0500 Authentication-Results: spf=pass (sender IP is 63.163.107.172) smtp.mailfrom=sandisk.com; dev.mellanox.co.il; dkim=none (message not signed) header.d=none;dev.mellanox.co.il; dmarc=bestguesspass action=none header.from=sandisk.com; X-AuditID: ac160a68-f790b6d00000123b-c0-5669c78e7ec9 Subject: Re: [PATCH 07/13] IB: add a proper completion queue abstraction To: Christoph Hellwig , References: <1449521512-22921-1-git-send-email-hch@lst.de> <1449521512-22921-8-git-send-email-hch@lst.de> CC: , , , , From: Bart Van Assche Message-ID: <5669C78E.6070302@sandisk.com> Date: Thu, 10 Dec 2015 10:42:22 -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: <1449521512-22921-8-git-send-email-hch@lst.de> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpikeLIzCtJLcpLzFFi42JZI8azSLfveGaYwcfXihb/9xxjs1i5+iiT xeVdc9gsnh3qZbHovr6DzeL18adMDmwe02dsYvWY2PyO3WP3zQY2j8+b5AJYorhsUlJzMstS i/TtErgyTh/cx1Jwlb9i2gP/BsZD3F2MnBwSAiYSS9+8Y4ewxSQu3FvP1sXIxSEkcIJRoufW AXYIZwejxPzrZxlhOt5uv8kMkdjEKPF47QlmkISwgIfE6ef7wGwRAQeJGZ9mgo0VEsiWmHDm GRNIA7NAD6PE0Xv7wSaxCRhJfHs/k6WLkYODV0BL4mQT2EksAqoSH1ongvWKCkRITJzQwApi 8woISpyc+YQFxOYUsJbYvrmHDaSVWcBe4sHWMpAws4C8xPa3c8BukxC4ySrxatcdNogb1CVO LpnPNIFRZBaSUbMQ2mchaV/AyLyKUSw3M6c4Nz21wNBQrzgxLyWzOFsvOT93EyM4ZrgydjBu nWR+iFGAg1GJh/eFdGaYEGtiWXFl7iFGCQ5mJRHe7zuBQrwpiZVVqUX58UWlOanFhxilOViU xHmtW9TChATSE0tSs1NTC1KLYLJMHJxSDYyss/5pyCTtu/rFwVT1wsGALZIWZTb96naxFkpZ /+21uBw/LqzzUmROdJrvtnPqgj3a8+eeznkln5XeUasd0rsm+uMK8/OMYWy/C0SEb/vekQ+9 +U1AtMfXe2pb1rTPjGlLhEp3dL/uZ5rT4ZFybqX1vsWPTaekSMq/0PKXyPcwqmS1v+zvrsRS nJFoqMVcVJwIAP9K9kSVAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpmluLIzCtJLcpLzFFi42Lh2siRott3PDPMYOtKYYv/e46xWRz82cZo sXL1USaLy7vmsFk8O9TLYtF9fQebxevjT5kc2D2mz9jE6jGx+R27x+6bDWwe09acZ/L4vEku gDWKyyYlNSezLLVI3y6BK+P0wX0sBVf5K6Y98G9gPMTdxcjJISFgIvF2+01mCFtM4sK99Wxd jFwcQgIbGCUurL7ABJIQFvCQOP18H1iRiICdxPrXTawgtpBAtsSEM8+YQBqYBZoYJe593ckG kmATMJL49n4mSxcjBwevgJbEySawZSwCqhIfWieyg9iiAhESEyc0gM3hFRCUODnzCQuIzSlg LbF9cw/YGGYBW4k7c3czQ9jyEtvfzmGewMg/C0nLLCRls5CULWBkXsUolpuZU5ybnllgaKhX nJiXklmcrZecn7uJERzMnJE7GJ9OND/EyMTBKdXAyPglRUjhWMLP7pdVTbrn/rJLy7DmfchO Vp4hkuK9Ourdiz+PV0ce+JfbfntmZ/crx/DbqkKrJL9VLrY70jp56cPsnGfWBl280vVN0n8/ SpX9P3bDunv2tITgDj8lnxunGxu2iVWu+/XbLXqCaYdY3q3UazzfPlnODoyb+mWJsIONe9Gq RT7lSizFGYmGWsxFxYkAsjGKdRYCAAA= X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11FD002;1:POPM+iZtsmLeHKtRVo4Vl28aQ+FpJEZBxjbC5fwnNUOAq4wqqP2xpPbr5ZLj2dFQRnF7SRT14UXzoVp0sp/t7wc/E2YCLnSty37+DGwEDutIOj/F9EfCvs69t22G7OYU0ydjgwQSanGAWrvdmOd9DZfDGEDEV6HI5K+jD+N9jMlijW1g5L7fb3nQOCvx6VDRsRgt6cACVVOeMsUdbDEtAxjhGxJT361r128CENrpWsLWbHgc3MKaIMHAURcnKJCtuPViaiQ/MW8p66zYAzZFSdLm7WWxAzl4yP+Cv3tYM7iWJdtLHBd7MTd5scwgedOo8ZWAmD3Lk0yfiQUcUFZAS6ecsfhSes9bYjMuEFpxaHFHNane+N5w03daQWvy39pndE4ezS4Y9j0h4jGGxzGyyxOnh2p0KjSMJp3eICFVEBI= X-Forefront-Antispam-Report: CIP:63.163.107.172;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(438002)(24454002)(479174004)(189002)(199003)(377454003)(1096002)(4001350100001)(65806001)(54356999)(69596002)(87936001)(33656002)(97736004)(87266999)(65816999)(5001960100002)(11100500001)(2950100001)(77096005)(92566002)(76176999)(189998001)(50466002)(1220700001)(5001770100001)(586003)(83506001)(80316001)(64126003)(59896002)(65956001)(36756003)(47776003)(81156007)(86362001)(106466001)(23746002)(5008740100001)(50986999)(230700001);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR0201MB0825;H:milsmgep11.sandisk.com;FPR:;SPF:Pass;PTR:InfoDomainNonexistent;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB0825;2:l9iBDqRDLCJdhoVY8mX/h0cmKptkfByfse16pyVpPJJIMcn4GPMV+l52sbvS2BEBpzAc9WCToMl5FydPdSDasSnHZIoPcTXyixF8+k9Gd4He6EYwZEZhpRqU/FUxwwmA9l0M9ZpabYvYlYt1xDDBHQ==;3:hD0nsmKM5ttPNEJgi8dIEbFtdGohyEcoFvbGh11/Dazxr3jnW5Oo/inx/mlKB1s6L1+vG26ACydRXpo6oxnoeiDQ6QZ1gad85GcxLz5+c7ER5sC3S8tmnQWaXtsW+8t42qt44hD0KYn44buJUSSzFOqOOaHX9zMctidTze8KdZW45VLZ4dhBKTaEjCpi1i3BDpAmC3cji6nssY9lFmg5g9t3SdiK+SCn47U6+58i0jetdYYjJyx9QAwDVgCrlDHNomDrr/IOYL6jqrPrzfk6dQ==;25:Hwo99dzremF5lIK1VojkprEJWuooqjhpkeP1JXd/Ef1GpuvAdlkka9LnpJl/vtx2U47knd6v+dXauvaZKV6/guGb55tWpsO9tGWr5Tecw5bMW9NNIT1PDP6A/lDfsCXiNTqtirv3d3O3QtuDH89AP1Sp8y2CT+yU7BS6WZcFmmYmTXiBQdMh5Xp4E0a/g6n62QRQ+T59hqrvEM5bZbgYlb07EEcJroREs65B0G5wLA4GGSPGVVjTsaTQdrLckG66 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(8251501001);SRVR:CY1PR0201MB0825; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB0825;20:bOkTwr9xjE5ups0knh629k1A9PQMrupeXt6r8GRif0RY/DZ/wVSPSucPOcXEv/dzfqh/CccCTlmj187hrC5DUOIvNmSo7X6rJOc/1esAcZpf85KlMQ//qTF4VuxF5KM0oEMiq4DjvPW92bfwrbcIXhDpt/QCrwqgs+lX0XvfCQgKYIaXyJTmMX3U2Bf7jOlTqmoOF0qG4orMfYyNRFIiHpTrETjtxyYEIfadGSLTVNF/Z+Az5bJMfOIj8kFJOmsSAKRj5/6yPD85VIKSYNiKX6mPoQEn0/lJnm4ABHRGU1fCbxBGKbVvyndfyjTk9iGyzy0l4sLES8IiJ7ESefVa+LcoOoU3wTK2l8/eE8MyI5mWn7Jp0uf9g2UnqXWm3UJl5G22xZlwZNySWx6t+JysfKZodeBaYrJM6R5sLvX7WGV1X6DidOr+7J0/tTLf2XqVJH5io1Q90z4PTeXC1+SOQjqID3IdTMjrwrIaQz3p7jQ3tWUnI/Zj4DVKGv5/3ePG;4:hi9fzJnMYFlDcfsQmItk76L9NHgN6Ww+tBJsUPBf9Zy9FHNy74CEgL6Pde9VouSA+T4SgdRcnbYLaiiq/4bZifbYn9jIF0bVpvDqSVcrod7SegT9JFTyeYeBbBd8HQmhLGZE3LrV2ICL40lJClGbMH6O/UchDZNpOFKkoUDMLvAXtIhZEZc+P9Qs8hlNE5pzsfg0qftun9YxCdxSxj7OYPCOmkKu9b3BkpBBFSb+cVbtKQto63aukAiRURlw9lPz1ng0jnAAqeg5yr/soqIDGk/Z0Gl8UOryip74V0Ns2nnjFBHaSJKhdEz1Y+H+SKkPRHGOFERTgy7M29IJga3hDmXV9URsCYiWa2LF86vm7t6Ajj0LlCmFXwz+jU0oqRRC X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(520078)(5005006)(10201501046)(3002001);SRVR:CY1PR0201MB0825;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0201MB0825; X-Forefront-PRVS: 078693968A X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;CY1PR0201MB0825;23:ihGchtYOE+wa1JGV2ag+hq06rGYDsKXvXhz?= =?Windows-1252?Q?8Sprex7NE2wnx5BORNTF+HjRFitkFFy8prN1W3aEWOFUMscjsxW7LoU9?= =?Windows-1252?Q?d1EjflKy8RSXXSHz5YDNT/XFwzINrTpqv/aqD/tVWlqiSfHWCJmNSvdC?= =?Windows-1252?Q?mF1JQacdStpF54k/1/mDng871+RmjNA+HpvCGLw6oP/uEHjuHcalENBq?= =?Windows-1252?Q?FkzAxMeop+bP7nMZZfCU+F4Vs07ConYv9aRArEYdysGNCMI3BEz5FDaV?= =?Windows-1252?Q?/wAP/RAsmUJkhsavCwsFHTRxkVV2dbOyO2aeGKDjZjCLEYWTAf1CTsUC?= =?Windows-1252?Q?Gj03CCLQ6/Ex32a+C50fq12LzDcHoPNNh+TXOTRtmLk4erQhCUr7/2wO?= =?Windows-1252?Q?+bZy+dF3IMC2dH71CANKpF++hjP20If5Lvfi2wLny+1LBN05ZNfmkvrb?= =?Windows-1252?Q?wyF3odPTjZJzNVeXnPOHRkhyQMkrE7T2M8w9X2QwEPKK7/jOvSH/tmAC?= =?Windows-1252?Q?9a86pbBjTuxYx6HGnmrxTObtnBkGE8El9BNSXvAMdHdYsPw0nIuAvLWv?= =?Windows-1252?Q?ozSPtTgw11rFhvg6PimnOYWwXdiqP2qqXHO3n3DZjcgNmr2/R5Z7OOF2?= =?Windows-1252?Q?rt/zVSFSy2Tf6nSbT83MZt9BgaTNDlpUVBHub8PfxQjlwQwIrwr+1mYQ?= =?Windows-1252?Q?HB1mg+61mpSlW6IP5x4zOnIKyw8kxEq1M7lJrfG9FCkcvdtCDNVEogZa?= =?Windows-1252?Q?CFEYLcGABpPFDeXKP6RkF6U7typKlElv2VKaWlevALAuIU8umOERLvYz?= =?Windows-1252?Q?wYIMkBdOFagX3EqAdX5LZ/+bL5b/L8HY7qZ2FMzXN6e/gpgBKjGgev1y?= =?Windows-1252?Q?XUCdNlpOE8h1FMNUx0A+3vXJqyK73RqLc97ba9Z9F0xGJWoQYRHKHy8S?= =?Windows-1252?Q?8Btd8jijKB8GniUBB+Uq41AOx7uhJX7UaXlt0wDSw6x5ntS7qre4EgI5?= =?Windows-1252?Q?ixDux8fS376okTaXbOGzmaZc2GTk6xPVYpaLWTyyABtKzHh7pfesPBpf?= =?Windows-1252?Q?AYXQ1H8GEJu0i/HKWdUmXuQ8sUTTyQaaTTUFltO8PHqj6yt8XMnZ2pgt?= =?Windows-1252?Q?BADcG9OK47v4/519+qUZYIi0Nh/sMck9a0ZeOEOF0crAYiAq9Wiv2V9n?= =?Windows-1252?Q?ls0e5QCvUelsXOzHczh5frEKy1PyshOPVnUQK7qfqbb0sjFHgH/EJ?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR0201MB0825;5:D/dPLJvNUd2eJ1HflLQ1ilMVvCZn91z8Xg70h6E28EyBaJgfskVkAZQfK4xmM65rV8ed1BYYxnU814g3A54Hf3VwUpKSkAPCNFsqSaIIyF0revPnXthp8SjgPwEOYveCBBIY/wp/2VhVvBpjycMuwg==;24:WOT0PTPNSTyYARDxPV8gdHJOSe6XJOlnpc7KRGJpSF7HqnIkcjZye/FoAtxopKjNVBEZovPwlUhkMVwYQaDtLARHcjavsEYW7Yt4jIBD+No=;20:Az5xnx4wyKGezCsRKyxCX/pWmhF/aAIBOTcEN7oMhE4HfxBuBw8d/MIxgeQQ0/SffHVwc7OYd8RahOknbpqntdvSk4dDq/8WOgBOJTi8qNO6M7BvZa7Ibd8KX280uAX0om67bxaQ5jueLgCtmm6a+28EmcN88SmDA3gvVMXWoYebRQ0eGmuCp288rpL20tZhPM++qHGq9n8ZQGe7DuT9ni/rDd98BAZFe3Z+IbABV8s38M0PaseXpLWAFNUL9rzb SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: sandisk.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Dec 2015 18:42:22.6175 (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.172];Helo=[milsmgep11.sandisk.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0201MB0825 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/07/2015 12:51 PM, Christoph Hellwig wrote: > This adds an abstraction that allows ULP to simply pass a completion ^^^ I think this should either be changed into either "an ULP" or "ULPs". > +/** > + * ib_process_direct_cq - process a CQ in caller context > + * @cq: CQ to process > + * @budget: number of CQEs to poll for > + * > + * This function is used to process all outstanding CQ entries on a > + * %IB_POLL_DIRECT CQ. It does not offload CQ processing to a different > + * context and does not ask from completion interrupts from the HCA. ^^^^ Should this perhaps be changed into "for" ? > + * > + * Note: for compatibility reasons -1 can be passed in %budget for unlimited > + * polling. Do not use this feature in new code, it will be remove soon. ^^^^^^ Did you perhaps intend "removed" ? > +struct ib_cq *ib_alloc_cq(struct ib_device *dev, void *private, > + int nr_cqe, int comp_vector, enum ib_poll_context poll_ctx) > +{ > [ ... ] > + cq->wc = kmalloc_array(IB_POLL_BATCH, sizeof(*cq->wc), GFP_KERNEL); Why is the wc array allocated separately instead of being embedded in struct ib_cq ? I think the faster completion queues can be created the better so if it is possible to eliminate the above kmalloc() call I would prefer that. > --- 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; Is explicit initialization to "{ 0 }" really needed for static structures ? Bart.