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=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=ham 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 31754C43381 for ; Wed, 20 Feb 2019 16:54:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0578F2084D for ; Wed, 20 Feb 2019 16:54:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="HPa+/ffN" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726412AbfBTQx6 (ORCPT ); Wed, 20 Feb 2019 11:53:58 -0500 Received: from mail-pg1-f196.google.com ([209.85.215.196]:44370 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725885AbfBTQx6 (ORCPT ); Wed, 20 Feb 2019 11:53:58 -0500 Received: by mail-pg1-f196.google.com with SMTP id y1so12133066pgk.11 for ; Wed, 20 Feb 2019 08:53:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=iv2iwGmP+7W6uBfG7l16/2Jyy5yBqYdmsKkaiXnHk8g=; b=HPa+/ffNjc/eMmOzektAt77m690Bp1x/Snb5hP1AEo0SskpsUwB86teBZs/jfJOiuU OOoPYoyBrYcZXwullI3ny3hL2HF/I2WIuTHX40HxfHld5G9Y5pTipXTEQlRRjHFiEFa4 43/Hv+wOYYIhscPbW02Pz+qp/nJsc/L3v3uD/yVEqWC8gr2Dc/LX5w8bcslXEzgPOMqE a8xWwK/t40eFE7EekbbPGt1JN01bCZAbQMdQ/106gaaKGUjUsih6QlhxvskZjfmO48uq 7Tunp0ys2F7s0ka8kDHyz/CWs+dWdIP2uPYpi94I7XASo70YgjE9KfEPpOHG64MOSmpa 4/6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=iv2iwGmP+7W6uBfG7l16/2Jyy5yBqYdmsKkaiXnHk8g=; b=S8kYjdEJoGB2nq5xS3Cv/roQ+GscABFzM1njxdprlD7V1ipuJ/HQA+KJ+qMmKa0mTd gxb0j501qOUPZYEeAqT81/YLKSqcLBzm/qgmx/wRyJayJ7bN9Gzyqlia+gz2ghv+yDQP z/aJi8oJxTIW8p0HoGP7FLZHRxPlVi08ywsMfL0jImCAGdwdiiMWUUBEf3uM46xP9sX6 UY6ztakM/6+CpZ+sqWXsHhQeTOJWGZKbCyYVfvVZXAujk4AdRxioLRfq8NA20OtkaafI 0mKaZC+3p3VBVl+QWtKch5bYXoymlErxEdTSxlDSsZvDOqmyKHWW2aWgm9n7+5703PdG 5nZw== X-Gm-Message-State: AHQUAuYDdwaHz4vgJavOl+2BqW0IyVc1cLq4a3udheSz4J1GyEdZQ4bt WJQ34XIWxqCL/kU++9zE8UI6SA== X-Google-Smtp-Source: AHgI3IaT+NRVr5PbXbvX4rwmiaUtCHsT/YwYS30xaAdk/7IADo6OVmUPYZqvl/fU5K6U6ZBmEwcwVA== X-Received: by 2002:a63:f412:: with SMTP id g18mr30244541pgi.262.1550681637252; Wed, 20 Feb 2019 08:53:57 -0800 (PST) Received: from ziepe.ca (S010614cc2056d97f.ed.shawcable.net. [174.3.196.123]) by smtp.gmail.com with ESMTPSA id e7sm21508792pgv.6.2019.02.20.08.53.56 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Feb 2019 08:53:56 -0800 (PST) Received: from jgg by mlx.ziepe.ca with local (Exim 4.90_1) (envelope-from ) id 1gwV87-0000Yf-Sp; Wed, 20 Feb 2019 09:53:55 -0700 Date: Wed, 20 Feb 2019 09:53:55 -0700 From: Jason Gunthorpe To: "Saleem, Shiraz" Cc: "dledford@redhat.com" , "davem@davemloft.net" , "linux-rdma@vger.kernel.org" , "netdev@vger.kernel.org" , "Ismail, Mustafa" , "Kirsher, Jeffrey T" Subject: Re: [RFC v1 15/19] RDMA/irdma: Add miscellaneous utility definitions Message-ID: <20190220165355.GG8429@ziepe.ca> References: <20190215171107.6464-1-shiraz.saleem@intel.com> <20190215171107.6464-16-shiraz.saleem@intel.com> <20190215174714.GE30706@ziepe.ca> <9DD61F30A802C4429A01CA4200E302A7A5A460B3@fmsmsx124.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A460B3@fmsmsx124.amr.corp.intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Feb 20, 2019 at 02:53:18PM +0000, Saleem, Shiraz wrote: > >Subject: Re: [RFC v1 15/19] RDMA/irdma: Add miscellaneous utility definitions > > > >On Fri, Feb 15, 2019 at 11:11:02AM -0600, Shiraz Saleem wrote: > >> From: Mustafa Ismail > >> > >> Add miscellaneous utility functions and headers. > >> > > [....] > > > > >> +#define to_device(ptr) \ > >> + (&((struct pci_dev *)((ptr)->hw->dev_context))->dev) > > > >?? Seems like this wants to be container_of?? > > Yes. > > > > >> +/** > >> + * irdma_insert_wqe_hdr - write wqe header > >> + * @wqe: cqp wqe for header > >> + * @header: header for the cqp wqe > >> + */ > >> +static inline void irdma_insert_wqe_hdr(__le64 *wqe, u64 hdr) { > >> + wmb(); /* make sure WQE is populated before polarity is set */ > >> + set_64bit_val(wqe, 24, hdr); > > > >Generally don't like seeing wmbs in drivers.. Are you sure this isn't supposed to > >be smp_store_release(), or dma_wmb() perhaps? > > Why is wmb() an issue in drivers? There are many ways to get a barrier that are much clearer and maintainable then some random wmb. Rarely do drivers actually need wmb. > >> +/** > >> + * irdma_allocate_dma_mem - Memory alloc helper fn > >> + * @hw: pointer to the HW structure > >> + * @mem: ptr to mem struct to fill out > >> + * @size: size of memory requested > >> + * @alignment: what to align the allocation to */ enum > >> +irdma_status_code irdma_allocate_dma_mem(struct irdma_hw *hw, > >> + struct irdma_dma_mem *mem, > >> + u64 size, > >> + u32 alignment) > >> +{ > >> + struct pci_dev *pcidev = (struct pci_dev *)hw->dev_context; > >> + > >> + if (!mem) > >> + return IRDMA_ERR_PARAM; > >> + > >> + mem->size = ALIGN(size, alignment); > >> + mem->va = dma_alloc_coherent(&pcidev->dev, mem->size, > >> + (dma_addr_t *)&mem->pa, GFP_KERNEL); > >> + if (!mem->va) > >> + return IRDMA_ERR_NO_MEMORY; > >> + > >> + return 0; > >> +} > > > >More wrappers? Why? > > I agree on your previous comment on wrapper for usecnt tracking but this does > consolidate some common code and avoid duplication. IRDMA_ERR_PARAM seems like an nonsense thing to do, and why does this driver have its own custom error codes anyhow? Don't like it, it is very stange. Once you strip that away there is no code to share. Jason