Re: [PATCH rdma-next] RDMA: Explicitly pass in the dma_device to ib_register_device

From: Christoph Hellwig <hch@infradead.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Bernard Metzler <BMT@zurich.ibm.com>,
	Leon Romanovsky <leon@kernel.org>,
	Doug Ledford <dledford@redhat.com>,
	Adit Ranadive <aditr@vmware.com>,
	Ariel Elior <aelior@marvell.com>,
	Christian Benvenuti <benve@cisco.com>,
	Dennis Dalessandro <dennis.dalessandro@intel.com>,
	Devesh Sharma <devesh.sharma@broadcom.com>,
	Faisal Latif <faisal.latif@intel.com>,
	Gal Pressman <galpress@amazon.com>, Lijun Ou <oulijun@huawei.com>,
	linux-rdma@vger.kernel.org,
	Michal Kalderon <mkalderon@marvell.com>,
	Mike Marciniszyn <mike.marciniszyn@intel.com>,
	Naresh Kumar PBS <nareshkumar.pbs@broadcom.com>,
	Nelson Escobar <neescoba@cisco.com>,
	Parav Pandit <parav@nvidia.com>,
	Parvi Kaustubhi <pkaustub@cisco.com>,
	Potnuri Bharat Teja <bharat@chelsio.com>,
	Selvin Xavier <selvin.xavier@broadcom.com>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>,
	VMware PV-Drivers <pv-drivers@vmware.com>,
	Weihang Li <liweihang@huawei.com>,
	"Wei Hu(Xavier)" <huwei87@hisilicon.com>,
	Yishai Hadas <yishaih@nvidia.com>,
	Zhu Yanjun <yanjunz@nvidia.com>
Subject: Re: [PATCH rdma-next] RDMA: Explicitly pass in the dma_device to ib_register_device
Date: Wed, 23 Sep 2020 06:39:55 +0100	[thread overview]
Message-ID: <20200923053955.GB4809@infradead.org> (raw)
In-Reply-To: <20200922162206.GD3699@nvidia.com>

On Tue, Sep 22, 2020 at 01:22:06PM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 22, 2020 at 02:22:01PM +0000, Bernard Metzler wrote:
> > ...
> > 
> > >> >diff --git a/drivers/infiniband/sw/siw/siw_main.c
> > >> >b/drivers/infiniband/sw/siw/siw_main.c
> > >> >index d862bec84376..0362d57b4db8 100644
> > >> >+++ b/drivers/infiniband/sw/siw/siw_main.c
> > >> >@@ -69,7 +69,7 @@ static int siw_device_register(struct siw_device
> > >> >*sdev, const char *name)
> > >> >
> > >> > 	sdev->vendor_part_id = dev_id++;
> > >> >
> > >> >-	rv = ib_register_device(base_dev, name);
> > >> >+	rv = ib_register_device(base_dev, name, NULL);
> > >> > 	if (rv) {
> > >> > 		pr_warn("siw: device registration error %d\n", rv);
> > >> > 		return rv;
> > >> >@@ -386,6 +386,8 @@ static struct siw_device
> > >> >*siw_device_create(struct net_device *netdev)
> > >> > 	base_dev->dev.dma_parms = &sdev->dma_parms;
> > >> > 	sdev->dma_parms = (struct device_dma_parameters)
> > >> > 		{ .max_segment_size = SZ_2G };
> > >> >+	dma_coerce_mask_and_coherent(&base_dev->dev,
> > >> >+				     dma_get_required_mask(&base_dev->dev));
> > >>
> > >> Leon, can you please help me to understand this
> > >> additional logic? Do we need to setup the DMA device
> > >> for (software) RDMA devices which rely on dma_virt_ops
> > >> in the end, or better leave it untouched?
> > >
> > >The logic that driver is responsible to give right DMA device,
> > >so yes, you are setting here mask from dma_virt_ops, as RXE did.
> > >
> > Thanks Leon!
> > 
> > I wonder how this was working w/o that before!
> 
> I wonder if dma_virt_ops ignores the masking.. Still seems best to set
> it consistently when using dma_virt_ops.

dma_virt_ops doesn't look at the mask.  But in retrospective
dma_virt_ops has been a really bad idea.  I'd much rather move the
handling of non-physical devices back into the RDMA core in the long
run.