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

["Margolin, Michael" <mrgolin@amazon.com>]

On 2/13/2025 4:04 PM, Jason Gunthorpe wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Thu, Feb 13, 2025 at 02:51:26PM +0200, Leon Romanovsky wrote:
>> 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;
> I'm not sure about this, what ensures sg_dma_address() > curr_len?
>
> curr_base + curr_len could also overflow, we've seen that AMD IOMMU
> sometimes uses the very high addresess already

I think the only case we care about where curr_base + curr_len can 
overflow is when next sg_dma_address() == 0.

But maybe we should just add an explicit check:

-               if (curr_base + curr_len != sg_dma_address(sg)) {
+               if (curr_base + curr_len < curr_base ||
+                   curr_base + curr_len != sg_dma_address(sg)) {
                         curr_base = sg_dma_address(sg);
                         curr_len = 0;

Michael