Re: [PATCH for-next] RDMA/core: Fix best page size finding when it can cross SG entries

[Leon Romanovsky <leon@kernel.org>]

On Sun, Feb 09, 2025 at 02:26:08PM +0000, Michael Margolin wrote:
> A single scatter-gather entry is limited by a 32 bits "length" field
> that is practically 4GB - PAGE_SIZE. This means that even when the
> memory is physically contiguous, we might need more than one entry to
> represent it. Additionally when using dmabuf, the sg_table might be
> originated outside the subsystem and optimized for other needs.
> 
> For instance an SGT of 16GB GPU continuous memory might look like this:
> (a real life example)
> 
> dma_address 34401400000, length fffff000
> dma_address 345013ff000, length fffff000
> dma_address 346013fe000, length fffff000
> dma_address 347013fd000, length fffff000
> dma_address 348013fc000, length 4000
> 
> Since ib_umem_find_best_pgsz works within SG entries, in the above case
> we will result with the worst possible 4KB page size.
> 
> Fix this by taking into consideration only the alignment of addresses of
> real discontinuity points rather than treating SG entries as such, and
> adjust the page iterator to correctly handle cross SG entry pages.
> 
> There is currently an assumption that drivers do not ask for pages
> bigger than maximal DMA size supported by their devices.
> 
> Reviewed-by: Firas Jahjah <firasj@amazon.com>
> Reviewed-by: Yonatan Nachum <ynachum@amazon.com>
> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> ---
>  drivers/infiniband/core/umem.c  | 34 +++++++++++++++++++++++----------
>  drivers/infiniband/core/verbs.c | 11 ++++++-----
>  2 files changed, 30 insertions(+), 15 deletions(-)

Applied with the following change to prevent arithmetic overflow.

diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index e7e428369159..63a92d6cfbc2 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -112,8 +112,7 @@ unsigned long ib_umem_find_best_pgsz(struct ib_umem *umem,
                /* If the current entry is physically contiguous with the previous
                 * one, no need to take its start addresses into consideration.
                 */
-               if (curr_base + curr_len != sg_dma_address(sg)) {
-
+               if (curr_base != sg_dma_address(sg) - curr_len) {
                        curr_base = sg_dma_address(sg);
                        curr_len = 0;