Re: [PATCH v3 25/25] IB/mlx4: Workaround for mlx4_alloc_priv_pages() array allocator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Jun 22, 2016, at 10:04 AM, Sagi Grimberg <sagi@xxxxxxxxxxx> wrote:
> 
> 
>> +    /* This is overkill, but hardware requires that the
>> +     * PBL array begins at a properly aligned address and
>> +     * never occupies the last 8 bytes of a page.
>> +     */
>> +    mr->pages = (__be64 *)get_zeroed_page(GFP_KERNEL);
>> +    if (!mr->pages)
>>          return -ENOMEM;
> 
> Again, I'm not convinced that this is a better choice then allocating
> the exact needed size as dma coherent, but given that the dma coherent
> allocations are always page aligned I wander if it's not the same
> effect...

My concerns with DMA coherent were:

1. That pool may be a somewhat limited resource?

2. IMO DMA-API.txt suggests DMA coherent will perform less
well in some cases. Macro benchmarks I ran seemed to show
there was a slight performance hit with that approach, though
it was nearly in the noise.

I agree that the over-allocation in the streaming solution is a
concern. But as you say, there may be little we can do about it.

Wrt to Or's comment, the device's maximum page list depth
is advertised to consumers via the device's attributes. However,
it would be defensive if there was a sanity check added in
mlx4_alloc_priv_pages to ensure that the max_pages argument
is a reasonable value (ie, that the calculated array size does
indeed fit into a page).

> In any event, we can move forward with this for now:
> 
> Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx>

Thanks, I'll add that! Though as before, I'm happy to drop this
patch if there is a different preferred official fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux