On Thu, Sep 03, 2026 at 10:49:38AM -0300, Jason Gunthorpe wrote: > On Wed, Sep 02, 2026 at 07:07:50PM -0400, David Hu wrote: > > On Wed, Sep 2, 2026 at 8:08 AM Jason Gunthorpe <[email protected]> wrote: > > > > > > On Tue, Sep 01, 2026 at 05:08:49PM +0000, David Hu wrote: > > > > From: David Hu <[email protected]> > > > > > > > > Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`. > > > > This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the > > > > first entry, resulting in non-page-aligned DMA addresses for all > > > > subsequent entries. > > > > > > This patch is fine, but pedenatically a scatterlist's entry limit > > > should be bounded to dma_get_max_seg_size(), though I don't think it > > > helps this. Operating scatterlists at the size limits has proven > > > problematic in a number of places already... > > > > > > > In addition, a non-page-aligned sgl length will trigger an edge case > > > > in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later > > > > buffers, we will have a `va` with lowest bit set to 1. That will lead > > > > to `ib_umem_find_best_pgsz()` always return 0, and break the promise > > > > to find best page size for the mapping on the NIC side. > > > > > > That's an IB side bug, the newer logic that joins adjacent SGLs should > > > have avoided it? > > > > Hi Jason, > > > > Thank you for the review. I think you are right on both counts. > > Regarding the IB side, the new SGL joining logic in > > `ib_umem_find_best_pgsz()` indeed avoids the issue. `mask |= va` is > > skipped for artifically split, contiguous SGLs. Since Christian has > > already pulled v8 into drm-misc-next, I won't spin a v9 to avoid > > creating unnecessary noise on the list. If you prefer a followup, > > please let me know. > > Nope, I'm fine, it just explains why it wasn't seen in other > tested. You were backporting and mix&matched things. It confirms the > upstream kernel was fine from the start.
All that time, I wondered why our testing didn't uncover any issues like this, given that the requirement to support large BARs was raised almost immediately during testing. Thanks > > There are other importers besides RDMA, so I still think that this is > a good change regardless. Having each segment remain page aligned, and > staying away from ULONG_MAX that might trigger overflows is a friendly > and robust thing to do for less sophisticated importers. > > Jason >
