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=-4.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 4CD04C41604 for ; Tue, 6 Oct 2020 04:32:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 09E44208C3 for ; Tue, 6 Oct 2020 04:32:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1601958733; bh=xijHYrVizK0LARcsEQR9L2RFOjoZdVPTJmXmRyDE3Wc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=sH9IKfvaCC9KdnF17oxHtEwYWxJpg72L7zh28cVELKPUPD6Ad5dnmfjm6XeZy11RE RWFT39zLuGr1a8MXw0Fi81QuAT+9WosJ221536et/ba6b/ZuwhMdM0HEs/7m/qR5nf Qy9w2j023e/oUr9x0/qbmsrnCp5myp17tBQcqUQM= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726897AbgJFEcL (ORCPT ); Tue, 6 Oct 2020 00:32:11 -0400 Received: from mail.kernel.org ([198.145.29.99]:32878 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726101AbgJFEcL (ORCPT ); Tue, 6 Oct 2020 00:32:11 -0400 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 37B722075A; Tue, 6 Oct 2020 04:32:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1601958731; bh=xijHYrVizK0LARcsEQR9L2RFOjoZdVPTJmXmRyDE3Wc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KszoDIS4fp1NS7s2ADVB3V5fFISop5lBK/zHlwYO5fL9XP6qQj2wHRZt0RFuhGSTg TqVH4h98suMnB13Atw03FFzMIqx0jXEN9LbK1kOqGJRF2b0t+ZwXYAtbQxk2CFww1n NPi79u+OdQbu/NZSheU/5AtOM2XpURcpFrKU+6a0= Date: Tue, 6 Oct 2020 07:32:07 +0300 From: Leon Romanovsky To: Parav Pandit Cc: Christoph Hellwig , Doug Ledford , Jason Gunthorpe , Adit Ranadive , Ariel Elior , Bernard Metzler , Christian Benvenuti , Dennis Dalessandro , Devesh Sharma , Faisal Latif , Gal Pressman , Lijun Ou , "linux-rdma@vger.kernel.org" , Michal Kalderon , Mike Marciniszyn , Naresh Kumar PBS , Nelson Escobar , Parvi Kaustubhi , Potnuri Bharat Teja , Selvin Xavier , Shiraz Saleem , Somnath Kotur , Sriharsha Basavapatna , VMware PV-Drivers , Weihang Li , "Wei Hu(Xavier)" , Yishai Hadas , Yanjun Zhu Subject: Re: [PATCH rdma-next v1] RDMA: Explicitly pass in the dma_device to ib_register_device Message-ID: <20201006043207.GC1874917@unreal> References: <20201005110050.1703618-1-leon@kernel.org> <20201005135110.GA12620@infradead.org> <20201005145824.GB1874917@unreal> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Tue, Oct 06, 2020 at 02:59:32AM +0000, Parav Pandit wrote: > > > > From: Leon Romanovsky > > Sent: Monday, October 5, 2020 8:28 PM > > > > On Mon, Oct 05, 2020 at 02:51:10PM +0100, Christoph Hellwig wrote: > > > > > > > > -static void setup_dma_device(struct ib_device *device) > > > > +static void setup_dma_device(struct ib_device *device, > > > > + struct device *dma_device) > > > > { > > > > + if (!dma_device) { > > > > /* > > > > + * If the caller does not provide a DMA capable device then > > the > > > > + * IB device will be used. In this case the caller should fully > > > > + * setup the ibdev for DMA. This usually means using > > > > + * dma_virt_ops. > > > > */ > > > > +#ifdef CONFIG_DMA_OPS > > > > + if (WARN_ON(!device->dev.dma_ops)) > > > > + return; > > > > +#endif > > > > > > Per the discussion last round I think this needs to warn if the ops is > > > not dma_virt_ops, or even better force dma_virt_ops here. > > > > > > Something like: > > > > > > if (!dma_device) { > > > if > > (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_VIRT_OPS))) > > > return -EINVAL; > > > device->dev.dma_ops = &dma_virt_ops; > > > > > > > + if (WARN_ON(!device->dev.dma_parms)) > > > > + return; > > > > > > I think you either want this check to operate on the dma_device and be > > > called for both branches, or removed entirely now that the callers > > > setup the dma params. > > > > I would say that all those if(WARN_...) return are too zealous. They can't be > > in our subsystem, so it is better to simply delete all if()s and left blank > > WARN_ON(..). > > > > Something like that: > > if (!dma_device) { > > WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_VIRT_OPS)) > > device->dev.dma_ops = &dma_virt_ops; > > .... > Looks good to me. > Will you revise or I should? I will change it now and resend. Thanks