On Tue, Oct 20, 2020 at 01:31:27PM +0100, Tvrtko Ursulin wrote: > > On 20/10/2020 12:47, Jason Gunthorpe wrote: > > On Tue, Oct 20, 2020 at 12:37:05PM +0100, Tvrtko Ursulin wrote: > > > > > > Why put this confusing code in every caller? Especially for something > > > > a driver is supposed to call. Will just make bugs > > > > > > For max_segment to be aligned is a requirement today so callers are > > > ready. > > > > No, it turns out all the RDMA drivers were became broken when they > > converted to use the proper U32_MAX for their DMA max_segment size, > > then they couldn't form SGLs anymore. > > > > I don't want to see nonsense code like this: > > > > dma_set_max_seg_size(dev->dev, min_t(unsigned int, U32_MAX & PAGE_MASK, > > SCATTERLIST_MAX_SEGMENT)); > > > > In drivers. > > > > dma_set_max_seg_size is the *hardware* capability, and mixing in > > things like PAG_MASK here is just nonsense. > > Code was obviously a no-op non-sense. > > So the crux of the argument is that U32_MAX is conceptually the right thing > which defines the DMA max_segment size? Not some DMA define or anything, but > really U32_MAX? And that all/some DMA hardware does not think in pages but > really in bytes? Yes. The HW has 32 bits for a length field, so U32_MAX accurately defines the HW DMA capability Jason