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

From: Jason Gunthorpe <jgg@nvidia.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Leon Romanovsky <leon@kernel.org>,
	Doug Ledford <dledford@redhat.com>,
	Adit Ranadive <aditr@vmware.com>,
	"Ariel Elior" <aelior@marvell.com>,
	Bernard Metzler <bmt@zurich.ibm.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: Tue, 6 Oct 2020 13:53:46 -0300	[thread overview]
Message-ID: <20201006165346.GK4734@nvidia.com> (raw)
In-Reply-To: <f20a4639-7674-8d2c-66dc-ebc028b14ef0@acm.org>

On Tue, Oct 06, 2020 at 07:29:16AM -0700, Bart Van Assche wrote:
> On 9/24/20 4:49 AM, Jason Gunthorpe wrote:
> > On Thu, Sep 24, 2020 at 06:49:07AM +0100, Christoph Hellwig wrote:
> > > > > > +	} else {
> > > > > > +		device->dev.dma_parms = dma_device->dma_parms;
> > > > > >   		/*
> > > > > > +		 * Auto setup the segment size if a DMA device was passed in.
> > > > > > +		 * The PCI core sets the maximum segment size to 64 KB. Increase
> > > > > > +		 * this parameter to 2 GB.
> > > > > >   		 */
> > > > > > +		dma_set_max_seg_size(dma_device, SZ_2G);
> > > > > 
> > > > > You can't just inherity DMA properties like this this.  Please
> > > > > fix all code that looks at the seg size to look at the DMA device.
> > > > 
> > > > Inherit? This is overriding the PCI default of 64K to be 2G for RDMA
> > > > devices.
> > > 
> > > With inherit I mean the
> > > 
> > > 		device->dev.dma_parms = dma_device->dma_parms;
> > > 
> > > line, which is completely bogus.  All DMA mapping is done on the
> > > dma_device in the RDMA core and ULPs, so it also can't have an effect.
> > 
> > Oh. Yes, no idea why that is there..
> > 
> > commit c9121262d57b8a3be4f08073546436ba0128ca6a
> > Author: Bart Van Assche <bvanassche@acm.org>
> > Date:   Fri Oct 25 15:58:30 2019 -0700
> > 
> >      RDMA/core: Set DMA parameters correctly
> >      The dma_set_max_seg_size() call in setup_dma_device() does not have any
> >      effect since device->dev.dma_parms is NULL. Fix this by initializing
> >      device->dev.dma_parms first.
> > 
> > Bart?
> 
> (just noticed this email)
> 
> Hi Jason,
>
> That code may be a leftover from when the ib_dma_*() functions used &dev->dev as
> their first argument instead of dev->dma_device. 

Hmm the above was two years after the commit that added dma_device? I
assumed you added this because you were doing testing with rxe?

Jason