From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A841FC33CAA for ; Mon, 20 Jan 2020 13:30:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 804B122314 for ; Mon, 20 Jan 2020 13:30:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1579527012; bh=6dnPLM8mNQSGiS/EjQyZOON1MxnO1ZCcpX5QnQbw6YU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=C0G52c5I1r/FjH4BhjAnEMq6TS20D+9K+vsKU50r7SaMhzNUo2c4eVL6oovKyliR+ vw/mncyEg2QIVdlN7g+o7mDWrMTJWAY6I72OyJyasZSV96YAsukB+SXCliCCzGtJ2W xaLMkdkcvd8DSZlL4C+6KQbM85iXF6YK+Sl/oKaw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726728AbgATNaL (ORCPT ); Mon, 20 Jan 2020 08:30:11 -0500 Received: from mail.kernel.org ([198.145.29.99]:54044 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726626AbgATNaL (ORCPT ); Mon, 20 Jan 2020 08:30:11 -0500 Received: from localhost (unknown [213.57.247.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 84FDC21835; Mon, 20 Jan 2020 13:30:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1579527010; bh=6dnPLM8mNQSGiS/EjQyZOON1MxnO1ZCcpX5QnQbw6YU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=wx2UcaasQHgZBWELEmnGYbwZ3GNliteogm9+qeDJrAfM/Pzdu6AHJpOSptyE5CBIS bc5Q4dchas/yFzl4eA3ExGkNXtTt+Luid93rPKphFwEujtE1enYEdTpq8cgLR9Omgj sMStJH5uouN+fSPZhEzHn/YhsCXp1OEoAtwf8OuQ= Date: Mon, 20 Jan 2020 15:30:06 +0200 From: Leon Romanovsky To: Jinpu Wang Cc: Jack Wang , linux-block@vger.kernel.org, linux-rdma@vger.kernel.org, Jens Axboe , Christoph Hellwig , Sagi Grimberg , Bart Van Assche , Doug Ledford , Jason Gunthorpe , Danil Kipnis , Roman Penyaev Subject: Re: [PATCH v7 04/25] RDMA/rtrs: core: lib functions shared between client and server modules Message-ID: <20200120133006.GG51881@unreal> References: <20200116125915.14815-1-jinpuwang@gmail.com> <20200116125915.14815-5-jinpuwang@gmail.com> <20200119144837.GE51881@unreal> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org On Mon, Jan 20, 2020 at 12:32:00PM +0100, Jinpu Wang wrote: > On Sun, Jan 19, 2020 at 3:48 PM Leon Romanovsky wrote: > > > > On Thu, Jan 16, 2020 at 01:58:54PM +0100, Jack Wang wrote: > > > From: Jack Wang > > > > > > This is a set of library functions existing as a rtrs-core module, > > > used by client and server modules. > > > > > > Mainly these functions wrap IB and RDMA calls and provide a bit higher > > > abstraction for implementing of RTRS protocol on client or server > > > sides. > > > > > > Signed-off-by: Danil Kipnis > > > Signed-off-by: Jack Wang > > > --- > > > drivers/infiniband/ulp/rtrs/rtrs.c | 597 +++++++++++++++++++++++++++++ > > > 1 file changed, 597 insertions(+) > > > create mode 100644 drivers/infiniband/ulp/rtrs/rtrs.c > > > > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c > > > new file mode 100644 > > > index 000000000000..7b84d76e2a67 > > > --- /dev/null > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs.c > > > @@ -0,0 +1,597 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +/* > > > + * RDMA Transport Layer > > > + * > > > + * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved. > > > + * > > > + * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved. > > > + * > > > + * Copyright (c) 2019 - 2020 1&1 IONOS SE. All rights reserved. > > > + */ > > > +#undef pr_fmt > > > +#define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt > > > + > > > +#include > > > +#include > > > + > > > +#include "rtrs-pri.h" > > > +#include "rtrs-log.h" > > > + > > > +MODULE_DESCRIPTION("RDMA Transport Core"); > > > +MODULE_LICENSE("GPL"); > > > + > > > +struct rtrs_iu *rtrs_iu_alloc(u32 queue_size, size_t size, gfp_t gfp_mask, > > > + struct ib_device *dma_dev, > > > + enum dma_data_direction dir, > > > + void (*done)(struct ib_cq *cq, struct ib_wc *wc)) > > > +{ > > > + struct rtrs_iu *ius, *iu; > > > + int i; > > > + > > > + WARN_ON(!queue_size); > > > + ius = kcalloc(queue_size, sizeof(*ius), gfp_mask); > > > + if (unlikely(!ius)) > > > + return NULL; > > > > Let's do not add useless WARN_ON() and unlikely to every error path. > I can remove the WARN_ON, but the unlikey for error case seems normal to use, > small size memory allocation is unlikely to fail. The unlikely() makes sense in data-path only and mostly it doesn't give any performance advantage, kcalloc() in this function means that this function is not performance oriented. As general note, as long as this code will leave impression of not-stable enough, we will have hard time to accept it. The overuse of WARN_ON(), debug prints and micro-optimizations add to such feeling. Thanks